| Message ID | c33e49d6-ccca-93f7-014c-e712a21d283b@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | arm: Ignore --with-mode when CPU only supports one instruction set. | expand |
On Wed, 3 Mar 2021 at 14:55, Richard Earnshaw (lists) <Richard.Earnshaw@arm.com> wrote: > > Hopefully this change will reduce the number of times Christophe is > needing to tweak the testsuite. > Thanks! I guess this means we can now do some cleanup in the testsuite? Did you have a quick look at the amount of tests involved? Christophe > -------------- > > Arm processors can support up to two instruction sets. Some early > cores only support the traditional A32 (Arm) instructions, while some > more recent devices only support T32 (Thumb) instructions. > > When configuring the compiler, --with-mode can be used to select the > default instruction set to target if the user has not made an explicit > choice, but this can cause needless problems if the default is not > supported by the requested CPU. > > To fix this this patch adjusts the way that the --with-mode selection > is processed so that it can take into account the selected CPU or > architecture and not create a meaningless combination. > > gcc: > * common/config/arm/arm-common.c: Include configargs.h. > (arm_config_default): New function. > (arm_target_mode): Renamed from arm_target_thumb_only. Handle > processors that do not support Thumb. Take into account the > --with-mode configuration setting for selecting the default. > * config/arm/arm.h (OPTION_DEFAULT_SPECS): Remove entry for 'mode'. > (TARGET_MODE_SPEC_FUNCTIONS): Update for function name change. > --- > gcc/common/config/arm/arm-common.c | 49 ++++++++++++++++++++++++++---- > gcc/config/arm/arm.h | 10 +++--- > 2 files changed, 49 insertions(+), 10 deletions(-) > >
On 03/03/2021 14:11, Christophe Lyon via Gcc-patches wrote: > On Wed, 3 Mar 2021 at 14:55, Richard Earnshaw (lists) > <Richard.Earnshaw@arm.com> wrote: >> >> Hopefully this change will reduce the number of times Christophe is >> needing to tweak the testsuite. >> > > Thanks! > > I guess this means we can now do some cleanup in the testsuite? > Did you have a quick look at the amount of tests involved? > No, I wasn't expecting to change the existing tests again where you've already done this. But hopefully you won't need to do any more changes for this reason in future. R. > Christophe > >> -------------- >> >> Arm processors can support up to two instruction sets. Some early >> cores only support the traditional A32 (Arm) instructions, while some >> more recent devices only support T32 (Thumb) instructions. >> >> When configuring the compiler, --with-mode can be used to select the >> default instruction set to target if the user has not made an explicit >> choice, but this can cause needless problems if the default is not >> supported by the requested CPU. >> >> To fix this this patch adjusts the way that the --with-mode selection >> is processed so that it can take into account the selected CPU or >> architecture and not create a meaningless combination. >> >> gcc: >> * common/config/arm/arm-common.c: Include configargs.h. >> (arm_config_default): New function. >> (arm_target_mode): Renamed from arm_target_thumb_only. Handle >> processors that do not support Thumb. Take into account the >> --with-mode configuration setting for selecting the default. >> * config/arm/arm.h (OPTION_DEFAULT_SPECS): Remove entry for 'mode'. >> (TARGET_MODE_SPEC_FUNCTIONS): Update for function name change. >> --- >> gcc/common/config/arm/arm-common.c | 49 ++++++++++++++++++++++++++---- >> gcc/config/arm/arm.h | 10 +++--- >> 2 files changed, 49 insertions(+), 10 deletions(-) >> >>
On Wed, 3 Mar 2021 at 15:13, Richard Earnshaw (lists) <Richard.Earnshaw@arm.com> wrote: > > On 03/03/2021 14:11, Christophe Lyon via Gcc-patches wrote: > > On Wed, 3 Mar 2021 at 14:55, Richard Earnshaw (lists) > > <Richard.Earnshaw@arm.com> wrote: > >> > >> Hopefully this change will reduce the number of times Christophe is > >> needing to tweak the testsuite. > >> > > > > Thanks! > > > > I guess this means we can now do some cleanup in the testsuite? > > Did you have a quick look at the amount of tests involved? > > > > No, I wasn't expecting to change the existing tests again where you've > already done this. But hopefully you won't need to do any more changes > for this reason in future. > OK thanks! > R. > > > Christophe > > > >> -------------- > >> > >> Arm processors can support up to two instruction sets. Some early > >> cores only support the traditional A32 (Arm) instructions, while some > >> more recent devices only support T32 (Thumb) instructions. > >> > >> When configuring the compiler, --with-mode can be used to select the > >> default instruction set to target if the user has not made an explicit > >> choice, but this can cause needless problems if the default is not > >> supported by the requested CPU. > >> > >> To fix this this patch adjusts the way that the --with-mode selection > >> is processed so that it can take into account the selected CPU or > >> architecture and not create a meaningless combination. > >> > >> gcc: > >> * common/config/arm/arm-common.c: Include configargs.h. > >> (arm_config_default): New function. > >> (arm_target_mode): Renamed from arm_target_thumb_only. Handle > >> processors that do not support Thumb. Take into account the > >> --with-mode configuration setting for selecting the default. > >> * config/arm/arm.h (OPTION_DEFAULT_SPECS): Remove entry for 'mode'. > >> (TARGET_MODE_SPEC_FUNCTIONS): Update for function name change. > >> --- > >> gcc/common/config/arm/arm-common.c | 49 ++++++++++++++++++++++++++---- > >> gcc/config/arm/arm.h | 10 +++--- > >> 2 files changed, 49 insertions(+), 10 deletions(-) > >> > >> >
diff --git a/gcc/common/config/arm/arm-common.c b/gcc/common/config/arm/arm-common.c index 98824517c7b..5b03b86724d 100644 --- a/gcc/common/config/arm/arm-common.c +++ b/gcc/common/config/arm/arm-common.c @@ -33,6 +33,8 @@ #include "sbitmap.h" #include "diagnostic.h" +#include "configargs.h" + /* Set default optimization options. */ static const struct default_options arm_option_optimization_table[] = { @@ -240,16 +242,34 @@ check_isa_bits_for (const enum isa_feature* bits, enum isa_feature bit) return false; } +/* Look up NAME in the configuration defaults for this build of the + the compiler. Return the value associated with that name, or NULL + if no value is found. */ +static const char * +arm_config_default (const char *name) +{ + int i; + + if (configure_default_options[0].name == NULL) + return NULL; + + for (i = 0; i < ARRAY_SIZE (configure_default_options); i++) + if (strcmp (configure_default_options[i].name, name) == 0) + return configure_default_options[i].value; + + return NULL; +} + /* Called by the driver to check whether the target denoted by current - command line options is a Thumb-only target. ARGV is an array of - tupples (normally only one) where the first element of the tupple - is 'cpu' or 'arch' and the second is the option passed to the - compiler for that. An architecture tupple is always taken in - preference to a cpu tupple and the last of each type always + command line options is a Thumb-only, or ARM-only, target. ARGV is + an array of tupples (normally only one) where the first element of + the tupple is 'cpu' or 'arch' and the second is the option passed + to the compiler for that. An architecture tupple is always taken + in preference to a cpu tupple and the last of each type always overrides any earlier setting. */ const char * -arm_target_thumb_only (int argc, const char **argv) +arm_target_mode (int argc, const char **argv) { const char *arch = NULL; const char *cpu = NULL; @@ -285,6 +305,9 @@ arm_target_thumb_only (int argc, const char **argv) if (arch_opt && !check_isa_bits_for (arch_opt->common.isa_bits, isa_bit_notm)) return "-mthumb"; + if (arch_opt && !check_isa_bits_for (arch_opt->common.isa_bits, + isa_bit_thumb)) + return "-marm"; } else if (cpu) { @@ -294,6 +317,20 @@ arm_target_thumb_only (int argc, const char **argv) if (cpu_opt && !check_isa_bits_for (cpu_opt->common.isa_bits, isa_bit_notm)) return "-mthumb"; + if (cpu_opt && !check_isa_bits_for (cpu_opt->common.isa_bits, + isa_bit_thumb)) + return "-marm"; + } + + const char *default_mode = arm_config_default ("mode"); + if (default_mode) + { + if (strcmp (default_mode, "thumb") == 0) + return "-mthumb"; + else if (strcmp (default_mode, "arm") == 0) + return "-marm"; + else + gcc_unreachable (); } /* Compiler hasn't been configured with a default, and the CPU diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h index 6bc03ada0bf..113c015c455 100644 --- a/gcc/config/arm/arm.h +++ b/gcc/config/arm/arm.h @@ -390,7 +390,10 @@ emission of floating point pcs attributes. */ --with-float is ignored if -mfloat-abi is specified. --with-fpu is ignored if -mfpu is specified. --with-abi is ignored if -mabi is specified. - --with-tls is ignored if -mtls-dialect is specified. */ + --with-tls is ignored if -mtls-dialect is specified. + Note: --with-mode is not handled here, that has a special rule + TARGET_MODE_CHECK that also takes into account the selected CPU and + architecture. */ #define OPTION_DEFAULT_SPECS \ {"arch", "%{!march=*:%{!mcpu=*:-march=%(VALUE)}}" }, \ {"cpu", "%{!march=*:%{!mcpu=*:-mcpu=%(VALUE)}}" }, \ @@ -398,7 +401,6 @@ emission of floating point pcs attributes. */ {"float", "%{!mfloat-abi=*:-mfloat-abi=%(VALUE)}" }, \ {"fpu", "%{!mfpu=*:-mfpu=%(VALUE)}"}, \ {"abi", "%{!mabi=*:-mabi=%(VALUE)}"}, \ - {"mode", "%{!marm:%{!mthumb:-m%(VALUE)}}"}, \ {"tls", "%{!mtls-dialect=*:-mtls-dialect=%(VALUE)}"}, extern const struct arm_fpu_desc @@ -2424,9 +2426,9 @@ extern const char *arm_asm_auto_mfpu (int argc, const char **argv); " mcpu=*:-mcpu=%:rewrite_mcpu(%{mcpu=*:%*})" \ " }" -extern const char *arm_target_thumb_only (int argc, const char **argv); +extern const char *arm_target_mode (int argc, const char **argv); #define TARGET_MODE_SPEC_FUNCTIONS \ - { "target_mode_check", arm_target_thumb_only }, + { "target_mode_check", arm_target_mode }, /* -mcpu=native handling only makes sense with compiler running on an ARM chip. */