build-many-glibcs.py: Add some s390x glibc variants.

There is a s390x configure check which checks the architecture level set.

This ALS influences e.g. which ifunc variants are needed or which one is
the default variant or if the symbol will be an ifunc-symbol at all.

The ALS also enables to use the gcc builtins for e.g. the round function
in libm and others.

Therefore this patch adds some glibc variants which are using different
architecture level sets for s390x.
---
 scripts/build-many-glibcs.py | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

* Stefan Liebler via Libc-alpha:

> There is a s390x configure check which checks the architecture level set.
>
> This ALS influences e.g. which ifunc variants are needed or which one is
> the default variant or if the symbol will be an ifunc-symbol at all.
>
> The ALS also enables to use the gcc builtins for e.g. the round function
> in libm and others.
>
> Therefore this patch adds some glibc variants which are using different
> architecture level sets for s390x.

Do these additional build targets actually result in build breakage?

I'm not sure if anyone does run-time testing on build-many-glibcs.py
output.

I would rather suggest adding an -O3 targets (maybe s390x due to its
inliner differences, and x86-64), where we know we have occasional build
breakage.

Thanks,
Florian

On 05/08/2020 12:39, Florian Weimer via Libc-alpha wrote:
> * Stefan Liebler via Libc-alpha:
> 
>> There is a s390x configure check which checks the architecture level set.
>>
>> This ALS influences e.g. which ifunc variants are needed or which one is
>> the default variant or if the symbol will be an ifunc-symbol at all.
>>
>> The ALS also enables to use the gcc builtins for e.g. the round function
>> in libm and others.
>>
>> Therefore this patch adds some glibc variants which are using different
>> architecture level sets for s390x.
> 
> Do these additional build targets actually result in build breakage?
> 
> I'm not sure if anyone does run-time testing on build-many-glibcs.py
> output.> 
> I would rather suggest adding an -O3 targets (maybe s390x due to its
> inliner differences, and x86-64), where we know we have occasional build
> breakage.

Such build permutations stress how s390 port is organized internally,
where it adds an optimization to avoid build some ifunc variations
if the minimum defined architecture avoids selecting it (different
tha x86_64 that always build all ifunc variant regardless of the
target chip/ABI). Another build permutation is whether it will use
the compiler builtins on some match function or whether it will use
the generic implementation.

I don't have a strong opinion on a '-O3' variant, although currently
build-many-glibcs.py usually focus on ABIs variants that might
trigger different code path within glibc itself (although it does
exercise the compiler itself).

On 8/5/20 5:39 PM, Florian Weimer wrote:
> * Stefan Liebler via Libc-alpha:
> 
>> There is a s390x configure check which checks the architecture level set.
>>
>> This ALS influences e.g. which ifunc variants are needed or which one is
>> the default variant or if the symbol will be an ifunc-symbol at all.
>>
>> The ALS also enables to use the gcc builtins for e.g. the round function
>> in libm and others.
>>
>> Therefore this patch adds some glibc variants which are using different
>> architecture level sets for s390x.
> 
> Do these additional build targets actually result in build breakage?
I've run the script with those new extra-glibcs and all passed.
> 
> I'm not sure if anyone does run-time testing on build-many-glibcs.py
> output.
Sure, but I don't mean run-time testing. Usually all the available
ifunc-variants are run-time tested with one "make check" invocation.

I mean the build-time:
If build with -march=zEC12, there are plenty IFUNC symbols (e.g. for the
string functions).
If build with -march=z15, there is no IFUNC symbol and the string
functions equals the vector-string-functions introduced with z13.

> 
> I would rather suggest adding an -O3 targets (maybe s390x due to its
> inliner differences, and x86-64), where we know we have occasional build
> breakage.
This is a great idea. I will add -O3 for s390x/s390.

> 
> Thanks,
> Florian
>

* Stefan Liebler:

> On 8/5/20 5:39 PM, Florian Weimer wrote:
>> * Stefan Liebler via Libc-alpha:
>> 
>>> There is a s390x configure check which checks the architecture level set.
>>>
>>> This ALS influences e.g. which ifunc variants are needed or which one is
>>> the default variant or if the symbol will be an ifunc-symbol at all.
>>>
>>> The ALS also enables to use the gcc builtins for e.g. the round function
>>> in libm and others.
>>>
>>> Therefore this patch adds some glibc variants which are using different
>>> architecture level sets for s390x.
>> 
>> Do these additional build targets actually result in build breakage?

> I've run the script with those new extra-glibcs and all passed.

Sorry, this is not what I meant.  Let me rephrase.

There is some cost to adding more targets to build-many-glibcs.py.  I
think we should focus on targets where we see breakage due to generic
tree-wide changes.  This means coverage of all ABIs, and ABI variants
which have different toolchain exposure (e.g., PIE, restricted machine
code output with verification in binutils).

Or put different, if we don't see build breakage with -march=z10 like
*ever*, then it does not make sense to build a variant for this.

