Re: [PATCH 5/8] Remove __P and convert to ANSI prototypes.

[Corinna Vinschen <vinschen at redhat dot com>]

On Jan 31 12:05, Craig Howland wrote:
> On 1/31/19 8:05 AM, Sebastian Huber wrote:
> > From: obrien <obrien@FreeBSD.org>
> > 
> > * Remove 'register'. (some functions had 7+ register functions...)
> > * Fix SCM ID's.
> > ---
> >   newlib/libc/posix/scandir.c | 15 ++++++---------
> >   1 file changed, 6 insertions(+), 9 deletions(-)
> > 
> > diff --git a/newlib/libc/posix/scandir.c b/newlib/libc/posix/scandir.c
> > index 97a16cf7b..8404cd0de 100644
> > --- a/newlib/libc/posix/scandir.c
> > +++ b/newlib/libc/posix/scandir.c
> > @@ -33,6 +33,7 @@
> >   #include <sys/cdefs.h>
> >   __SCCSID("@(#)scandir.c	8.3 (Berkeley) 1/2/94");
> > +__FBSDID("$FreeBSD$");
> >   /*
> >    * Scan the directory dirname calling select to make a list of selected
> > @@ -64,18 +65,14 @@ __SCCSID("@(#)scandir.c	8.3 (Berkeley) 1/2/94");
> >       (offsetof (struct dirent, d_name) + ((strlen((dp)->d_name)+1 + 3) &~ 3))
> >   #endif
> > -#ifndef __P
> > -#define __P(args) ()
> > -#endif
> >   int
> > -scandir (const char *dirname,
> > -	struct dirent ***namelist,
> > -	int (*select) __P((const struct dirent *)),
> > -	int (*dcomp) __P((const struct dirent **, const struct dirent **)))
> > +scandir(const char *dirname, struct dirent ***namelist,
> > +    int (*select)(const struct dirent *), int (*dcomp)(const struct dirent **,
> > +	const struct dirent **))
> >   {
> > -	register struct dirent *d, *p, **names;
> > -	register size_t nitems;
> > +	struct dirent *d, *p, **names;
> > +	size_t nitems;
> >   	struct stat stb;
> >   	long arraysz;
> >   	DIR *dirp;
>      Why?  This seems a step backwards, as the coder is giving a
> recommendation to the compiler, presumably based on the coder's knowledge of
> the algorithm.  register can effectively be ignored by compilers (C99
> section 6.7.1), so there is no harm in having them. In this particular case
> there are 9 variables declared and register is put on 4 of them.  Even if
> you put stock into the general complaint of 'some functions had 7+ register
> functions', it is not so applicable to 4.
>       I do not support the complaint as a general rule, anyway.  If there
> were a function with 30 variables and 8 had register, that could be fine.  7
> out of 7, perhaps then reasonable to wonder.  One of the hallmarks of RISC
> is lots of registers.  Plenty more than 7+, even, should generally be
> available on most platforms.  (32 GPRs is pretty common, minus a few for ABI
> purposes leaves perhaps a couple dozen.)  Now if this had been brought up
> back in 1994 when this file is dated, then 4 register would have been a more
> interesting discussion (M68k only had 8 GPR, etc.), but not now.
>      I understand the desire to stay in sync with FreeBSD, but does that
> include bad decisions?  I suppose perhaps it should--but only with
> discussion.
>                 Craig

If the developer guesses on what should be done to optimize, rather then
letting the compiler decide what to optimize, the developer is usually
wrong.  That's what -O2 is for, no?  On a RISC with lots of registers
I expect the compiler to use them wisely.


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

Attachment: signature.asc
Description: PGP signature