[Bug target/88530] [8 Regression] AArch64 Unsupported options passed to assemblers when it doesn't need to.

tnfchris at gcc dot gnu.org gcc-bugzilla@gcc.gnu.org
Wed Mar 6 09:49:00 GMT 2019


https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88530

--- Comment #9 from Tamar Christina <tnfchris at gcc dot gnu.org> ---
Author: tnfchris
Date: Wed Mar  6 09:49:00 2019
New Revision: 269413

URL: https://gcc.gnu.org/viewcvs?rev=269413&root=gcc&view=rev
Log:
AArch64: Fix command line options canonicalization version.

Commandline options on AArch64 don't get canonicalized into the smallest
possible set before output to the assembler. This means that overlapping
feature
sets are emitted with superfluous parts.

Normally this isn't an issue, but in the case of crypto we have retro-actively
split it into aes and sha2. We need to emit only +crypto to the assembler
so old assemblers continue to work.

Because of how -mcpu=native and -march=native work they end up enabling all
feature bits. Instead we need to get the smallest possible set, which would
also
fix the problem with older the assemblers and the retro-active split.

The function that handles this is called quite often.  It is called for any
push/pop options or attribute that changes arch, cpu etc.  In order to not make
this search for the smallest set too expensive we sort the options based on the
number of features (bits) they enable.  This allows us to process the list
linearly instead of quadratically (Once we have enabled a feature, we know that
anything else that enables it can be ignored.  By sorting we'll get the biggest
groups first and thus the smallest combination of commandline flags).

The Option handling structures have been extended to have a boolean to indicate
whether the option is synthetic, with that I mean if the option flag itself
enables a new feature.

e.g. +crypto isn't an actual feature, it just enables other features, but
others
like +rdma enable multiple dependent features but is itself also a feature.

There are two ways to solve this.

1) Either have the options that are feature bits also turn themselves on, e.g.
   change rdma to turn on FP, SIMD and RDMA as dependency bits.

2) Make a distinction between these two different type of features and have the
   framework handle it correctly.

Even though it's more code I went for the second approach, as it's the one
that'll be less fragile (people can't forget it) and gives the least surprises.

Effectively this patch changes the following:

The values before the => are the old compiler and after the => the new code.

-march=armv8.2-a+crypto+sha2 => -march=armv8.2-a+crypto
-march=armv8.2-a+sha2+aes => -march=armv8.2-a+crypto

The remaining behaviors stay the same.

gcc/ChangeLog:

        Backport from trunk.
        2019-02-25  Tamar Christina  <tamar.christina@arm.com>

        PR target/88530
        * common/config/aarch64/aarch64-common.c
        (struct aarch64_option_extension): Add is_synthetic.
        (all_extensions): Use it.
        (TARGET_OPTION_INIT_STRUCT): Define hook.
        (struct gcc_targetm_common): Moved to end.
        (all_extensions_by_on): New.
        (opt_ext_cmp, typedef opt_ext): New.
        (aarch64_option_init_struct): New.
        (aarch64_contains_opt): New.
        (aarch64_get_extension_string_for_isa_flags): Output smallest set.
        * config/aarch64/aarch64-option-extensions.def
        (AARCH64_OPT_EXTENSION): Explicitly include AES and SHA2 in crypto.
        (fp, simd, crc, lse, fp16, rcpc, rdma, dotprod, aes, sha2, sha3,
        sm4, fp16fml, sve):
        Set is_synthetic to false.
        (crypto): Set is_synthetic to true.
        * config/aarch64/driver-aarch64.c (AARCH64_OPT_EXTENSION): Add
        SYNTHETIC.

gcc/testsuite/ChangeLog:

        Backport from trunk.
        2019-02-25  Tamar Christina  <tamar.christina@arm.com>

        PR target/88530
        * gcc.target/aarch64/options_set_1.c: New test.
        * gcc.target/aarch64/options_set_2.c: New test.
        * gcc.target/aarch64/options_set_3.c: New test.
        * gcc.target/aarch64/options_set_4.c: New test.
        * gcc.target/aarch64/options_set_5.c: New test.
        * gcc.target/aarch64/options_set_6.c: New test.
        * gcc.target/aarch64/options_set_7.c: New test.
        * gcc.target/aarch64/options_set_8.c: New test.
        * gcc.target/aarch64/options_set_9.c: New test.


Added:
    branches/gcc-8-branch/gcc/testsuite/gcc.target/aarch64/options_set_1.c
    branches/gcc-8-branch/gcc/testsuite/gcc.target/aarch64/options_set_2.c
    branches/gcc-8-branch/gcc/testsuite/gcc.target/aarch64/options_set_3.c
    branches/gcc-8-branch/gcc/testsuite/gcc.target/aarch64/options_set_4.c
    branches/gcc-8-branch/gcc/testsuite/gcc.target/aarch64/options_set_5.c
    branches/gcc-8-branch/gcc/testsuite/gcc.target/aarch64/options_set_6.c
    branches/gcc-8-branch/gcc/testsuite/gcc.target/aarch64/options_set_7.c
    branches/gcc-8-branch/gcc/testsuite/gcc.target/aarch64/options_set_8.c
    branches/gcc-8-branch/gcc/testsuite/gcc.target/aarch64/options_set_9.c
Modified:
    branches/gcc-8-branch/gcc/ChangeLog
    branches/gcc-8-branch/gcc/common/config/aarch64/aarch64-common.c
    branches/gcc-8-branch/gcc/config/aarch64/aarch64-option-extensions.def
    branches/gcc-8-branch/gcc/config/aarch64/driver-aarch64.c
    branches/gcc-8-branch/gcc/testsuite/ChangeLog


More information about the Gcc-bugs mailing list