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

[Andre McCurdy]

On Thu, Jun 7, 2018 at 5:57 PM, Khem Raj <raj.khem at gmail.com> wrote:
> On 6/7/18 4:38 PM, Andre McCurdy wrote:
>> On Thu, Jun 7, 2018 at 7:04 AM, Khem Raj <raj.khem at gmail.com> wrote:
>>> On Thu, Jun 7, 2018 at 12:14 AM, Andre McCurdy <armccurdy at gmail.com>
>>> wrote:
>>>> On Wed, Jun 6, 2018 at 10:58 PM, Khem Raj <raj.khem at 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 at 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.

If you only set -mcpu then you're likely to run into issues when
-march is set externally to something incompatible.

>> 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.

Right. This is the one I'd vote for. For ARM there's little point
including runtime alternatives when we always compile everything in
rootfs for one particular target architecture level anyway.