Thanks,
Florian

On 8/6/20 10:34 AM, Florian Weimer wrote:
> * Stefan Liebler:
> 
>> On 8/5/20 5:39 PM, Florian Weimer wrote:
>>> * Stefan Liebler via Libc-alpha:
>>>
>>>> There is a s390x configure check which checks the architecture level set.
>>>>
>>>> This ALS influences e.g. which ifunc variants are needed or which one is
>>>> the default variant or if the symbol will be an ifunc-symbol at all.
>>>>
>>>> The ALS also enables to use the gcc builtins for e.g. the round function
>>>> in libm and others.
>>>>
>>>> Therefore this patch adds some glibc variants which are using different
>>>> architecture level sets for s390x.
>>>
>>> Do these additional build targets actually result in build breakage?
> 
>> I've run the script with those new extra-glibcs and all passed.
> 
> Sorry, this is not what I meant.  Let me rephrase.
> 
> There is some cost to adding more targets to build-many-glibcs.py.  I
> think we should focus on targets where we see breakage due to generic
> tree-wide changes.  This means coverage of all ABIs, and ABI variants
> which have different toolchain exposure (e.g., PIE, restricted machine
> code output with verification in binutils).
> 
> Or put different, if we don't see build breakage with -march=z10 like
> *ever*, then it does not make sense to build a variant for this.
> 
> Thanks,
> Florian
> 
Okay. Then I won't add the march extra-glibcs.

But I've just tried to add a "s390x -O3" extra-glibc as you've suggested
before:
diff --git a/scripts/build-many-glibcs.py b/scripts/build-many-glibcs.py
index 325591b2c6..b0fb36ba15 100755
--- a/scripts/build-many-glibcs.py
+++ b/scripts/build-many-glibcs.py
@@ -365,7 +365,9 @@ class Context(object):
         self.add_config(arch='s390x',
                         os_name='linux-gnu',
                         glibcs=[{},
-                                {'arch': 's390', 'ccopts': '-m31'}])
+                                {'arch': 's390', 'ccopts': '-m31'}],
+                        extra_glibcs=[{'variant': 'O3',
+                                       'ccopts': '-O3'}])


But unfortunately, "make check" fails with:
FAIL: conform/XPG4/stdio.h/conform
FAIL: conform/XPG42/stdio.h/conform
...
PASSCOMBINED: Availability of variable optopt
PASSCOMBINED: Type of variable optopt
    Namespace violation: "getc_unlocked"
    Namespace violation: "getchar_unlocked"
    Namespace violation: "putc_unlocked"
    Namespace violation: "putchar_unlocked"
FAIL: Namespace of <stdio.h>
----------------------------------------------------------------------------
  Total number of tests   :  168
  Number of failed tests  :    1
  Number of xfailed tests :    0
  Number of skipped tests :    0

Here, conformtest.py is running something like this:
echo "#include <stdio.h>" > tst.c
s390x-glibc-linux-gnu-gcc -O3 -I../include ...  -I.. -fno-builtin -ansi
-D_XOPEN_SOURCE -D_ISOMAC -E tst.c -P -Wp,-dN
=> The output contains the __USE_EXTERN_INLINES from
<glibc>/libio/bits/stdio.h for getc_unlocked, getchar_unlocked,
putc_unlocked and putchar_unlocked.

<glibc>/conform/data/stdio.h-data contains:
#if defined POSIX || defined UNIX98 || defined XOPEN2K || defined
XOPEN2K8 || defined POSIX2008
function int getc_unlocked (FILE*)
function int getchar_unlocked (void)
#endif
#if defined POSIX || defined UNIX98 || defined XOPEN2K || defined
XOPEN2K8 || defined POSIX2008
function int putc_unlocked (int, FILE*)
function int putchar_unlocked (int)
#endif

FAIL: conform/POSIX2008/sys/stat.h/conform
...
PASSCOMBINED: Availability of function utimensat
PASSCOMBINED: Type of function utimensat
    Namespace violation: "mknodat"
FAIL: Namespace of <sys/stat.h>
----------------------------------------------------------------------------
  Total number of tests   :   97
  Number of failed tests  :    1
  Number of xfailed tests :    0
  Number of skipped tests :    0
=> Similar to above, the __USE_EXTERN_INLINES for mknodat is included.


Is there a reason, why ...
- scripts/build-many-glibcs.py configures glibc with
CC="<target-triple>-gcc <ccopts>" instead of CC="<target-triple>-gcc"
CFLAGS="<ccopts>" CXX/CXXFLAGS? If build-many-glibcs.py is configuring
the glibc with the FLAGS, the the "make check" for the O3 extra-glibc is
passing.

- conform/conformtest.py is called without the configured CFLAGS and
thus without optimization? I assume one reason is -Wall -Werror.

Bye
Stefan

On Mon, 10 Aug 2020, Stefan Liebler via Libc-alpha wrote:

