[PATCH] Makerules: pass CFLAGS when building .S files, BZ #23273

[PATCH] Makerules: pass CFLAGS when building .S files, BZ #23273

[Sergei Trofimovich]

From: Sergei Trofimovich <slyfox@gentoo.org>

When glibc is built on armv7 with CFLAGS="-pipe -march=armv7-a -O2 -mfpu=neon"
rtld fails to link due to missing memcpy symbol.

This happens because .c files are built with -mfpu=neon flag but .S/.s files
are built without it. Among other things -mfpu=neon defines __ARM_NEON__.

To fix this mismatch CFLAGS should be passed consistently to .c and .S files.

Bug: https://sourceware.org/PR23273
Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
---
 Makerules | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makerules b/Makerules
index b2c2724fcb..ddf72b3723 100644
--- a/Makerules
+++ b/Makerules
@@ -510,10 +510,10 @@ native-compile-mkdep-flags = -MMD -MP -MF $@.dt -MT $@
 # GCC can grok options after the file name, and it looks nicer that way.
 compile.c = $(CC) $< -c $(CFLAGS) $(CPPFLAGS)
 compile.cc = $(CXX) $< -c $(CXXFLAGS) $(CPPFLAGS)
-compile.S = $(CC) $< -c $(CPPFLAGS) $(S-CPPFLAGS) \
+compile.S = $(CC) $< -c $(CFLAGS) $(CPPFLAGS) $(S-CPPFLAGS) \
 		  $(ASFLAGS) $(ASFLAGS-$(suffix $@))
 COMPILE.c = $(CC) -c $(CFLAGS) $(CPPFLAGS)
-COMPILE.S = $(CC) -c $(CPPFLAGS) $(S-CPPFLAGS) \
+COMPILE.S = $(CC) -c $(CFLAGS) $(CPPFLAGS) $(S-CPPFLAGS) \
 		  $(ASFLAGS) $(ASFLAGS-$(suffix $@))
 
 # We need this for the output to go in the right place.  It will default to
-- 
2.17.1

Re: [PATCH] Makerules: pass CFLAGS when building .S files, BZ #23273

[Florian Weimer]

On 06/11/2018 12:28 PM, Sergei Trofimovich wrote:
> When glibc is built on armv7 with CFLAGS="-pipe -march=armv7-a -O2 -mfpu=neon"
> rtld fails to link due to missing memcpy symbol.
> 
> This happens because .c files are built with -mfpu=neon flag but .S/.s files
> are built without it. Among other things -mfpu=neon defines __ARM_NEON__.
> 
> To fix this mismatch CFLAGS should be passed consistently to .c and .S files.

Please check if you can fix this by putting the architecture selection 
flags into the CC and CXX variables when invoking configure.

Thanks,
Florian

Re: [PATCH] Makerules: pass CFLAGS when building .S files, BZ #23273

[Adhemerval Zanella]

On 11/06/2018 07:46, Florian Weimer wrote:
> On 06/11/2018 12:28 PM, Sergei Trofimovich wrote:
>> When glibc is built on armv7 with CFLAGS="-pipe -march=armv7-a -O2 -mfpu=neon"
>> rtld fails to link due to missing memcpy symbol.
>>
>> This happens because .c files are built with -mfpu=neon flag but .S/.s files
>> are built without it. Among other things -mfpu=neon defines __ARM_NEON__.
>>
>> To fix this mismatch CFLAGS should be passed consistently to .c and .S files.
> 
> Please check if you can fix this by putting the architecture selection flags into the CC and CXX variables when invoking configure.

That is the usual way build-many-glibcs.py does to check for different
variant and this is the way I used to check such flag combination when
I refactor arm ifunc.  The armv7 configuration does build fine when
the options are used for CC and CXX.

Re: [PATCH] Makerules: pass CFLAGS when building .S files, BZ #23273

[Sergei Trofimovich]

