This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Avoid use of "register" as optimization hint


On Wed, 5 Jun 2013, Ondrej Bilka wrote:

> Nice, I written similar tool (attached.) It should do job more
> exhaustively. Perhaps we can merge these lists?

"more exhaustively" isn't in general a good thing here.  Rather, cleanups 
should be optimized for it being transparent to the human reader that 
everything in the patch is indeed the same kind of genuine cleanup that is 
obviously and unambiguously desirable, with anything possibly 
controversial or distinctive being split into separate smaller patches.

* Cleanup patches should have a size suitable for the human posting them 
to review for any consequent changes (e.g. reindentation) needed, and for 
anything that looks odd or is best considered separately, before posting 
them.  (In this case, there was one bit of reindentation needed in 
strtod_l.c, and several places needing adjustment of backslashes after a 
line in a macro became shorter.)

* Cleanup patches should best have a size in the human window for review, 
which means producing logical (to humans) units of moderate size.

* If parts of a cleanup feel (subjectively, to humans) different from 
other parts, it's best to separate them out for separate consideration.

In this case, do we want to make these changes to the deprecated sunrpc 
code - and is any of that code actually something generated by rpcgen or 
another tool rather than a source file?  And do we want to clean up code 
from GMP - if we do, perhaps we should at least send cleanups upstream at 
the same time, if the corresponding cleanup hasn't been made there?  For 
the cases where "register" seems to be documentation, it seems a lot less 
clear whether we should clean them up or not.  These are things to 
*discuss* before possibly cleaning things up later.

> As you mentioned to exclude generated files is there simple make rule
> to regenerate them all? 

If you modify the source file, then "make" will regenerate the generated 
file.  In this case, the use of "register" is only in the gperf output, 
not the source file, so there is no regeneration to be done; just don't 
make any changes to those generated files at all.  I determined manually 
which files it seemed appropriate to edit with sed, based on output of 
grep showing "register" uses.

> > * Use of "register" with "asm" to allocate a variable to a particular
> >   register (in syscall sequences, in particular).
> >
> Does this force gcc to use variable as particular register?

Roughly, yes.  See the GCC manual for details.

> > * The legacy sunrpc code, which has lots of uses of "register".
> > 
> > * Code from GMP, which also has lots of uses of "register".  I don't
> >   know if GMP upstream has since cleaned them up; we don't actually
> >   try to stay in sync with GMP upstream (updating GMP code from there
> >   would require FSF permission to use the current GMP versions of the
> >   relevant code under LGPLv2.1+).
> > 
> My patch should also handle these.

The point is first to *discuss* what is appropriate with these rather than 
jumping to patches that may well not have consensus.  These files seemed 
subjectively different enough from the rest to be worth dealing with 
separately.

> > 2013-06-05  Joseph Myers  <joseph@codesourcery.com>
> > 
> Did you generate changelog from diff? If so that script would also
> simplify futher cleanups?

Only the list of files was generated, with sed, not the function entries 
for individual files.

-- 
Joseph S. Myers
joseph@codesourcery.com


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]