> Here, conformtest.py is running something like this:
> echo "#include <stdio.h>" > tst.c
> s390x-glibc-linux-gnu-gcc -O3 -I../include ...  -I.. -fno-builtin -ansi
> -D_XOPEN_SOURCE -D_ISOMAC -E tst.c -P -Wp,-dN
> => The output contains the __USE_EXTERN_INLINES from
> <glibc>/libio/bits/stdio.h for getc_unlocked, getchar_unlocked,
> putc_unlocked and putchar_unlocked.

So this should be reported as a bug in Bugzilla; the inline functions 
ought to have the same feature test macro conditionals as the main 
declarations (i.e. __USE_POSIX199506).

> Is there a reason, why ...
> - scripts/build-many-glibcs.py configures glibc with
> CC="<target-triple>-gcc <ccopts>" instead of CC="<target-triple>-gcc"
> CFLAGS="<ccopts>" CXX/CXXFLAGS? If build-many-glibcs.py is configuring
> the glibc with the FLAGS, the the "make check" for the O3 extra-glibc is
> passing.

The general principle is that all options required in all compilations, 
including ABI options, should go in CC, while options that might get 
overridden in some cases go in CFLAGS.  -g and -O options belong in 
CFLAGS.  build-many-glibcs.py doesn't currently have a mechanism for 
overriding CFLAGS (when testing -Os builds I hardcoded a CFLAGS setting 
for all builds).  It probably does make sense to include some -O3 and -Os 
builds (with some new mechanism to change CFLAGS) by default, so we know 
when those break.

> - conform/conformtest.py is called without the configured CFLAGS and
> thus without optimization? I assume one reason is -Wall -Werror.

There are lots of other variants that could meaningfully be tested with 
conformtest.py and linknamespace.py.  Enabling optimization is one, 
_FILE_OFFSET_BITS=64 is another that's known to have namespace problems 
(bug 14106).  It's useful to know about failures when someone runs tests 
with different options, but less clear exactly what variants should be 
tested by default.

On 8/10/20 7:23 PM, Joseph Myers wrote:
> On Mon, 10 Aug 2020, Stefan Liebler via Libc-alpha wrote:
> 
>> Here, conformtest.py is running something like this:
>> echo "#include <stdio.h>" > tst.c
>> s390x-glibc-linux-gnu-gcc -O3 -I../include ...  -I.. -fno-builtin -ansi
>> -D_XOPEN_SOURCE -D_ISOMAC -E tst.c -P -Wp,-dN
>> => The output contains the __USE_EXTERN_INLINES from
>> <glibc>/libio/bits/stdio.h for getc_unlocked, getchar_unlocked,
>> putc_unlocked and putchar_unlocked.
> 
> So this should be reported as a bug in Bugzilla; the inline functions 
> ought to have the same feature test macro conditionals as the main 
> declarations (i.e. __USE_POSIX199506).
I've created the bugzilla:
"Bug 26376 - Namespace violation in stdio.h and sys/stat.h if build with
optimization."
https://sourceware.org/bugzilla/show_bug.cgi?id=26376

and posted this patch:
"[PATCH] Fix namespace violation in stdio.h and sys/stat.h if build with
optimization. [BZ #26376]"
https://sourceware.org/pipermail/libc-alpha/2020-August/116979.html

> 
>> Is there a reason, why ...
>> - scripts/build-many-glibcs.py configures glibc with
>> CC="<target-triple>-gcc <ccopts>" instead of CC="<target-triple>-gcc"
>> CFLAGS="<ccopts>" CXX/CXXFLAGS? If build-many-glibcs.py is configuring
>> the glibc with the FLAGS, the the "make check" for the O3 extra-glibc is
>> passing.
> 
> The general principle is that all options required in all compilations, 
> including ABI options, should go in CC, while options that might get 
> overridden in some cases go in CFLAGS.  -g and -O options belong in 
> CFLAGS.  build-many-glibcs.py doesn't currently have a mechanism for 
> overriding CFLAGS (when testing -Os builds I hardcoded a CFLAGS setting 
> for all builds).  It probably does make sense to include some -O3 and -Os 
> builds (with some new mechanism to change CFLAGS) by default, so we know 
> when those break.
I've just posted this patch:
"[PATCH] build-many-glibcs.py: Add a s390x -O3 glibc variant."
https://sourceware.org/pipermail/libc-alpha/2020-August/116980.html

> 
>> - conform/conformtest.py is called without the configured CFLAGS and
>> thus without optimization? I assume one reason is -Wall -Werror.
> 
> There are lots of other variants that could meaningfully be tested with 
> conformtest.py and linknamespace.py.  Enabling optimization is one, 
> _FILE_OFFSET_BITS=64 is another that's known to have namespace problems 
> (bug 14106).  It's useful to know about failures when someone runs tests 
> with different options, but less clear exactly what variants should be 
> tested by default.
> 

Thanks,
Stefan