[-- Attachment #1: Type: text/plain, Size: 2359 bytes --]

On Mon, 11 Jun 2018 08:17:37 -0300
Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:

> On 11/06/2018 07:46, Florian Weimer wrote:
> > On 06/11/2018 12:28 PM, Sergei Trofimovich wrote:  
> >> When glibc is built on armv7 with CFLAGS="-pipe -march=armv7-a -O2 -mfpu=neon"
> >> rtld fails to link due to missing memcpy symbol.
> >>
> >> This happens because .c files are built with -mfpu=neon flag but .S/.s files
> >> are built without it. Among other things -mfpu=neon defines __ARM_NEON__.
> >>
> >> To fix this mismatch CFLAGS should be passed consistently to .c and .S files.  
> > 
> > Please check if you can fix this by putting the architecture selection flags into the CC and CXX variables when invoking configure.  

I'm not sure just adding an arch flag is enough:
    https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/arm/armv7/multiarch/memcpy_neon.S;h=1a8d8bbe9e128618105be0652de42a34fbc52d73;hb=HEAD
   ...
   2 #ifndef __ARM_NEON__
   3 # define memcpy __memcpy_neon
Here memcpy_neon.S explicitly supports both neon-enabled and disabled flags.

C code assumes the same automatic selection:
    https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/arm/armv7/multiarch/ifunc-impl-list.c;h=48e43da66e6c307dca7cce5f397bca9fbbc265a7;hb=HEAD#l35
   ...
   35 #ifdef __ARM_NEON__
   36 # define __memcpy_neon  memcpy
   37 # define __memchr_neon  memchr
   ...

I see a few ways to do it:
- filter out CFLAGS from -mfpu=neon from C compilation (sounds too harsh)
- add CFLAGS only for memcpy_neon.S file (might be ok)
- do not use __ARM_NEON__ explicitly as an implementation switch but use
  new configure-based flag (

How should mismatch be handled to reach consistency?

> That is the usual way build-many-glibcs.py does to check for different
> variant and this is the way I used to check such flag combination when
> I refactor arm ifunc.  The armv7 configuration does build fine when
> the options are used for CC and CXX.

Looks like -mfpu=neon is not covered by build-many-glibcs.py. -mfpu=neon is needed
to trigger this type of failure as .S/.c files both use #ifndef __ARM_NEON__
and expect consistent setting:
    https://sourceware.org/bugzilla/show_bug.cgi?id=23273#c1

Should it be added explicitly as well? I never used build-many-glibcs.py before.

-- 

  Sergei

[-- Attachment #2: Цифровая подпись OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH] Makerules: pass CFLAGS when building .S files, BZ #23273

[Florian Weimer]

On 06/11/2018 02:35 PM, Sergei Trofimovich wrote:
> On Mon, 11 Jun 2018 08:17:37 -0300
> Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
> 
>> On 11/06/2018 07:46, Florian Weimer wrote:
>>> On 06/11/2018 12:28 PM, Sergei Trofimovich wrote:
>>>> When glibc is built on armv7 with CFLAGS="-pipe -march=armv7-a -O2 -mfpu=neon"
>>>> rtld fails to link due to missing memcpy symbol.
>>>>
>>>> This happens because .c files are built with -mfpu=neon flag but .S/.s files
>>>> are built without it. Among other things -mfpu=neon defines __ARM_NEON__.
>>>>
>>>> To fix this mismatch CFLAGS should be passed consistently to .c and .S files.
>>>
>>> Please check if you can fix this by putting the architecture selection flags into the CC and CXX variables when invoking configure.
> 
> I'm not sure just adding an arch flag is enough:
>      https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/arm/armv7/multiarch/memcpy_neon.S;h=1a8d8bbe9e128618105be0652de42a34fbc52d73;hb=HEAD
>     ...
>     2 #ifndef __ARM_NEON__
>     3 # define memcpy __memcpy_neon
> Here memcpy_neon.S explicitly supports both neon-enabled and disabled flags.

Does this mean your proposed patch fails to address the issue, too?

The tricky part here is that the IFUNC selector is C, but the 
alternative implementation is in assembler, so it's not a matter of 
adding an #include for the rtld build.

I think a file

sysdeps/arm/armv7/multiarch/rtld-memcpy.S

with

#ifdef __ARM_NEON__
# include <sysdeps/arm/armv7/multiarch/memcpy_neon.S>
#else
# include <sysdeps/arm/armv7/multiarch/memcpy_noneon.S>
#endif

might fix this, without affecting the IFUNCs in the libc.so build for 
!__ARM_NEON__.  Similarly for memchr.

Thanks,
Florian

Re: [PATCH] Makerules: pass CFLAGS when building .S files, BZ #23273

[Sergei Trofimovich]

[-- Attachment #1: Type: text/plain, Size: 2733 bytes --]

On Mon, 11 Jun 2018 14:50:24 +0200
Florian Weimer <fweimer@redhat.com> wrote:

> On 06/11/2018 02:35 PM, Sergei Trofimovich wrote:
> > On Mon, 11 Jun 2018 08:17:37 -0300
> > Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
> >   
> >> On 11/06/2018 07:46, Florian Weimer wrote:  
> >>> On 06/11/2018 12:28 PM, Sergei Trofimovich wrote:  
> >>>> When glibc is built on armv7 with CFLAGS="-pipe -march=armv7-a -O2 -mfpu=neon"
> >>>> rtld fails to link due to missing memcpy symbol.
> >>>>
> >>>> This happens because .c files are built with -mfpu=neon flag but .S/.s files
> >>>> are built without it. Among other things -mfpu=neon defines __ARM_NEON__.
> >>>>
> >>>> To fix this mismatch CFLAGS should be passed consistently to .c and .S files.  
> >>>
> >>> Please check if you can fix this by putting the architecture selection flags into the CC and CXX variables when invoking configure.  
> > 
> > I'm not sure just adding an arch flag is enough:
> >      https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/arm/armv7/multiarch/memcpy_neon.S;h=1a8d8bbe9e128618105be0652de42a34fbc52d73;hb=HEAD
> >     ...
> >     2 #ifndef __ARM_NEON__
> >     3 # define memcpy __memcpy_neon
> > Here memcpy_neon.S explicitly supports both neon-enabled and disabled flags.  
> 
> Does this mean your proposed patch fails to address the issue, too?

My proposed patch fixes ld.so linkage (and ld.so seems to work).

> The tricky part here is that the IFUNC selector is C, but the 
> alternative implementation is in assembler, so it's not a matter of 
> adding an #include for the rtld build.

As I understand IFUNC selector is not used at all for -mfpu=neon
target (due to "!defined (__ARM_NEON__)"):
    https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/arm/armv7/multiarch/memcpy.c;h=02776b6fe6ec9aa6aa7efb8c1770970d73fe50d5;hb=HEAD

  22 #if IS_IN (libc) && !defined (__ARM_NEON__)
  ...
  32 arm_libc_ifunc_redirected (__redirect_memcpy, memcpy, IFUNC_SELECTOR);
  33 
  34 arm_libc_ifunc_hidden_def (__redirect_memcpy, memcpy);
  35 #endif

Or in other words how glibc seems to work today:
- -mfpu=neon -> no IFUNC selection, memcpy_neon.S is supposed to define __memcpy
- no -mfpu=neon -> IFUNC selection is used, memcpy_neon.S is supposed to define __memcpy_neon

> I think a file
> 
> sysdeps/arm/armv7/multiarch/rtld-memcpy.S
> 
> with
> 
> #ifdef __ARM_NEON__
> # include <sysdeps/arm/armv7/multiarch/memcpy_neon.S>
> #else
> # include <sysdeps/arm/armv7/multiarch/memcpy_noneon.S>
> #endif
> 
> might fix this, without affecting the IFUNCs in the libc.so build for 
> !__ARM_NEON__.  Similarly for memchr.
> 
> Thanks,
> Florian

-- 

  Sergei

[-- Attachment #2: Цифровая подпись OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH] Makerules: pass CFLAGS when building .S files, BZ #23273

[Florian Weimer]

On 06/11/2018 03:18 PM, Sergei Trofimovich wrote:
> On Mon, 11 Jun 2018 14:50:24 +0200
> Florian Weimer <fweimer@redhat.com> wrote:
> 
>> On 06/11/2018 02:35 PM, Sergei Trofimovich wrote:
>>> On Mon, 11 Jun 2018 08:17:37 -0300
>>> Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
>>>    
>>>> On 11/06/2018 07:46, Florian Weimer wrote:
>>>>> On 06/11/2018 12:28 PM, Sergei Trofimovich wrote:
>>>>>> When glibc is built on armv7 with CFLAGS="-pipe -march=armv7-a -O2 -mfpu=neon"
>>>>>> rtld fails to link due to missing memcpy symbol.
>>>>>>
>>>>>> This happens because .c files are built with -mfpu=neon flag but .S/.s files
>>>>>> are built without it. Among other things -mfpu=neon defines __ARM_NEON__.
>>>>>>
>>>>>> To fix this mismatch CFLAGS should be passed consistently to .c and .S files.
>>>>>
>>>>> Please check if you can fix this by putting the architecture selection flags into the CC and CXX variables when invoking configure.
>>>
>>> I'm not sure just adding an arch flag is enough:
>>>       https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/arm/armv7/multiarch/memcpy_neon.S;h=1a8d8bbe9e128618105be0652de42a34fbc52d73;hb=HEAD
>>>      ...
>>>      2 #ifndef __ARM_NEON__
>>>      3 # define memcpy __memcpy_neon
>>> Here memcpy_neon.S explicitly supports both neon-enabled and disabled flags.
>>
>> Does this mean your proposed patch fails to address the issue, too?
> 
> My proposed patch fixes ld.so linkage (and ld.so seems to work).

Yes, but what about using CC/CXX setting at configure time to achieve 
the same effect?

Thanks,
Florian

Re: [PATCH] Makerules: pass CFLAGS when building .S files, BZ #23273

[Sergei Trofimovich]

[-- Attachment #1: Type: text/plain, Size: 2052 bytes --]

On Mon, 11 Jun 2018 15:22:50 +0200
Florian Weimer <fweimer@redhat.com> wrote:

> On 06/11/2018 03:18 PM, Sergei Trofimovich wrote:
> > On Mon, 11 Jun 2018 14:50:24 +0200
> > Florian Weimer <fweimer@redhat.com> wrote:
> >   
> >> On 06/11/2018 02:35 PM, Sergei Trofimovich wrote:  
> >>> On Mon, 11 Jun 2018 08:17:37 -0300
> >>> Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
> >>>      
> >>>> On 11/06/2018 07:46, Florian Weimer wrote:  
> >>>>> On 06/11/2018 12:28 PM, Sergei Trofimovich wrote:  
> >>>>>> When glibc is built on armv7 with CFLAGS="-pipe -march=armv7-a -O2 -mfpu=neon"
> >>>>>> rtld fails to link due to missing memcpy symbol.
> >>>>>>
> >>>>>> This happens because .c files are built with -mfpu=neon flag but .S/.s files
> >>>>>> are built without it. Among other things -mfpu=neon defines __ARM_NEON__.
> >>>>>>
> >>>>>> To fix this mismatch CFLAGS should be passed consistently to .c and .S files.  
> >>>>>
> >>>>> Please check if you can fix this by putting the architecture selection flags into the CC and CXX variables when invoking configure.  
> >>>
> >>> I'm not sure just adding an arch flag is enough:
> >>>       https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/arm/armv7/multiarch/memcpy_neon.S;h=1a8d8bbe9e128618105be0652de42a34fbc52d73;hb=HEAD
> >>>      ...
> >>>      2 #ifndef __ARM_NEON__
> >>>      3 # define memcpy __memcpy_neon
> >>> Here memcpy_neon.S explicitly supports both neon-enabled and disabled flags.  
> >>
> >> Does this mean your proposed patch fails to address the issue, too?  
> > 
> > My proposed patch fixes ld.so linkage (and ld.so seems to work).  
> 
> Yes, but what about using CC/CXX setting at configure time to achieve 
> the same effect?

The below works on unmodified glibc as well:

$ ../glibc/configure \
    --build=x86_64-pc-linux-gnu \
    --host=armv7a-hardfloat-linux-gnueabi \
    CC="armv7a-hardfloat-linux-gnueabi-gcc -march=armv7-a -mfpu=neon" \
    CFLAGS="-pipe -O2" \
    --prefix=/usr

-- 

  Sergei

[-- Attachment #2: Цифровая подпись OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH] Makerules: pass CFLAGS when building .S files, BZ #23273

[Florian Weimer]

On 06/11/2018 03:41 PM, Sergei Trofimovich wrote:
> The below works on unmodified glibc as well:
> 
> $ ../glibc/configure \
>      --build=x86_64-pc-linux-gnu \
>      --host=armv7a-hardfloat-linux-gnueabi \
>      CC="armv7a-hardfloat-linux-gnueabi-gcc -march=armv7-a -mfpu=neon" \
>      CFLAGS="-pipe -O2" \
>      --prefix=/usr

Then please use that instead.  It's what we do on other architectures, too.

Thanks,
Florian

Re: [PATCH] Makerules: pass CFLAGS when building .S files, BZ #23273

[Sergei Trofimovich]

[-- Attachment #1: Type: text/plain, Size: 1164 bytes --]

On Mon, 11 Jun 2018 15:47:41 +0200
Florian Weimer <fweimer@redhat.com> wrote:

> On 06/11/2018 03:41 PM, Sergei Trofimovich wrote:
> > The below works on unmodified glibc as well:
> > 
> > $ ../glibc/configure \
> >      --build=x86_64-pc-linux-gnu \
> >      --host=armv7a-hardfloat-linux-gnueabi \
> >      CC="armv7a-hardfloat-linux-gnueabi-gcc -march=armv7-a -mfpu=neon" \
> >      CFLAGS="-pipe -O2" \
> >      --prefix=/usr  
> 
> Then please use that instead.  It's what we do on other architectures, too.

Aha. Gentoo allows user to specify CFLAGS from a limited (but not small) set of
flags when glibc is built locally:
    https://gitweb.gentoo.org/repo/gentoo.git/tree/eclass/flag-o-matic.eclass#n25

Does it mean that canonical way to build glibc would be to always pass all
CFLAGS/CXXFLAGS as part of CC/CXX?
    CC="${TARGET_CC} ${CFLAGS}"
    CXX="${TARGET_CC} ${CXXFLAGS}"

Or should only a small subset of CFLAGS/CXXFLAGS go to CC/CXX?
    CC="${TARGET_CC} ${filtered_CFLAGS}"
    CXX="${TARGET_CC} ${filtered_CXXFLAGS}"

If it should be a filtered subset, how can we easily infer supported subset?

-- 

  Sergei

[-- Attachment #2: Цифровая подпись OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH] Makerules: pass CFLAGS when building .S files, BZ #23273

[Adhemerval Zanella]

[-- Attachment #1.1: Type: text/plain, Size: 1810 bytes --]



On 11/06/2018 12:30, Sergei Trofimovich wrote:
> On Mon, 11 Jun 2018 15:47:41 +0200
> Florian Weimer <fweimer@redhat.com> wrote:
> 
>> On 06/11/2018 03:41 PM, Sergei Trofimovich wrote:
>>> The below works on unmodified glibc as well:
>>>
>>> $ ../glibc/configure \
>>>      --build=x86_64-pc-linux-gnu \
>>>      --host=armv7a-hardfloat-linux-gnueabi \
>>>      CC="armv7a-hardfloat-linux-gnueabi-gcc -march=armv7-a -mfpu=neon" \
>>>      CFLAGS="-pipe -O2" \
>>>      --prefix=/usr  
>>
>> Then please use that instead.  It's what we do on other architectures, too.
> 
> Aha. Gentoo allows user to specify CFLAGS from a limited (but not small) set of
> flags when glibc is built locally:
>     https://gitweb.gentoo.org/repo/gentoo.git/tree/eclass/flag-o-matic.eclass#n25
> 
> Does it mean that canonical way to build glibc would be to always pass all
> CFLAGS/CXXFLAGS as part of CC/CXX?
>     CC="${TARGET_CC} ${CFLAGS}"
>     CXX="${TARGET_CC} ${CXXFLAGS}"
> 
> Or should only a small subset of CFLAGS/CXXFLAGS go to CC/CXX?
>     CC="${TARGET_CC} ${filtered_CFLAGS}"
>     CXX="${TARGET_CC} ${filtered_CXXFLAGS}"
> 
> If it should be a filtered subset, how can we easily infer supported subset?
> 

It is not exactly the canonical way, but rather the usual one which should be
simpler when the compiler flags may change the target ABI.  One option is
to set the ABI flags on CC/CXX, another option is to set on both CFLAGS and
CPPFLAGS.

For armv7 neon you can either set:

./configure CC="gcc -march=armv7-a -mfpu=neon" CXX="g++ -march=armv7-a -mfpu=neon"

or

./configure CFLAGS="-O2 -march=armv7-a -mfpu=neon" CPPFLAGS="-O2 -march=armv7-a -mfpu=neon"

The only difference is you will need to explicit set the optimization level when
setting CFLAGS.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] Makerules: pass CFLAGS when building .S files, BZ #23273

[Sergei Trofimovich]

[-- Attachment #1: Type: text/plain, Size: 2511 bytes --]

On Mon, 11 Jun 2018 18:25:19 -0300
Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:

> On 11/06/2018 12:30, Sergei Trofimovich wrote:
> > On Mon, 11 Jun 2018 15:47:41 +0200
> > Florian Weimer <fweimer@redhat.com> wrote:
> >   
> >> On 06/11/2018 03:41 PM, Sergei Trofimovich wrote:  
> >>> The below works on unmodified glibc as well:
> >>>
> >>> $ ../glibc/configure \
> >>>      --build=x86_64-pc-linux-gnu \
> >>>      --host=armv7a-hardfloat-linux-gnueabi \
> >>>      CC="armv7a-hardfloat-linux-gnueabi-gcc -march=armv7-a -mfpu=neon" \
> >>>      CFLAGS="-pipe -O2" \
> >>>      --prefix=/usr    
> >>
> >> Then please use that instead.  It's what we do on other architectures, too.  
> > 
> > Aha. Gentoo allows user to specify CFLAGS from a limited (but not small) set of
> > flags when glibc is built locally:
> >     https://gitweb.gentoo.org/repo/gentoo.git/tree/eclass/flag-o-matic.eclass#n25
> > 
> > Does it mean that canonical way to build glibc would be to always pass all
> > CFLAGS/CXXFLAGS as part of CC/CXX?
> >     CC="${TARGET_CC} ${CFLAGS}"
> >     CXX="${TARGET_CC} ${CXXFLAGS}"
> > 
> > Or should only a small subset of CFLAGS/CXXFLAGS go to CC/CXX?
> >     CC="${TARGET_CC} ${filtered_CFLAGS}"
> >     CXX="${TARGET_CC} ${filtered_CXXFLAGS}"
> > 
> > If it should be a filtered subset, how can we easily infer supported subset?
> >   
> 
> It is not exactly the canonical way, but rather the usual one which should be
> simpler when the compiler flags may change the target ABI.  One option is
> to set the ABI flags on CC/CXX, another option is to set on both CFLAGS and
> CPPFLAGS.
> 
> For armv7 neon you can either set:
> 
> ./configure CC="gcc -march=armv7-a -mfpu=neon" CXX="g++ -march=armv7-a -mfpu=neon"
> 
> or
> 
> ./configure CFLAGS="-O2 -march=armv7-a -mfpu=neon" CPPFLAGS="-O2 -march=armv7-a -mfpu=neon"
> 
> The only difference is you will need to explicit set the optimization level when
> setting CFLAGS.

CFLAGS/CPPFLAGS should work for us.

I was about to ask why glibc is special in handling CFLAGS. But I guess it's not
special and is mimicked after default rules of GNU make. Which is:

  .o: .S
    $(AS) $(CPPFLAGS) $(ASFLAGS) ...
  .o: .s
    $(AS) $(ASFLAGS) ...

Thus passing ABI flags as part of ASFLAGS might be more generic (for projects
that use .s and .S). But for glibc CPPFLAGS are used in the same context as CPPFLAGS.

Thanks! I'll close the bug as invalid.

-- 

  Sergei

[-- Attachment #2: Цифровая подпись OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH] Makerules: pass CFLAGS when building .S files, BZ #23273

[Adhemerval Zanella]

On 11/06/2018 10:22, Florian Weimer wrote:
> On 06/11/2018 03:18 PM, Sergei Trofimovich wrote:
>> On Mon, 11 Jun 2018 14:50:24 +0200
>> Florian Weimer <fweimer@redhat.com> wrote:
>>
>>> On 06/11/2018 02:35 PM, Sergei Trofimovich wrote:
>>>> On Mon, 11 Jun 2018 08:17:37 -0300
>>>> Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
>>>>   
>>>>> On 11/06/2018 07:46, Florian Weimer wrote:
>>>>>> On 06/11/2018 12:28 PM, Sergei Trofimovich wrote:
>>>>>>> When glibc is built on armv7 with CFLAGS="-pipe -march=armv7-a -O2 -mfpu=neon"
>>>>>>> rtld fails to link due to missing memcpy symbol.
>>>>>>>
>>>>>>> This happens because .c files are built with -mfpu=neon flag but .S/.s files
>>>>>>> are built without it. Among other things -mfpu=neon defines __ARM_NEON__.
>>>>>>>
>>>>>>> To fix this mismatch CFLAGS should be passed consistently to .c and .S files.
>>>>>>
>>>>>> Please check if you can fix this by putting the architecture selection flags into the CC and CXX variables when invoking configure.
>>>>
>>>> I'm not sure just adding an arch flag is enough:
>>>>       https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/arm/armv7/multiarch/memcpy_neon.S;h=1a8d8bbe9e128618105be0652de42a34fbc52d73;hb=HEAD
>>>>      ...
>>>>      2 #ifndef __ARM_NEON__
>>>>      3 # define memcpy __memcpy_neon
>>>> Here memcpy_neon.S explicitly supports both neon-enabled and disabled flags.
>>>
>>> Does this mean your proposed patch fails to address the issue, too?
>>
>> My proposed patch fixes ld.so linkage (and ld.so seems to work).
> 
> Yes, but what about using CC/CXX setting at configure time to achieve the same effect?

It works as expected by using the flags on CC/CXX:

  1. For only '-march=armv7-a' sysdeps/arm/armv7/multiarch/rtld-memcpy.S will be used
     and it will redirect to use sysdeps/arm/armv7/multiarch/rtld-memcpy.S.

  2. For '-march=armv7-a -mfpu=neon' string/memcpy.os will be empty which will force
     librtld.map to point to memcpy_neon.os to be used as the linker implementation
     (which creates the rtld-memcpy_neon.os).

Now the question is whether we should support the same behaviour with the ABI options
set only in CFLAGS instead.

NOTE: from previous discussion it seems armv7-a should imply NEON, so I think 1. should
behave as 2. which means I think we should test for __ARM_ARCH == 7 instead of __ARM_NEON__.

Re: [PATCH] Makerules: pass CFLAGS when building .S files, BZ #23273

[Adhemerval Zanella]

On 11/06/2018 10:47, Adhemerval Zanella wrote:

> 
> NOTE: from previous discussion it seems armv7-a should imply NEON, so I think 1. should
> behave as 2. which means I think we should test for __ARM_ARCH == 7 instead of __ARM_NEON__.
> 

nvm this part, I was a confusion from my part from the thumb2 discussion.