Re: [PATCH 01/12] tune/arm: Set -mtune instead of -mcpu

[Khem Raj <raj.khem@gmail.com>]

On 6/7/18 4:38 PM, Andre McCurdy wrote:
> On Thu, Jun 7, 2018 at 7:04 AM, Khem Raj <raj.khem@gmail.com> wrote:
>> On Thu, Jun 7, 2018 at 12:14 AM, Andre McCurdy <armccurdy@gmail.com> wrote:
>>> On Wed, Jun 6, 2018 at 10:58 PM, Khem Raj <raj.khem@gmail.com> wrote:
>>>> On 6/6/18 4:42 PM, Andre McCurdy wrote:
>>>>> On Wed, Jun 6, 2018 at 3:43 PM, Khem Raj <raj.khem@gmail.com> wrote:
>>>>>
>>>>> The -mcpu, -march and -mtune options are not new and gcc 6 and 7 catch
>>>>> the same conflicts. It doesn't make sense that gcc8 is just catching
>>>>> more issues.
>>>>
>>>> It does make sense. the option parsing for these specific options on arm
>>>> have been revamped after gcc7, see
>>>>
>>>> https://github.com/kraj/gcc/compare/a99ae290af49793cd3db7a74f3dbc59e64d356a1...68b54adbd7b10c66d968d74b96fba552bd46ebb7
>>>
>>> Thanks. These commits seem to be related to handling of options like
>>> "-mcpu=cortexa9+nosimd". Was that the error you saw in testing?
>>>
>>> If you can provide the command line that caused the error then it
>>> should be quick to establish whether it's gcc8 being more picky.
>>>
>>> Or perhaps there's always been a warning and -Werror has been added to
>>> a gcc8 Makefile where it wasn't before?
>>>
>>>>> If we are trying to build something which is reusable across multiple
>>>>> machines with the same architecture then it's a bug to be passing
>>>>> machine specific CFLAGS. Making the machine specific CFLAGS more
>>>>> generic is not the right solution.
>>>>
>>>> being reusable is a side-effect and a good one. Real problem is we are not
>>>> matching to what we say in package arches, Probably you are confusing tunes
>>>> to be meant for static code generation for a given CPU.
>>>
>>> Sorry, I don't really follow what you mean?
>>>
>>>> I am interested to
>>>> hear more ideas to what would be right solution if this is not it.
>>>
>>> I'd like to understand what the problem is first before trying to
>>> propose any solutions.
>>>
>>> ie what specifically has changed with gcc8 to cause the error which
>>> wasn't seen before?
>>
>> I would suggest take this gcc8 patch series and revert this one then build
>> gcc-runtime for rpi3
> 
> That's the answer to "how do I reproduce the issue" not to "what is the issue".
> 

another way to nudge for some help :) Thanks for digging further details.

> Anyway, I can reproduce the issue. The root cause is that gcc-runtime
> libatomic tries to support runtime selection between different
> implementations of a few low level functions by making use of the gcc
> "ifunc" function attribute:
> 
>    https://gcc.gnu.org/onlinedocs/gcc-8.1.0/gcc/Common-Function-Attributes.html#index-ifunc-function-attribute
> 
> ie libatomic can contain versions of functions specific to armv6 or
> armv7 with selection between them being made at runtime via ifunc.
> 
> In order to build the armv7 versions of these few functions, the
> libatomic Makefiles selectively add -march=armv7-a to CFLAGS. This
> isn't new - it goes back to at least 2012 in gcc git history.
> 
>    https://gcc.gnu.org/ml/gcc/2014-01/msg00141.html
> 
> What _is_ new is that for ARM, support for ifunc function attributes
> was not enabled prior to gcc8. ie when building with gcc7, the
> libatomic configure script determines that the toolchain doesn't
> support ifunc and libatomic therefore builds without support for
> runtime function selection... since it never needs to compile armv7
> specific versions of the runtime selectable functions the -march -vs-
> -mcpu conflict never happens.
> 
>    https://gcc.gnu.org/ml/gcc-patches/2017-10/msg00521.html
> 
> (Note that ifunc support for ARM in gcc8 is still only enabled for
> glibc, so this issue doesn't show up at all with musl).
> 
> Various solutions are possible:
> 
> 1) Let libatomic continue to build with ifunc support enabled, but
> avoid -march -vs- mcpu conflicts by dropping -mcpu from OE's CFLAGS in
> the gcc-runtime recipe.

I don't think this is required. Even though it might be useful for other 
reasons.

> 
> 2) Let libatomic continue to build with ifunc support enabled, but
> avoid -march -vs- -mcpu conflicts by updating the libatomic Makefiles
> so that they always safely over-ride any combination of -march, -mcpu,
> etc passed in from the build environment. ie patch the libatomic
> Makefiles to replace:
> 
>    IFUNC_OPTIONS = -march=armv7-a+fp -DHAVE_KERNEL64
> 
> with:
> 
>    IFUNC_OPTIONS = -march=armv7-a+fp -mcpu=generic-armv7-a -DHAVE_KERNEL64
> 
> (Regardless of the solution we pick for OE, I think that fix should be
> submitted upstream to libatomic. There's no need for libatomic to risk
> build errors by not defining -mcpu in cases where it specifically
> wants to target armv7a).

I think this is better. Now my memory serves me right I had a local 
patch few months ago which I discarded where I was using -march with 
armv7ve but it was limiting for other reasons. I think it would be 
better to drop -march completely.

IFUNC_OPTIONS = -mcpu=generic-armv7-a -DHAVE_KERNEL64

I will test it out locally and see if that works.

> 
> 3) Prevent libatomic from building with ifunc support enabled for ARM
> by forcing "libat_cv_have_ifunc=no" from the gcc-runtime recipe.
> 

This would be ok too for OE ifuncs dont serve much since we target a 
specific CPU anyway.

> Votes?
> 

approach 2 is what we should use.