From: "Richard Earnshaw (lists)" <Richard.Earnshaw@arm.com>
To: Christophe Lyon <christophe.lyon@linaro.org>
Cc: "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>,
joseph@codesourcery.com
Subject: Re: [PATCH 00/30] [ARM] Reworking the -mcpu, -march and -mfpu options
Date: Mon, 12 Jun 2017 14:34:00 -0000 [thread overview]
Message-ID: <72966d49-38bf-c007-06d0-592af94d9009@arm.com> (raw)
In-Reply-To: <CAKdteOYnSaOQdY529BaJ24RF1jVWZPb8UMnXD0hEs0kyg0AXQw@mail.gmail.com>
On 12/06/17 12:49, Christophe Lyon wrote:
> On 10 June 2017 at 01:27, Richard Earnshaw (lists)
> <Richard.Earnshaw@arm.com> wrote:
>> On 09/06/17 23:45, Christophe Lyon wrote:
>>> Hi Richard,
>>>
>>>
>>> On 9 June 2017 at 14:53, Richard Earnshaw <Richard.Earnshaw@arm.com> 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.
>>>>
>>>> R.
>>>>
>>>> Richard Earnshaw (30):
>>>> [arm] Use strings for -march, -mcpu and -mtune options
>>>> [arm] Rewrite -march and -mcpu options for passing to the assembler
>>>> [arm] Don't pass -mfpu=auto through to the assembler.
>>>> [arm] Allow +opt on arbitrary cpu and architecture specifications
>>>> [arm] Add architectural options
>>>> [arm] Add default FPUs for CPUs.
>>>> [build] Make sbitmap code available to the driver programs
>>>> [arm] Split CPU, architecture and tuning data tables.
>>>> [ARM] Move cpu and architecture option name parsing code to
>>>> arm-common.c
>>>> [arm] Use standard option parsing code for detecting thumb-only
>>>> targets
>>>> [arm] Allow CPU and architecture extensions to be defined as aliases
>>>> [arm] Allow new extended syntax CPU and architecture names during
>>>> configure
>>>> [arm] Force a CPU default in the config args defaults list.
>>>> [arm] Generate a canonical form for -march
>>>> [arm] Make -mfloat-abi=softfp work when there are no FPU instructions
>>>> [arm] Update basic multilib configuration
>>>> [arm] Make 'auto' the default FPU selection option.
>>>> [arm] Rewrite t-aprofile using new selector methodology
>>>> [arm] Explicitly set .fpu in cmse_nonsecure_call.S
>>>> [genmultilib] Allow explicit periods to be escaped in MULTILIB_REUSE
>>>> [arm][testsuite] Use -march=armv7-a+fp when testing hard-float ABI.
>>>> [arm] Rewrite t-rmprofile multilib specification
>>>> [arm][rtems] Update t-rtems for new option framework
>>>> [arm][linux-eabi] Ensure all multilib variables are reset
>>>> [arm][phoenix] reset all multilib variables
>>>> [arm] Rework multlib builds for symbianelf
>>>> [arm][fuchsia] Rework multilib support
>>>> [arm] Add a few missing architecture extension options.
>>>> [arm][doc] Document new -march= syntax.
>>>> [arm][doc] Document changes to -mcpu, -mtune and -mfpu.
>>>>
>>>> gcc/Makefile.in | 2 +-
>>>> gcc/common/config/arm/arm-common.c | 651 +++++++-
>>>> gcc/config.gcc | 17 +-
>>>> gcc/config/arm/arm-builtins.c | 4 +-
>>>> gcc/config/arm/arm-cpu-cdata.h | 2444 +++++++++++++++++++++++------
>>>> gcc/config/arm/arm-cpu-data.h | 1410 ++---------------
>>>> gcc/config/arm/arm-cpu.h | 38 +
>>>> gcc/config/arm/arm-cpus.in | 237 ++-
>>>> gcc/config/arm/arm-isa.h | 20 +-
>>>> gcc/config/arm/arm-protos.h | 56 +-
>>>> gcc/config/arm/arm-tables.opt | 21 +-
>>>> gcc/config/arm/arm.c | 337 ++--
>>>> gcc/config/arm/arm.h | 75 +-
>>>> gcc/config/arm/arm.opt | 15 +-
>>>> gcc/config/arm/bpabi.h | 4 -
>>>> gcc/config/arm/elf.h | 6 +-
>>>> gcc/config/arm/linux-elf.h | 3 -
>>>> gcc/config/arm/netbsd-elf.h | 4 -
>>>> gcc/config/arm/parsecpu.awk | 295 +++-
>>>> gcc/config/arm/t-aprofile | 200 +--
>>>> gcc/config/arm/t-arm-elf | 173 +-
>>>> gcc/config/arm/t-fuchsia | 33 +
>>>> gcc/config/arm/t-linux-eabi | 4 +
>>>> gcc/config/arm/t-multilib | 126 +-
>>>> gcc/config/arm/t-phoenix | 20 +-
>>>> gcc/config/arm/t-rmprofile | 147 +-
>>>> gcc/config/arm/t-rtems | 49 +-
>>>> gcc/config/arm/t-symbian | 34 +-
>>>> gcc/config/arm/vxworks.h | 2 -
>>>> gcc/doc/fragments.texi | 10 +-
>>>> gcc/doc/invoke.texi | 371 ++++-
>>>> gcc/genmultilib | 4 +-
>>>> gcc/testsuite/gcc.dg/pr59418.c | 2 +-
>>>> gcc/testsuite/gcc.target/arm/multilib.exp | 685 ++++++++
>>>> gcc/testsuite/gcc.target/arm/pr51915.c | 2 +-
>>>> gcc/testsuite/gcc.target/arm/pr52006.c | 2 +-
>>>> gcc/testsuite/gcc.target/arm/pr53187.c | 2 +-
>>>> libgcc/config/arm/cmse_nonsecure_call.S | 8 +
>>>> 38 files changed, 5073 insertions(+), 2440 deletions(-)
>>>> create mode 100644 gcc/config/arm/t-fuchsia
>>>> create mode 100644 gcc/testsuite/gcc.target/arm/multilib.exp
>>>>
>>>> ----------------2.7.4--
>>>>
>>>
>>> I wanted to run a validation with the full series applied as one patch
>>> over r249050,
>>> so I just downloaded all the patches, concatenated them in order, but the result
>>> fails to apply. (conflicts with arm-cpu-cdata.h, arm-cpus.in,
>>> t-aprofile, t-rmprofile)
>>>
>>> Am I missing something?
>>>
>>> Thanks,
>>>
>>> Christophe
>>>
>>
>> I really appreciate you trying to do this...
>>
>> I rebased the patch series sometime on Tuesday/Wednesday just before Jim
>> Wilson committed his falkor change; you'll need to back out to just
>> before r248944. I can't think of anything else that might have just
>> gone in that might conflict. For arm-cpu-cdata.h (and the other
>> auto-generated files, like arm-cpu-data.h) you can just delete the file
>> and it will be rebuilt automatically, the others really do need the
>> conflict to be resolved.
>>
>> Obviously I'll rebase again just before the commit, but propagating such
>> changes through the patch series is quite tedious and I don't want to do
>> it any more than necessary :-).
>>
>> It would be easier if the generated files weren't checked in to the
>> repository...
>>
>> R.
>
> Hi Richard,
>
> Starting a validation of the whole patch against r248942 did work, thanks.
>
> The results are here:
> http://people.linaro.org/~christophe.lyon/cross-validation/gcc-test-patches/248942-rework-cpu-arch-fpu.patch/report-build-info.html
>
> Regressions are detected in many configurations, unfortunately.
> Some may be caused by the fact that I've upgraded to dejagnu-1.6+ for
> my testing,
> and thus "multilib flags" are now prepended rather than appended, but
> I don't think that's the majority.
>
> To help you understand/group all the reports:
> - all "arm-none-linux-gnueabi" with REGRESSED now have:
> FAIL: gcc.target/arm/neon-thumb2-move.c (test for excess errors)
See later.
> FAIL: gcc.target/arm/no-volatile-in-it.c scan-assembler-not ldrgt
> - same for the 2 configs arm-none-eabi --with-cpu=cortex-a9
I'm checking a fix for this, I think I understand what's going on here.
>
> - all "arm-none-linux-gnueabihf" and "armeb-none-linux-gnueabihf" with
> REGRESSED now have:
> FAIL: gcc.target/arm/no-volatile-in-it.c scan-assembler-not ldrgt
>
> - arm-none-eabi --with-cpu=cortex-m3 now has:
> FAIL:
> gcc.target/arm/no-volatile-in-it.c scan-assembler-not ldrgt
> gcc.target/arm/thumb2-slow-flash-data-2.c (test for excess errors)
> gcc.target/arm/thumb2-slow-flash-data-3.c (test for excess errors)
> gcc.target/arm/thumb2-slow-flash-data-4.c (test for excess errors)
> gcc.target/arm/thumb2-slow-flash-data-5.c (test for excess errors)
> UNRESOLVED:
> gcc.target/arm/thumb2-slow-flash-data-4.c scan-assembler-times #1\\.0e\\+0 3
> gcc.target/arm/thumb2-slow-flash-data-5.c scan-assembler-not #1\\.0e\\+0
>
> - The 2 cells with "BIG-REG" mean regressions where detected, but the report
> is >100kb so it was attached as a .xz file, click on 'BIG-REG" to download it.
>
> - the cells with "BETTER" can be questionable: it means no new failure
> appeared, but PASS -> UNSUPPORTED is considered as "better". Here
> we have cases with several thousands (!) of tests becoming unsupported,
> which looks a bit suspicious.
>
> Among other things, I've noticed that when passing -march=armv5t,
> arm_neon_ok fails:
> xgcc -march=armv5t -fno-diagnostics-show-caret
> -fdiagnostics-color=never -mfpu=neon -mfloat-abi=softfp -march=
> armv7-a -c -o arm_neon_ok15574.o arm_neon_ok15574.c
> arm_neon_ok15574.c:9:4: error: #error Architecture does not support NEON.
> compiler exited with status 1
>
> I'd expect -march=armv7-a to allow to use -mfpu=neon?
>
It does. The problem seems to be a generic one in the driver in that
the rewrite rules are always passed the first instance of -march and not
the last. Indeed, with the aarch64 compiler, if I write
gcc -mcpu=native -mcpu=cortex-a53
then the driver will rewrite this as
./cc1 -mcpu=cortex-a53 -mcpu=<expansion of native cpu name>
which doesn't seem to be the right thing at all.
So we either have a generic problem with all option rewriting, or there
are some subtle details of it that we've not figured out yet.
Joseph, is there a way to get the rewrite rules to receive the *last*
instance of a flag rather than the first? Or is the current behaviour
simply wrong?
R.
> Regarding the new multilib.exp tests introduced by the patch, my testing
> did not run it because I have no configuration setting aprofile or rmprofile.
> Maybe it's time I add one, or update the existing ones.
> I didn't see any message "skipping multilib tests due to
> multilib_flags setting",
> but I guess that's because my runs are not verbose enough. That being said,
> I'm not sure how the multilib_flags are set in board_info? Is that derived from
> RUNTESTFLAGS?
>
> From this report page, you can also click on "sum" and "log" (when
> present) to download the gcc.sum or gcc.log files.
>
> Feel free to ask more details if I haven't been clear enough.
>
> Thanks,
>
> Christophe
>
next prev parent reply other threads:[~2017-06-12 14:34 UTC|newest]
Thread overview: 62+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-09 12:54 Richard Earnshaw
2017-06-09 12:54 ` [PATCH 25/30] [arm][phoenix] reset all multilib variables Richard Earnshaw
2017-06-09 12:54 ` [PATCH 24/30] [arm][linux-eabi] Ensure all multilib variables are reset Richard Earnshaw
2017-06-09 12:54 ` [PATCH 02/30] [arm] Rewrite -march and -mcpu options for passing to the assembler Richard Earnshaw
2017-06-09 12:54 ` [PATCH 12/30] [arm] Allow new extended syntax CPU and architecture names during configure Richard Earnshaw
2017-06-09 12:54 ` [PATCH 29/30] [arm][doc] Document new -march= syntax Richard Earnshaw
2017-06-09 12:54 ` [PATCH 10/30] [arm] Use standard option parsing code for detecting thumb-only targets Richard Earnshaw
2017-06-09 12:54 ` [PATCH 05/30] [arm] Add architectural options Richard Earnshaw
2017-06-09 12:54 ` [PATCH 06/30] [arm] Add default FPUs for CPUs Richard Earnshaw
2017-06-09 12:54 ` [PATCH 23/30] [arm][rtems] Update t-rtems for new option framework Richard Earnshaw
2017-06-09 13:03 ` Sebastian Huber
2017-06-09 12:54 ` [PATCH 28/30] [arm] Add a few missing architecture extension options Richard Earnshaw
2017-06-09 12:54 ` [PATCH 18/30] [arm] Rewrite t-aprofile using new selector methodology Richard Earnshaw
2017-06-09 12:54 ` [PATCH 17/30] [arm] Make 'auto' the default FPU selection option Richard Earnshaw
2017-06-09 12:54 ` [PATCH 04/30] [arm] Allow +opt on arbitrary cpu and architecture specifications Richard Earnshaw
2017-06-13 17:14 ` Richard Earnshaw (lists)
2017-06-09 12:55 ` [PATCH 20/30] [genmultilib] Allow explicit periods to be escaped in MULTILIB_REUSE Richard Earnshaw
2017-06-09 12:55 ` [PATCH 30/30] [arm][doc] Document changes to -mcpu, -mtune and -mfpu Richard Earnshaw
2017-06-09 22:19 ` Gerald Pfeifer
2017-06-09 12:55 ` [PATCH 19/30] [arm] Explicitly set .fpu in cmse_nonsecure_call.S Richard Earnshaw
2017-06-09 12:55 ` [PATCH 03/30] [arm] Don't pass -mfpu=auto through to the assembler Richard Earnshaw
2017-06-09 12:55 ` [PATCH 15/30] [arm] Make -mfloat-abi=softfp work when there are no FPU instructions Richard Earnshaw
2017-06-09 12:55 ` [PATCH 26/30] [arm] Rework multlib builds for symbianelf Richard Earnshaw
2017-06-09 12:55 ` [PATCH 21/30] [arm][testsuite] Use -march=armv7-a+fp when testing hard-float ABI Richard Earnshaw
2017-06-09 12:55 ` [PATCH 14/30] [arm] Generate a canonical form for -march Richard Earnshaw
2017-06-13 17:25 ` Richard Earnshaw (lists)
2017-06-09 12:55 ` [PATCH 11/30] [arm] Allow CPU and architecture extensions to be defined as aliases Richard Earnshaw
2017-06-09 12:55 ` [PATCH 01/30] [arm] Use strings for -march, -mcpu and -mtune options Richard Earnshaw
2017-06-13 13:23 ` Christophe Lyon
2017-06-13 15:33 ` Richard Earnshaw (lists)
2017-06-13 17:11 ` Richard Earnshaw (lists)
2017-06-09 12:55 ` [PATCH 07/30] [build] Make sbitmap code available to the driver programs Richard Earnshaw
2017-06-14 14:35 ` Richard Earnshaw (lists)
2017-06-16 8:03 ` Richard Biener
2017-06-09 12:55 ` [PATCH 22/30] [arm] Rewrite t-rmprofile multilib specification Richard Earnshaw
2017-06-09 12:55 ` [PATCH 16/30] [arm] Update basic multilib configuration Richard Earnshaw
2017-06-09 12:55 ` [PATCH 09/30] [ARM] Move cpu and architecture option name parsing code to arm-common.c Richard Earnshaw
2017-06-13 17:19 ` Richard Earnshaw (lists)
2017-06-09 12:55 ` [PATCH 08/30] [arm] Split CPU, architecture and tuning data tables Richard Earnshaw
2017-06-13 17:17 ` Richard Earnshaw (lists)
2017-06-09 12:55 ` [PATCH 13/30] [arm] Force a CPU default in the config args defaults list Richard Earnshaw
2017-06-09 12:55 ` [PATCH 27/30] [arm][fuchsia] Rework multilib support Richard Earnshaw
2017-06-09 22:45 ` [PATCH 00/30] [ARM] Reworking the -mcpu, -march and -mfpu options Christophe Lyon
2017-06-09 23:27 ` Richard Earnshaw (lists)
2017-06-12 11:49 ` Christophe Lyon
2017-06-12 14:34 ` Richard Earnshaw (lists) [this message]
2017-06-12 17:11 ` Joseph Myers
2017-06-12 21:27 ` Richard Earnshaw (lists)
2017-06-13 9:40 ` Richard Earnshaw (lists)
2017-06-13 10:29 ` Joseph Myers
2017-06-13 15:26 ` Richard Earnshaw (lists)
2017-06-13 16:08 ` Christophe Lyon
2017-06-12 11:48 ` Nathan Sidwell
2017-06-13 17:27 ` [PATCH 31/30] [arm] Mark -marm and -mthumb as being inverse options Richard Earnshaw (lists)
2017-06-13 17:29 ` [PATCH 32/30][arm][testsuite] Fix neon-thumb2-move.c test Richard Earnshaw (lists)
2017-06-14 14:27 ` [PATCH 32/30][arm][testsuite] Fix various tests Richard Earnshaw (lists)
2017-06-14 19:26 ` Christophe Lyon
2017-06-13 17:36 ` [PATCH 00/30] [ARM] Reworking the -mcpu, -march and -mfpu options Richard Earnshaw (lists)
2017-06-14 9:08 ` Christophe Lyon
2017-06-14 10:21 ` Richard Earnshaw (lists)
2017-06-16 21:11 ` Richard Earnshaw
2017-06-16 21:16 ` Ramana Radhakrishnan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=72966d49-38bf-c007-06d0-592af94d9009@arm.com \
--to=richard.earnshaw@arm.com \
--cc=christophe.lyon@linaro.org \
--cc=gcc-patches@gcc.gnu.org \
--cc=joseph@codesourcery.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).