Re: [PATCH 1/* v3] Generic string function optimization: Add skeleton

[OndÅej BÃlka <neleai at seznam dot cz>]

On Fri, May 29, 2015 at 11:38:51AM +0000, Joseph Myers wrote:
> On Fri, 29 May 2015, OndÅej BÃlka wrote:
> 
> > > If, instead, the code does
> > > 
> > > #if MACRO
> > > 
> > > and there's a sysdeps/generic header that defines MACRO one way, if an 
> > > architecture overrides that header with one that misspells the name, a 
> > > -Wundef warning will be immediately visible (though we still need to fix 
> > > the -Wundef warnings in the testsuite and remove the -Wno-error=undef).
> > > 
> > Joseph, read previous mail before writing. Your suggestion is pointless.
> 
> Which previous mail?  As far as I can tell, your last patch posting adding 
> common.h is <https://sourceware.org/ml/libc-alpha/2015-05/msg00751.html>,

This one. I already explained that for using if you would need
additional header to be able to undefine macros.

https://sourceware.org/ml/libc-alpha/2015-05/msg00812.html


> 
> (a) each patch submission is self-contained - has the full self-contained 
> write-up of the patch itself with the rationale for the patch and all the 
> choices made, that would go in the commit message, followed by the 
> description of changes from the previous version, rather than requiring a 
> trail of previous messages to be followed to get the full rationale; and
>
Almost nobody does that for good reason, see that most v2 on lists are
shorter than previous. You should write mostly what changed. Sure, I
could copy-paste three pages from original mail with rationale but most
readers would skip it as its duplicate and would skip changes made into
it.

I could make recapitulation once per while but for incremental
improvements its best to keep just increments.
 
> (b) the documentation is more important than the code (write first for 
> humans to read, only then for computers to execute); documenting the 
> interfaces (such as FAST_CLZ and NEED_BITWISE) should be a very early step 
> before any patches are sent to the list, not an afterthought, and the same 
> applies to each internal function and macro in the code, even those that 
> are not interfaces for architectures to reimplement.
> 
Thats not completely true as purpose of this is get every bit of
performance before you need to go into assembly. It depends how
technical my interface will become, with strcmp I found that I need
handle another primitive. As documentation its better to have good
overview than overly verbose one. Some macros there are just there for
optimizer to try both branches and select better one.


> > For a code
> > 
> > #ifndef CUSTOM_FOO
> > int foo()
> > {
> > }
> > #endif
> > 
> > If architecture does
> > #define CUTSOM_FOO
> > int foo()
> > {
> > }
> > 
> > Then it gets error with redefinition of foo.
> 
> Or you could avoid making readers think about whether the #ifndef is OK in 
> a particular case by simply following normal glibc practices and have a 
> sysdeps/generic/string-foo.h header that has a default definition with a 
> careful comment explaining the semantics, and then architectures can have 
> their own version to replace it as needed; no macros needed at all.
> 
First its too typo-prone. Architecture maintainer would create
string_foo.h file and never notice that it was silently ignored.


Then why didn't you said that directly? That saves time instead
suggesting if when you mean separate file. Thats correct as there are
several variants and you need to run benchmark to select correct one.