Re: [PATCH] powerpc: Fix build of wcscpy with --disable-multi-arch

["Gabriel F. T. Gomes" <gabriel at inconstante dot eti dot br>]

On Sun, Mar 03 2019, Adhemerval Zanella wrote:
>  
> On 02/03/2019 19:06, Gabriel F. T. Gomes wrote:
> > 
> > powerpc64 and powerpc64le builds fail when configured with
> > --disable-multi-arch, due to an undefined reference to __GI___wcscpy.
> > This patch fixes this on sysdeps/powerpc/powerpc64/power6/wcscpy.c,
> > which is only used when multi-arch is disabled.
> > 
> > This patch does nothing for the failures on 32-bits powerpc builds,
> > because the file is under the powerpc64 subdirectory, however, powerpc
> > builds were already failing with --disable-multi-arch, with multiple
> > error messages, even before the aforementioned commit.
> > 
> > Tested for powerpc, powerpc64, and powerpc64le with multi-arch enabled
> > (all pass) and disabled (powerpc still fails as explained above).
> > 
>
> LGTM.

Thanks.

> In fact you need either your compiler to setup to to use power6+
> as default or configure with --with-cpu (the powerpc64le gcc I used
> to check in fact do not have neither set).

I usually build powerpc64le with --with-cpu=power8, because that's the
machine where little-endian is supposed to work, and since that's what
distros typically use, and that's why I saw the failure.

I don't do the same for powerpce64...  I only wrote that such build is
failing, because I ran an extra test, just to check what was going on
(not because I know of any distros that use --with-cpu=power6 (or
newer)), I'm sorry if I gave the impression that this was the case.

Anyhow, yes, one would need to use --with-cpu to get the failure, thanks
for making that clear.

I will commit this patch with that information.

> As a side note, this is exactly the kind of annoyance I tried to avoid
> when I advocated to reduce the build permutations on glibc for the 
> recent s390 ifunc refactor (and it bit me with an issue recently [1]).
> So I would ask again if these micro-optimization indeed pays for the 
> over-complicated build options we need to actually test to make sure 
> nothing broken glibc build.

We will have to check if they actually do, and I'll review your new
patch set.  I'm not so sure about the representativeness (I hope that
this is an actual word in English) of the benchmarks, though.  This has
always been an open-question for me.

> For powerpc64 there are two issue: 1. can't we just infer the minimum
> cpu support from compiler or CC and get rid of --with-cpu?

I thought that --with-cpu was useful for distros, so that they could
decide what minimum cpu they want to build support for.  Are you
implying that they could set this in the compiler?

> I would like
> to avoid to add the possible permutation of --with-cpu=powerX and
> --disable-multi-arch of the required build to check. It won't help much
> now since powerpc still organizes itself with sysdeps subfolder, but
> it can be a first step.

This is new for me.  I thought that the options were good, in the sense
that distros can use whatever combination they deem appropriate for
their users.  I don't even know the reasons why one would disable
multi-arch builds, I only assumed that distros and users would know
better.
 
> Second, and this is minor one, I will send a patch to just remove the
> power6 wcs* optimized routines. They are rarely used in the wild, the
> optimization are just loop unrolling that we can either rely on 
> compiler (sadly gcc does not unroll this kind of loop either with
> aggressive optimization flags) or add on generic implementation,
> and power6 and power7 are essentially the same binary for
> powerpc64.

Thanks for doing this.