Re: [PATCH 00/30] [ARM] Reworking the -mcpu, -march and -mfpu options

["Richard Earnshaw (lists)" <Richard.Earnshaw@arm.com>]

On 14/06/17 10:08, Christophe Lyon wrote:
> On 13 June 2017 at 19:35, Richard Earnshaw (lists)
> <Richard.Earnshaw@arm.com> wrote:
>> On 09/06/17 13:53, Richard Earnshaw wrote:
>>>
>>> During the ARM BoF at the Cauldron last year I mentioned that I wanted
>>> to rework the way GCC on ARM handles the command line options.  The
>>> problem was that most users, and even many experts, can't remember
>>> which FPU/SIMD unit comes with which CPU and that consequently many
>>> users were inadvertenly generating sub-optimal code for their system.
>>>
>>> This patch series implements the proposed change and provides support
>>> for a generic way of adding optional features to architectures and CPU
>>> names.  The documentation patches at the end of the series explain the
>>> new syntax, so I won't repeat all that here.  Suffice to say here that
>>> the result is that the -mfpu option now defaults to 'auto', which
>>> allows the compiler to infer the floating-point and simd options from
>>> the CPU/architecture options and that these options can normally be
>>> expressed in a context-specific manner like +simd or +fp without
>>> having to know precisely which variant is implemented.  Long term I'd
>>> like to deprecate -mfpu and entirely move over to the new syntax; but
>>> it's too early to start that process now.
>>>
>>> All the patches in the series should build a working basic compiler,
>>> but the multilib selection will not work correctly until the relevant
>>> patches towards the end are applied.  It is not really feasible to
>>> retain that functionality without collapsing too many of the patches
>>> together into one hunk.  It's also possible that some tests in the
>>> testsuite may exhibit transient misbehaviour, but there should be no
>>> regressions by the end of the sequence (some tests no-longer run in
>>> the default configurations because the default CPU does not have
>>> floating-point support).
>>>
>>> Just two patches are to the generic code, but both are fairly trivial.
>>> One permits the sbitmap code to be used in the driver programs and the
>>> other provides a way of escaping the meta-character in some multilib
>>> reuse strings.
>>>
>>> I won't apply any of this series until those two patches have been
>>> approved, and I won't commit anything before the middle of next week
>>> even then.  This is a fairly complex change and it deserves some time
>>> for people to comment before committing.
>>>
>>
>> The attached is a roll-up of the entire series after the updates I've
>> posted today.  This should eliminate any potential issues with applying
>> the series for the purposes of testing.  Please don't post issues in
>> reply to this, but to the individual patches in the series.
>>
> 
> Hi Richard,
> 
> Thanks for the quick iteration, and sorry for responding here ;-)
> The validation results of the whole roll-up patch look much better:
> http://people.linaro.org/~christophe.lyon/cross-validation/gcc-test-patches/248942-roll-up.patch/report-build-info.html
> 

No problem, that request is aimed at code reviews, not at overall
comments on the test results.

> (you can ignore the "better" in the arm-none-linux-gnueabihf table,
> it's noise in the results).
> 
> The other 2 better are actually 100 "pass" becoming 26 "unsupported"
> because arm_fp_ok now returns false in the corresponding configs
> (there's no fpu for the selected processor/arch)

That's somewhat expected, and in some ways the 'right thing'.  The
selected target really doesn't have an FPU so it's correct that the
infrastructure deduces this.

Of course, we might want to adjust the tests so that they do try harder
to enable an FPU, but that can be done later on a case by case basis.

> 
> As of the regression --with-cpu=cortex-m3, the thumb2-slow-flash-data-[2345].c
> tests may need an update: they are compiled with
> -march=armv7e-m -mfloat-abi=hard -O2 -mthumb -mslow-flash-data
> resulting in:
> cc1: error: -mfloat-abi=hard: selected processor lacks an FPU
> 

Yes, those need updating to -march=armv7e-m+fp.  The tests are somewhat
bogus in that they are assuming that the internal 'vfp' fpu that was
previously configured was 'right' for this architecture.  That
assumption was bogus and it's this sort of nonsense that this patch set
is really trying to address.

Many thanks for the quick turn-around on the tests again.  It looks like
we're nearly there.

I'll update patch 32 to add these changes, but won't bother with another
roll-up this time.

R.


> Christophe
> 
> 
>> R.