[PATCH][ARM]Automatically add -mthumb for thumb-only target when mode isn't specified

[PATCH][ARM]Automatically add -mthumb for thumb-only target when mode isn't specified

[Terry Guo]

[-- Attachment #1: Type: text/plain, Size: 923 bytes --]

Hi there,

If target mode isn't specified via either gcc configuration option
--with-mode or command line, this patch intends to improve gcc driver to
automatically add option -mthumb for thumb-only target. Tested with gcc
regression test for various arm targets, no regression. Is it OK?

BR,
Terry

gcc/ChangeLog:

2015-03-02  Terry Guo  <terry.guo@arm.com>

        * common/config/arm/arm-common.c (arm_is_target_thumb_only): New
function.
        * config/arm/arm-protos.h (FL_ Macros): Move to ...
        * config/arm/arm-opts.h (FL_ Macros): ... here.
        (struct arm_arch_core_flag): New struct.
        (arm_arch_core_flags): New array for arch/core and flag map.
        * config/arm/arm.h (MODE_SET_SPEC_FUNCTIONS): Define new SPEC
function.
        (EXTRA_SPEC_FUNCTIONS): Include new SPEC function.
        (MODE_SET_SPECS): New SPEC.
        (DRIVER_SELF_SPECS): Include new SPEC.

[-- Attachment #2: gcc-mthumb-option-v5.txt --]
[-- Type: text/plain, Size: 11355 bytes --]

diff --git a/gcc/common/config/arm/arm-common.c b/gcc/common/config/arm/arm-common.c
index 86673b7..e17ee03 100644
--- a/gcc/common/config/arm/arm-common.c
+++ b/gcc/common/config/arm/arm-common.c
@@ -97,6 +97,28 @@ arm_rewrite_mcpu (int argc, const char **argv)
   return arm_rewrite_selected_cpu (argv[argc - 1]);
 }
 
+/* Called by driver to check whether the target denoted by current
+   command line options is thumb-only target.  If -march present,
+   check the last -march option.  If no -march, check the last -mcpu
+   option.  */
+const char *
+arm_is_target_thumb_only (int argc, const char **argv)
+{
+  unsigned int opt;
+
+  if (argc)
+    {
+      for (opt = 0; opt < (ARRAY_SIZE (arm_arch_core_flags) - 1); opt++)
+	if ((strcmp (argv[argc - 1], arm_arch_core_flags[opt].name) == 0)
+	    && ((arm_arch_core_flags[opt].flags & FL_NOTM) == 0))
+	  return "-mthumb";
+
+      return NULL;
+    }
+  else
+    return NULL;
+}
+
 #undef ARM_CPU_NAME_LENGTH
 
 
diff --git a/gcc/config/arm/arm-opts.h b/gcc/config/arm/arm-opts.h
index 039e333..222d20e 100644
--- a/gcc/config/arm/arm-opts.h
+++ b/gcc/config/arm/arm-opts.h
@@ -77,4 +77,93 @@ enum arm_tls_type {
   TLS_GNU,
   TLS_GNU2
 };
+
+/* Flags used to identify the presence of processor capabilities.  */
+
+/* Bit values used to identify processor capabilities.  */
+#define FL_CO_PROC    (1 << 0)        /* Has external co-processor bus */
+#define FL_ARCH3M     (1 << 1)        /* Extended multiply */
+#define FL_MODE26     (1 << 2)        /* 26-bit mode support */
+#define FL_MODE32     (1 << 3)        /* 32-bit mode support */
+#define FL_ARCH4      (1 << 4)        /* Architecture rel 4 */
+#define FL_ARCH5      (1 << 5)        /* Architecture rel 5 */
+#define FL_THUMB      (1 << 6)        /* Thumb aware */
+#define FL_LDSCHED    (1 << 7)	      /* Load scheduling necessary */
+#define FL_STRONG     (1 << 8)	      /* StrongARM */
+#define FL_ARCH5E     (1 << 9)        /* DSP extensions to v5 */
+#define FL_XSCALE     (1 << 10)	      /* XScale */
+/* spare	      (1 << 11)	*/
+#define FL_ARCH6      (1 << 12)       /* Architecture rel 6.  Adds
+					 media instructions.  */
+#define FL_VFPV2      (1 << 13)       /* Vector Floating Point V2.  */
+#define FL_WBUF	      (1 << 14)	      /* Schedule for write buffer ops.
+					 Note: ARM6 & 7 derivatives only.  */
+#define FL_ARCH6K     (1 << 15)       /* Architecture rel 6 K extensions.  */
+#define FL_THUMB2     (1 << 16)	      /* Thumb-2.  */
+#define FL_NOTM	      (1 << 17)	      /* Instructions not present in the 'M'
+					 profile.  */
+#define FL_THUMB_DIV  (1 << 18)	      /* Hardware divide (Thumb mode).  */
+#define FL_VFPV3      (1 << 19)       /* Vector Floating Point V3.  */
+#define FL_NEON       (1 << 20)       /* Neon instructions.  */
+#define FL_ARCH7EM    (1 << 21)	      /* Instructions present in the ARMv7E-M
+					 architecture.  */
+#define FL_ARCH7      (1 << 22)       /* Architecture 7.  */
+#define FL_ARM_DIV    (1 << 23)	      /* Hardware divide (ARM mode).  */
+#define FL_ARCH8      (1 << 24)       /* Architecture 8.  */
+#define FL_CRC32      (1 << 25)	      /* ARMv8 CRC32 instructions.  */
+
+#define FL_SMALLMUL   (1 << 26)       /* Small multiply supported.  */
+
+#define FL_IWMMXT     (1 << 29)	      /* XScale v2 or "Intel Wireless MMX technology".  */
+#define FL_IWMMXT2    (1 << 30)       /* "Intel Wireless MMX2 technology".  */
+
+/* Flags that only effect tuning, not available instructions.  */
+#define FL_TUNE		(FL_WBUF | FL_VFPV2 | FL_STRONG | FL_LDSCHED \
+			 | FL_CO_PROC)
+
+#define FL_FOR_ARCH2	FL_NOTM
+#define FL_FOR_ARCH3	(FL_FOR_ARCH2 | FL_MODE32)
+#define FL_FOR_ARCH3M	(FL_FOR_ARCH3 | FL_ARCH3M)
+#define FL_FOR_ARCH4	(FL_FOR_ARCH3M | FL_ARCH4)
+#define FL_FOR_ARCH4T	(FL_FOR_ARCH4 | FL_THUMB)
+#define FL_FOR_ARCH5	(FL_FOR_ARCH4 | FL_ARCH5)
+#define FL_FOR_ARCH5T	(FL_FOR_ARCH5 | FL_THUMB)
+#define FL_FOR_ARCH5E	(FL_FOR_ARCH5 | FL_ARCH5E)
+#define FL_FOR_ARCH5TE	(FL_FOR_ARCH5E | FL_THUMB)
+#define FL_FOR_ARCH5TEJ	FL_FOR_ARCH5TE
+#define FL_FOR_ARCH6	(FL_FOR_ARCH5TE | FL_ARCH6)
+#define FL_FOR_ARCH6J	FL_FOR_ARCH6
+#define FL_FOR_ARCH6K	(FL_FOR_ARCH6 | FL_ARCH6K)
+#define FL_FOR_ARCH6Z	FL_FOR_ARCH6
+#define FL_FOR_ARCH6ZK	FL_FOR_ARCH6K
+#define FL_FOR_ARCH6T2	(FL_FOR_ARCH6 | FL_THUMB2)
+#define FL_FOR_ARCH6M	(FL_FOR_ARCH6 & ~FL_NOTM)
+#define FL_FOR_ARCH7	((FL_FOR_ARCH6T2 & ~FL_NOTM) | FL_ARCH7)
+#define FL_FOR_ARCH7A	(FL_FOR_ARCH7 | FL_NOTM | FL_ARCH6K)
+#define FL_FOR_ARCH7VE	(FL_FOR_ARCH7A | FL_THUMB_DIV | FL_ARM_DIV)
+#define FL_FOR_ARCH7R	(FL_FOR_ARCH7A | FL_THUMB_DIV)
+#define FL_FOR_ARCH7M	(FL_FOR_ARCH7 | FL_THUMB_DIV)
+#define FL_FOR_ARCH7EM  (FL_FOR_ARCH7M | FL_ARCH7EM)
+#define FL_FOR_ARCH8A	(FL_FOR_ARCH7VE | FL_ARCH8)
+
+struct arm_arch_core_flag
+{
+  const char *const name;
+  const unsigned long flags;
+};
+
+static const struct arm_arch_core_flag arm_arch_core_flags[] =
+{
+#undef ARM_CORE
+#define ARM_CORE(NAME, X, IDENT, ARCH, FLAGS, COSTS) \
+  {NAME, FLAGS | FL_FOR_ARCH##ARCH},
+#include "arm-cores.def"
+#undef ARM_CORE
+#undef ARM_ARCH
+#define ARM_ARCH(NAME, CORE, ARCH, FLAGS) \
+  {NAME, FLAGS},
+#include "arm-arches.def"
+#undef ARM_ARCH
+  {NULL, 0}
+};
 #endif
diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
index 307babb..ebb341b 100644
--- a/gcc/config/arm/arm-protos.h
+++ b/gcc/config/arm/arm-protos.h
@@ -325,74 +325,6 @@ extern const char *arm_rewrite_selected_cpu (const char *name);
 
 extern bool arm_is_constant_pool_ref (rtx);
 
-/* Flags used to identify the presence of processor capabilities.  */
-
-/* Bit values used to identify processor capabilities.  */
-#define FL_CO_PROC    (1 << 0)        /* Has external co-processor bus */
-#define FL_ARCH3M     (1 << 1)        /* Extended multiply */
-#define FL_MODE26     (1 << 2)        /* 26-bit mode support */
-#define FL_MODE32     (1 << 3)        /* 32-bit mode support */
-#define FL_ARCH4      (1 << 4)        /* Architecture rel 4 */
-#define FL_ARCH5      (1 << 5)        /* Architecture rel 5 */
-#define FL_THUMB      (1 << 6)        /* Thumb aware */
-#define FL_LDSCHED    (1 << 7)	      /* Load scheduling necessary */
-#define FL_STRONG     (1 << 8)	      /* StrongARM */
-#define FL_ARCH5E     (1 << 9)        /* DSP extensions to v5 */
-#define FL_XSCALE     (1 << 10)	      /* XScale */
-/* spare	      (1 << 11)	*/
-#define FL_ARCH6      (1 << 12)       /* Architecture rel 6.  Adds
-					 media instructions.  */
-#define FL_VFPV2      (1 << 13)       /* Vector Floating Point V2.  */
-#define FL_WBUF	      (1 << 14)	      /* Schedule for write buffer ops.
-					 Note: ARM6 & 7 derivatives only.  */
-#define FL_ARCH6K     (1 << 15)       /* Architecture rel 6 K extensions.  */
-#define FL_THUMB2     (1 << 16)	      /* Thumb-2.  */
-#define FL_NOTM	      (1 << 17)	      /* Instructions not present in the 'M'
-					 profile.  */
-#define FL_THUMB_DIV  (1 << 18)	      /* Hardware divide (Thumb mode).  */
-#define FL_VFPV3      (1 << 19)       /* Vector Floating Point V3.  */
-#define FL_NEON       (1 << 20)       /* Neon instructions.  */
-#define FL_ARCH7EM    (1 << 21)	      /* Instructions present in the ARMv7E-M
-					 architecture.  */
-#define FL_ARCH7      (1 << 22)       /* Architecture 7.  */
-#define FL_ARM_DIV    (1 << 23)	      /* Hardware divide (ARM mode).  */
-#define FL_ARCH8      (1 << 24)       /* Architecture 8.  */
-#define FL_CRC32      (1 << 25)	      /* ARMv8 CRC32 instructions.  */
-
-#define FL_SMALLMUL   (1 << 26)       /* Small multiply supported.  */
-
-#define FL_IWMMXT     (1 << 29)	      /* XScale v2 or "Intel Wireless MMX technology".  */
-#define FL_IWMMXT2    (1 << 30)       /* "Intel Wireless MMX2 technology".  */
-
-/* Flags that only effect tuning, not available instructions.  */
-#define FL_TUNE		(FL_WBUF | FL_VFPV2 | FL_STRONG | FL_LDSCHED \
-			 | FL_CO_PROC)
-
-#define FL_FOR_ARCH2	FL_NOTM
-#define FL_FOR_ARCH3	(FL_FOR_ARCH2 | FL_MODE32)
-#define FL_FOR_ARCH3M	(FL_FOR_ARCH3 | FL_ARCH3M)
-#define FL_FOR_ARCH4	(FL_FOR_ARCH3M | FL_ARCH4)
-#define FL_FOR_ARCH4T	(FL_FOR_ARCH4 | FL_THUMB)
-#define FL_FOR_ARCH5	(FL_FOR_ARCH4 | FL_ARCH5)
-#define FL_FOR_ARCH5T	(FL_FOR_ARCH5 | FL_THUMB)
-#define FL_FOR_ARCH5E	(FL_FOR_ARCH5 | FL_ARCH5E)
-#define FL_FOR_ARCH5TE	(FL_FOR_ARCH5E | FL_THUMB)
-#define FL_FOR_ARCH5TEJ	FL_FOR_ARCH5TE
-#define FL_FOR_ARCH6	(FL_FOR_ARCH5TE | FL_ARCH6)
-#define FL_FOR_ARCH6J	FL_FOR_ARCH6
-#define FL_FOR_ARCH6K	(FL_FOR_ARCH6 | FL_ARCH6K)
-#define FL_FOR_ARCH6Z	FL_FOR_ARCH6
-#define FL_FOR_ARCH6ZK	FL_FOR_ARCH6K
-#define FL_FOR_ARCH6T2	(FL_FOR_ARCH6 | FL_THUMB2)
-#define FL_FOR_ARCH6M	(FL_FOR_ARCH6 & ~FL_NOTM)
-#define FL_FOR_ARCH7	((FL_FOR_ARCH6T2 & ~FL_NOTM) | FL_ARCH7)
-#define FL_FOR_ARCH7A	(FL_FOR_ARCH7 | FL_NOTM | FL_ARCH6K)
-#define FL_FOR_ARCH7VE	(FL_FOR_ARCH7A | FL_THUMB_DIV | FL_ARM_DIV)
-#define FL_FOR_ARCH7R	(FL_FOR_ARCH7A | FL_THUMB_DIV)
-#define FL_FOR_ARCH7M	(FL_FOR_ARCH7 | FL_THUMB_DIV)
-#define FL_FOR_ARCH7EM  (FL_FOR_ARCH7M | FL_ARCH7EM)
-#define FL_FOR_ARCH8A	(FL_FOR_ARCH7VE | FL_ARCH8)
-
 /* The bits in this mask specify which
    instructions we are allowed to generate.  */
 extern unsigned long insn_flags;
diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index 297dfe1..22b0f44 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -2388,13 +2388,18 @@ extern const char *arm_rewrite_mcpu (int argc, const char **argv);
    "   :%{march=*:-march=%*}}"					\
    BIG_LITTLE_SPEC
 
+extern const char *arm_is_target_thumb_only (int argc, const char **argv);
+#define MODE_SET_SPEC_FUNCTIONS						\
+  { "target_mode_check", arm_is_target_thumb_only },
+
 /* -mcpu=native handling only makes sense with compiler running on
    an ARM chip.  */
 #if defined(__arm__)
 extern const char *host_detect_local_cpu (int argc, const char **argv);
 # define EXTRA_SPEC_FUNCTIONS						\
   { "local_cpu_detect", host_detect_local_cpu },			\
-  BIG_LITTLE_CPU_SPEC_FUNCTIONS
+  BIG_LITTLE_CPU_SPEC_FUNCTIONS						\
+  MODE_SET_SPEC_FUNCTIONS
 
 # define MCPU_MTUNE_NATIVE_SPECS					\
    " %{march=native:%<march=native %:local_cpu_detect(arch)}"		\
@@ -2402,9 +2407,19 @@ extern const char *host_detect_local_cpu (int argc, const char **argv);
    " %{mtune=native:%<mtune=native %:local_cpu_detect(tune)}"
 #else
 # define MCPU_MTUNE_NATIVE_SPECS ""
-# define EXTRA_SPEC_FUNCTIONS BIG_LITTLE_CPU_SPEC_FUNCTIONS
+# define EXTRA_SPEC_FUNCTIONS						\
+  BIG_LITTLE_CPU_SPEC_FUNCTIONS						\
+  MODE_SET_SPEC_FUNCTIONS
 #endif
 
-#define DRIVER_SELF_SPECS MCPU_MTUNE_NATIVE_SPECS
+/* Automatically add -mthumb for thumb-only target if mode isn't specified
+   via configuration option --with-mode and command line.
+   If -march present, we collect just -march options.  Otherwise we
+   collect just -mcpu options.  The last one of them will be used to
+   decide target mode.  */
+#define MODE_SET_SPECS							\
+  " %{!marm:%{!mthumb:%:target_mode_check(%{march=*:%*;mcpu=*:%*})}}"
+
+#define DRIVER_SELF_SPECS MCPU_MTUNE_NATIVE_SPECS MODE_SET_SPECS
 #define TARGET_SUPPORTS_WIDE_INT 1
 #endif /* ! GCC_ARM_H */

Re: [PATCH][ARM]Automatically add -mthumb for thumb-only target when mode isn't specified

[Maxim Kuvyrkov]

> On Mar 2, 2015, at 4:44 AM, Terry Guo <terry.guo@arm.com> wrote:
> 
> Hi there,
> 
> If target mode isn't specified via either gcc configuration option
> --with-mode or command line, this patch intends to improve gcc driver to
> automatically add option -mthumb for thumb-only target. Tested with gcc
> regression test for various arm targets, no regression. Is it OK?
> 
> BR,
> Terry
> 
> gcc/ChangeLog:
> 
> 2015-03-02  Terry Guo  <terry.guo@arm.com>
> 
>        * common/config/arm/arm-common.c (arm_is_target_thumb_only): New
> function.
>        * config/arm/arm-protos.h (FL_ Macros): Move to ...
>        * config/arm/arm-opts.h (FL_ Macros): ... here.
>        (struct arm_arch_core_flag): New struct.
>        (arm_arch_core_flags): New array for arch/core and flag map.
>        * config/arm/arm.h (MODE_SET_SPEC_FUNCTIONS): Define new SPEC
> function.
>        (EXTRA_SPEC_FUNCTIONS): Include new SPEC function.
>        (MODE_SET_SPECS): New SPEC.
>        (DRIVER_SELF_SPECS): Include new SPEC.<gcc-mthumb-option-v5.txt>

Did you consider approach of implementing this purely inside cc1 rather than driver?

We do not seem to need to pass -mthumb to assembler or linker since those will pick up ARM-ness / Thumb-ness from function annotations.  Therefore we need to handle -marm / -mthumb for cc1 only.  What am I missing?

Also, what's the significance of moving FL_* flags to arm-opts.h?  If you had to separate FL_* definitions from the rest of arm-protos.h, then a new dedicated file (e.g., arm-fl.h) would be a better choice for new home of FL_* definitions.

Thank you,

--
Maxim Kuvyrkov
www.linaro.org

Re: [PATCH][ARM]Automatically add -mthumb for thumb-only target when mode isn't specified

[James Greenhalgh]

On Mon, Mar 02, 2015 at 01:08:13PM +0000, Maxim Kuvyrkov wrote:
> > On Mar 2, 2015, at 4:44 AM, Terry Guo <terry.guo@arm.com> wrote:
> > 
> > Hi there,
> > 
> > If target mode isn't specified via either gcc configuration option
> > --with-mode or command line, this patch intends to improve gcc driver to
> > automatically add option -mthumb for thumb-only target. Tested with gcc
> > regression test for various arm targets, no regression. Is it OK?
> > 
> > BR,
> > Terry
> > 
> > gcc/ChangeLog:
> > 
> > 2015-03-02  Terry Guo  <terry.guo@arm.com>
> > 
> >        * common/config/arm/arm-common.c (arm_is_target_thumb_only): New
> > function.
> >        * config/arm/arm-protos.h (FL_ Macros): Move to ...
> >        * config/arm/arm-opts.h (FL_ Macros): ... here.
> >        (struct arm_arch_core_flag): New struct.
> >        (arm_arch_core_flags): New array for arch/core and flag map.
> >        * config/arm/arm.h (MODE_SET_SPEC_FUNCTIONS): Define new SPEC
> > function.
> >        (EXTRA_SPEC_FUNCTIONS): Include new SPEC function.
> >        (MODE_SET_SPECS): New SPEC.
> >        (DRIVER_SELF_SPECS): Include new SPEC.<gcc-mthumb-option-v5.txt>
> 
> Did you consider approach of implementing this purely inside cc1 rather than driver?
> 
> We do not seem to need to pass -mthumb to assembler or linker since those
> will pick up ARM-ness / Thumb-ness from function annotations.  Therefore we
> need to handle -marm / -mthumb for cc1 only.  What am I missing?

I recently had a similar argument with myself regarding the usefulness
of rewriting -mcpu values in the driver before handing them off the
assembler (as we do for big.LITTLE systems), we could just rely on the
.arch directives in the assembler files.

The problem with this argument, and the one you make here, is that it
doesn't cover users driving the assembler with a GCC command to assemble
hand-rolled files without directives. i.e.

  gcc foo.s bar.c -mthumb -mcpu=cortex-a57.cortex-a53

Should have the effect when assembling foo.s of enforcing thumb mode,
and permitting ARMv8-A instructions, regardless whether foo.s explicitly
enables these through directives.

Cheers,
James

RE: [PATCH][ARM]Automatically add -mthumb for thumb-only target when mode isn't specified

[Kyrill Tkachov]

> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of James Greenhalgh
> Sent: 02 March 2015 13:28
> To: Maxim Kuvyrkov
> Cc: Terry Guo; gcc-patches@gcc.gnu.org; Ramana Radhakrishnan; Richard
> Earnshaw
> Subject: Re: [PATCH][ARM]Automatically add -mthumb for thumb-only
> target when mode isn't specified
> 
> On Mon, Mar 02, 2015 at 01:08:13PM +0000, Maxim Kuvyrkov wrote:
> > > On Mar 2, 2015, at 4:44 AM, Terry Guo <terry.guo@arm.com> wrote:
> > >
> > > Hi there,
> > >
> > > If target mode isn't specified via either gcc configuration option
> > > --with-mode or command line, this patch intends to improve gcc
> > > driver to automatically add option -mthumb for thumb-only target.
> > > Tested with gcc regression test for various arm targets, no
regression. Is it
> OK?


FWIW, I had this annoyance that the patch tries to fix as well and filed a
PR for it:
PR target/64802

Kyrill

> > >
> > > BR,
> > > Terry
> > >
> > > gcc/ChangeLog:
> > >
> > > 2015-03-02  Terry Guo  <terry.guo@arm.com>
> > >
> > >        * common/config/arm/arm-common.c (arm_is_target_thumb_only):
> > > New function.
> > >        * config/arm/arm-protos.h (FL_ Macros): Move to ...
> > >        * config/arm/arm-opts.h (FL_ Macros): ... here.
> > >        (struct arm_arch_core_flag): New struct.
> > >        (arm_arch_core_flags): New array for arch/core and flag map.
> > >        * config/arm/arm.h (MODE_SET_SPEC_FUNCTIONS): Define new
> SPEC
> > > function.
> > >        (EXTRA_SPEC_FUNCTIONS): Include new SPEC function.
> > >        (MODE_SET_SPECS): New SPEC.
> > >        (DRIVER_SELF_SPECS): Include new
> > > SPEC.<gcc-mthumb-option-v5.txt>
> >
> > Did you consider approach of implementing this purely inside cc1 rather
> than driver?
> >
> > We do not seem to need to pass -mthumb to assembler or linker since
> > those will pick up ARM-ness / Thumb-ness from function annotations.
> > Therefore we need to handle -marm / -mthumb for cc1 only.  What am I
> missing?
> 
> I recently had a similar argument with myself regarding the usefulness of
> rewriting -mcpu values in the driver before handing them off the assembler
> (as we do for big.LITTLE systems), we could just rely on the .arch
directives in
> the assembler files.
> 
> The problem with this argument, and the one you make here, is that it
> doesn't cover users driving the assembler with a GCC command to assemble
> hand-rolled files without directives. i.e.
> 
>   gcc foo.s bar.c -mthumb -mcpu=cortex-a57.cortex-a53
> 
> Should have the effect when assembling foo.s of enforcing thumb mode,
> and permitting ARMv8-A instructions, regardless whether foo.s explicitly
> enables these through directives.
> 
> Cheers,
> James
> 

Re: [PATCH][ARM]Automatically add -mthumb for thumb-only target when mode isn't specified

[Terry Guo]

On Mon, Mar 2, 2015 at 9:08 PM, Maxim Kuvyrkov
<maxim.kuvyrkov@linaro.org> wrote:
>> On Mar 2, 2015, at 4:44 AM, Terry Guo <terry.guo@arm.com> wrote:
>>
>> Hi there,
>>
>> If target mode isn't specified via either gcc configuration option
>> --with-mode or command line, this patch intends to improve gcc driver to
>> automatically add option -mthumb for thumb-only target. Tested with gcc
>> regression test for various arm targets, no regression. Is it OK?
>>
>> BR,
>> Terry
>>
>> gcc/ChangeLog:
>>
>> 2015-03-02  Terry Guo  <terry.guo@arm.com>
>>
>>        * common/config/arm/arm-common.c (arm_is_target_thumb_only): New
>> function.
>>        * config/arm/arm-protos.h (FL_ Macros): Move to ...
>>        * config/arm/arm-opts.h (FL_ Macros): ... here.
>>        (struct arm_arch_core_flag): New struct.
>>        (arm_arch_core_flags): New array for arch/core and flag map.
>>        * config/arm/arm.h (MODE_SET_SPEC_FUNCTIONS): Define new SPEC
>> function.
>>        (EXTRA_SPEC_FUNCTIONS): Include new SPEC function.
>>        (MODE_SET_SPECS): New SPEC.
>>        (DRIVER_SELF_SPECS): Include new SPEC.<gcc-mthumb-option-v5.txt>
>
> Did you consider approach of implementing this purely inside cc1 rather than driver?
>
> We do not seem to need to pass -mthumb to assembler or linker since those will pick up ARM-ness / Thumb-ness from function annotations.  Therefore we need to handle -marm / -mthumb for cc1 only.  What am I missing?
>

The way GCC uses to find multitlib prevents us from doing this via
cc1. The target options should be properly constructed for gcc driver
to decide multilib path, which happens before cc1. For example, for
command line "arm-nonee-abi-gcc -mcpu=cortex-m3 -o hello.axf hello.c",
we need to figure out that -mthumb should be added inside the gcc
driver, otherwise such command line works like "arm-nonee-abi-gcc
-marm -mcpu=cortex-m3 -o hello.axf hello.c" and the arm mode multilib
will be linked. Thus we have to do this in gcc driver rather than cc1.

> Also, what's the significance of moving FL_* flags to arm-opts.h?  If you had to separate FL_* definitions from the rest of arm-protos.h, then a new dedicated file (e.g., arm-fl.h) would be a better choice for new home of FL_* definitions.
>

I set up an arch/core<->flags map array for gcc driver to figure out
whether the target is thumb-only. Those FL_* flags are needed for this
map array. The arm-opts.h is used to share back end information with
gcc driver. Normally we tend to minimize such information. That's why
I just moved those FL_* flags rather than simply including the header
file which has FL_* flags. But maybe it is a good idea to save FL_*
into a separate file. I will try.

BR,
Terry

Re: [PATCH][ARM]Automatically add -mthumb for thumb-only target when mode isn't specified

[Terry Guo]

On Mon, Mar 2, 2015 at 9:08 PM, Maxim Kuvyrkov
<maxim.kuvyrkov@linaro.org> wrote:
>> On Mar 2, 2015, at 4:44 AM, Terry Guo <terry.guo@arm.com> wrote:
>>
>> Hi there,
>>
>> If target mode isn't specified via either gcc configuration option
>> --with-mode or command line, this patch intends to improve gcc driver to
>> automatically add option -mthumb for thumb-only target. Tested with gcc
>> regression test for various arm targets, no regression. Is it OK?
>>
>> BR,
>> Terry
>>
>> gcc/ChangeLog:
>>
>> 2015-03-02  Terry Guo  <terry.guo@arm.com>
>>
>>        * common/config/arm/arm-common.c (arm_is_target_thumb_only): New
>> function.
>>        * config/arm/arm-protos.h (FL_ Macros): Move to ...
>>        * config/arm/arm-opts.h (FL_ Macros): ... here.
>>        (struct arm_arch_core_flag): New struct.
>>        (arm_arch_core_flags): New array for arch/core and flag map.
>>        * config/arm/arm.h (MODE_SET_SPEC_FUNCTIONS): Define new SPEC
>> function.
>>        (EXTRA_SPEC_FUNCTIONS): Include new SPEC function.
>>        (MODE_SET_SPECS): New SPEC.
>>        (DRIVER_SELF_SPECS): Include new SPEC.<gcc-mthumb-option-v5.txt>
>
> Did you consider approach of implementing this purely inside cc1 rather than driver?
>
> We do not seem to need to pass -mthumb to assembler or linker since those will pick up ARM-ness / Thumb-ness from function annotations.  Therefore we need to handle -marm / -mthumb for cc1 only.  What am I missing?
>
> Also, what's the significance of moving FL_* flags to arm-opts.h?  If you had to separate FL_* definitions from the rest of arm-protos.h, then a new dedicated file (e.g., arm-fl.h) would be a better choice for new home of FL_* definitions.
>

Please find my answers in another email. The attached patch tries to
follow your idea that puts those FL_* into separate file named
arm-flags.h. Does it look good to you?

BR,
Terry

Re: [PATCH][ARM]Automatically add -mthumb for thumb-only target when mode isn't specified

[Terry Guo]

[-- Attachment #1: Type: text/plain, Size: 2024 bytes --]

On Wed, Mar 4, 2015 at 10:44 AM, Terry Guo <flameroc@gmail.com> wrote:
> On Mon, Mar 2, 2015 at 9:08 PM, Maxim Kuvyrkov
> <maxim.kuvyrkov@linaro.org> wrote:
>>> On Mar 2, 2015, at 4:44 AM, Terry Guo <terry.guo@arm.com> wrote:
>>>
>>> Hi there,
>>>
>>> If target mode isn't specified via either gcc configuration option
>>> --with-mode or command line, this patch intends to improve gcc driver to
>>> automatically add option -mthumb for thumb-only target. Tested with gcc
>>> regression test for various arm targets, no regression. Is it OK?
>>>
>>> BR,
>>> Terry
>>>
>>> gcc/ChangeLog:
>>>
>>> 2015-03-02  Terry Guo  <terry.guo@arm.com>
>>>
>>>        * common/config/arm/arm-common.c (arm_is_target_thumb_only): New
>>> function.
>>>        * config/arm/arm-protos.h (FL_ Macros): Move to ...
>>>        * config/arm/arm-opts.h (FL_ Macros): ... here.
>>>        (struct arm_arch_core_flag): New struct.
>>>        (arm_arch_core_flags): New array for arch/core and flag map.
>>>        * config/arm/arm.h (MODE_SET_SPEC_FUNCTIONS): Define new SPEC
>>> function.
>>>        (EXTRA_SPEC_FUNCTIONS): Include new SPEC function.
>>>        (MODE_SET_SPECS): New SPEC.
>>>        (DRIVER_SELF_SPECS): Include new SPEC.<gcc-mthumb-option-v5.txt>
>>
>> Did you consider approach of implementing this purely inside cc1 rather than driver?
>>
>> We do not seem to need to pass -mthumb to assembler or linker since those will pick up ARM-ness / Thumb-ness from function annotations.  Therefore we need to handle -marm / -mthumb for cc1 only.  What am I missing?
>>
>> Also, what's the significance of moving FL_* flags to arm-opts.h?  If you had to separate FL_* definitions from the rest of arm-protos.h, then a new dedicated file (e.g., arm-fl.h) would be a better choice for new home of FL_* definitions.
>>
>
> Please find my answers in another email. The attached patch tries to
> follow your idea that puts those FL_* into separate file named
> arm-flags.h. Does it look good to you?
>
> BR,
> Terry

Sorry for missing patch.

[-- Attachment #2: gcc-mthumb-option-v6.txt --]
[-- Type: text/plain, Size: 12412 bytes --]

diff --git a/gcc/common/config/arm/arm-common.c b/gcc/common/config/arm/arm-common.c
index 86673b7..e17ee03 100644
--- a/gcc/common/config/arm/arm-common.c
+++ b/gcc/common/config/arm/arm-common.c
@@ -97,6 +97,28 @@ arm_rewrite_mcpu (int argc, const char **argv)
   return arm_rewrite_selected_cpu (argv[argc - 1]);
 }
 
+/* Called by driver to check whether the target denoted by current
+   command line options is thumb-only target.  If -march present,
+   check the last -march option.  If no -march, check the last -mcpu
+   option.  */
+const char *
+arm_is_target_thumb_only (int argc, const char **argv)
+{
+  unsigned int opt;
+
+  if (argc)
+    {
+      for (opt = 0; opt < (ARRAY_SIZE (arm_arch_core_flags) - 1); opt++)
+	if ((strcmp (argv[argc - 1], arm_arch_core_flags[opt].name) == 0)
+	    && ((arm_arch_core_flags[opt].flags & FL_NOTM) == 0))
+	  return "-mthumb";
+
+      return NULL;
+    }
+  else
+    return NULL;
+}
+
 #undef ARM_CPU_NAME_LENGTH
 
 
diff --git a/gcc/config/arm/arm-flags.h b/gcc/config/arm/arm-flags.h
new file mode 100644
index 0000000..fe3a723
--- /dev/null
+++ b/gcc/config/arm/arm-flags.h
@@ -0,0 +1,92 @@
+/* Flags used to identify the presence of processor capabilities. 
+
+   Copyright (C) 2015 Free Software Foundation, Inc.
+   Contributed by ARM Ltd.
+
+   This file is part of GCC.
+
+   GCC is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published
+   by the Free Software Foundation; either version 3, or (at your
+   option) any later version.
+
+   GCC is distributed in the hope that it will be useful, but WITHOUT
+   ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+   or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
+   License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with GCC; see the file COPYING3.  If not see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef GCC_ARM_FLAGS_H
+#define GCC_ARM_FLAGS_H
+
+/* Bit values used to identify processor capabilities.  */
+#define FL_CO_PROC    (1 << 0)        /* Has external co-processor bus */
+#define FL_ARCH3M     (1 << 1)        /* Extended multiply */
+#define FL_MODE26     (1 << 2)        /* 26-bit mode support */
+#define FL_MODE32     (1 << 3)        /* 32-bit mode support */
+#define FL_ARCH4      (1 << 4)        /* Architecture rel 4 */
+#define FL_ARCH5      (1 << 5)        /* Architecture rel 5 */
+#define FL_THUMB      (1 << 6)        /* Thumb aware */
+#define FL_LDSCHED    (1 << 7)	      /* Load scheduling necessary */
+#define FL_STRONG     (1 << 8)	      /* StrongARM */
+#define FL_ARCH5E     (1 << 9)        /* DSP extensions to v5 */
+#define FL_XSCALE     (1 << 10)	      /* XScale */
+/* spare	      (1 << 11)	*/
+#define FL_ARCH6      (1 << 12)       /* Architecture rel 6.  Adds
+					 media instructions.  */
+#define FL_VFPV2      (1 << 13)       /* Vector Floating Point V2.  */
+#define FL_WBUF	      (1 << 14)	      /* Schedule for write buffer ops.
+					 Note: ARM6 & 7 derivatives only.  */
+#define FL_ARCH6K     (1 << 15)       /* Architecture rel 6 K extensions.  */
+#define FL_THUMB2     (1 << 16)	      /* Thumb-2.  */
+#define FL_NOTM	      (1 << 17)	      /* Instructions not present in the 'M'
+					 profile.  */
+#define FL_THUMB_DIV  (1 << 18)	      /* Hardware divide (Thumb mode).  */
+#define FL_VFPV3      (1 << 19)       /* Vector Floating Point V3.  */
+#define FL_NEON       (1 << 20)       /* Neon instructions.  */
+#define FL_ARCH7EM    (1 << 21)	      /* Instructions present in the ARMv7E-M
+					 architecture.  */
+#define FL_ARCH7      (1 << 22)       /* Architecture 7.  */
+#define FL_ARM_DIV    (1 << 23)	      /* Hardware divide (ARM mode).  */
+#define FL_ARCH8      (1 << 24)       /* Architecture 8.  */
+#define FL_CRC32      (1 << 25)	      /* ARMv8 CRC32 instructions.  */
+
+#define FL_SMALLMUL   (1 << 26)       /* Small multiply supported.  */
+#define FL_NO_VOLATILE_CE   (1 << 27) /* No volatile memory in IT block.  */
+
+#define FL_IWMMXT     (1 << 29)	      /* XScale v2 or "Intel Wireless MMX technology".  */
+#define FL_IWMMXT2    (1 << 30)       /* "Intel Wireless MMX2 technology".  */
+
+/* Flags that only effect tuning, not available instructions.  */
+#define FL_TUNE		(FL_WBUF | FL_VFPV2 | FL_STRONG | FL_LDSCHED \
+			 | FL_CO_PROC)
+
+#define FL_FOR_ARCH2	FL_NOTM
+#define FL_FOR_ARCH3	(FL_FOR_ARCH2 | FL_MODE32)
+#define FL_FOR_ARCH3M	(FL_FOR_ARCH3 | FL_ARCH3M)
+#define FL_FOR_ARCH4	(FL_FOR_ARCH3M | FL_ARCH4)
+#define FL_FOR_ARCH4T	(FL_FOR_ARCH4 | FL_THUMB)
+#define FL_FOR_ARCH5	(FL_FOR_ARCH4 | FL_ARCH5)
+#define FL_FOR_ARCH5T	(FL_FOR_ARCH5 | FL_THUMB)
+#define FL_FOR_ARCH5E	(FL_FOR_ARCH5 | FL_ARCH5E)
+#define FL_FOR_ARCH5TE	(FL_FOR_ARCH5E | FL_THUMB)
+#define FL_FOR_ARCH5TEJ	FL_FOR_ARCH5TE
+#define FL_FOR_ARCH6	(FL_FOR_ARCH5TE | FL_ARCH6)
+#define FL_FOR_ARCH6J	FL_FOR_ARCH6
+#define FL_FOR_ARCH6K	(FL_FOR_ARCH6 | FL_ARCH6K)
+#define FL_FOR_ARCH6Z	FL_FOR_ARCH6
+#define FL_FOR_ARCH6ZK	FL_FOR_ARCH6K
+#define FL_FOR_ARCH6T2	(FL_FOR_ARCH6 | FL_THUMB2)
+#define FL_FOR_ARCH6M	(FL_FOR_ARCH6 & ~FL_NOTM)
+#define FL_FOR_ARCH7	((FL_FOR_ARCH6T2 & ~FL_NOTM) | FL_ARCH7)
+#define FL_FOR_ARCH7A	(FL_FOR_ARCH7 | FL_NOTM | FL_ARCH6K)
+#define FL_FOR_ARCH7VE	(FL_FOR_ARCH7A | FL_THUMB_DIV | FL_ARM_DIV)
+#define FL_FOR_ARCH7R	(FL_FOR_ARCH7A | FL_THUMB_DIV)
+#define FL_FOR_ARCH7M	(FL_FOR_ARCH7 | FL_THUMB_DIV)
+#define FL_FOR_ARCH7EM  (FL_FOR_ARCH7M | FL_ARCH7EM)
+#define FL_FOR_ARCH8A	(FL_FOR_ARCH7VE | FL_ARCH8)
+
+#endif /* GCC_ARM_FLAGS_H */
diff --git a/gcc/config/arm/arm-opts.h b/gcc/config/arm/arm-opts.h
index 039e333..dd727cf 100644
--- a/gcc/config/arm/arm-opts.h
+++ b/gcc/config/arm/arm-opts.h
@@ -25,6 +25,8 @@
 #ifndef ARM_OPTS_H
 #define ARM_OPTS_H
 
+#include "arm-flags.h"
+
 /* The various ARM cores.  */
 enum processor_type
 {
@@ -77,4 +79,25 @@ enum arm_tls_type {
   TLS_GNU,
   TLS_GNU2
 };
+
+struct arm_arch_core_flag
+{
+  const char *const name;
+  const unsigned long flags;
+};
+
+static const struct arm_arch_core_flag arm_arch_core_flags[] =
+{
+#undef ARM_CORE
+#define ARM_CORE(NAME, X, IDENT, ARCH, FLAGS, COSTS) \
+  {NAME, FLAGS | FL_FOR_ARCH##ARCH},
+#include "arm-cores.def"
+#undef ARM_CORE
+#undef ARM_ARCH
+#define ARM_ARCH(NAME, CORE, ARCH, FLAGS) \
+  {NAME, FLAGS},
+#include "arm-arches.def"
+#undef ARM_ARCH
+  {NULL, 0}
+};
 #endif
diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
index 28ffe52..325a81c 100644
--- a/gcc/config/arm/arm-protos.h
+++ b/gcc/config/arm/arm-protos.h
@@ -325,75 +325,6 @@ extern const char *arm_rewrite_selected_cpu (const char *name);
 
 extern bool arm_is_constant_pool_ref (rtx);
 
-/* Flags used to identify the presence of processor capabilities.  */
-
-/* Bit values used to identify processor capabilities.  */
-#define FL_CO_PROC    (1 << 0)        /* Has external co-processor bus */
-#define FL_ARCH3M     (1 << 1)        /* Extended multiply */
-#define FL_MODE26     (1 << 2)        /* 26-bit mode support */
-#define FL_MODE32     (1 << 3)        /* 32-bit mode support */
-#define FL_ARCH4      (1 << 4)        /* Architecture rel 4 */
-#define FL_ARCH5      (1 << 5)        /* Architecture rel 5 */
-#define FL_THUMB      (1 << 6)        /* Thumb aware */
-#define FL_LDSCHED    (1 << 7)	      /* Load scheduling necessary */
-#define FL_STRONG     (1 << 8)	      /* StrongARM */
-#define FL_ARCH5E     (1 << 9)        /* DSP extensions to v5 */
-#define FL_XSCALE     (1 << 10)	      /* XScale */
-/* spare	      (1 << 11)	*/
-#define FL_ARCH6      (1 << 12)       /* Architecture rel 6.  Adds
-					 media instructions.  */
-#define FL_VFPV2      (1 << 13)       /* Vector Floating Point V2.  */
-#define FL_WBUF	      (1 << 14)	      /* Schedule for write buffer ops.
-					 Note: ARM6 & 7 derivatives only.  */
-#define FL_ARCH6K     (1 << 15)       /* Architecture rel 6 K extensions.  */
-#define FL_THUMB2     (1 << 16)	      /* Thumb-2.  */
-#define FL_NOTM	      (1 << 17)	      /* Instructions not present in the 'M'
-					 profile.  */
-#define FL_THUMB_DIV  (1 << 18)	      /* Hardware divide (Thumb mode).  */
-#define FL_VFPV3      (1 << 19)       /* Vector Floating Point V3.  */
-#define FL_NEON       (1 << 20)       /* Neon instructions.  */
-#define FL_ARCH7EM    (1 << 21)	      /* Instructions present in the ARMv7E-M
-					 architecture.  */
-#define FL_ARCH7      (1 << 22)       /* Architecture 7.  */
-#define FL_ARM_DIV    (1 << 23)	      /* Hardware divide (ARM mode).  */
-#define FL_ARCH8      (1 << 24)       /* Architecture 8.  */
-#define FL_CRC32      (1 << 25)	      /* ARMv8 CRC32 instructions.  */
-
-#define FL_SMALLMUL   (1 << 26)       /* Small multiply supported.  */
-#define FL_NO_VOLATILE_CE   (1 << 27) /* No volatile memory in IT block.  */
-
-#define FL_IWMMXT     (1 << 29)	      /* XScale v2 or "Intel Wireless MMX technology".  */
-#define FL_IWMMXT2    (1 << 30)       /* "Intel Wireless MMX2 technology".  */
-
-/* Flags that only effect tuning, not available instructions.  */
-#define FL_TUNE		(FL_WBUF | FL_VFPV2 | FL_STRONG | FL_LDSCHED \
-			 | FL_CO_PROC)
-
-#define FL_FOR_ARCH2	FL_NOTM
-#define FL_FOR_ARCH3	(FL_FOR_ARCH2 | FL_MODE32)
-#define FL_FOR_ARCH3M	(FL_FOR_ARCH3 | FL_ARCH3M)
-#define FL_FOR_ARCH4	(FL_FOR_ARCH3M | FL_ARCH4)
-#define FL_FOR_ARCH4T	(FL_FOR_ARCH4 | FL_THUMB)
-#define FL_FOR_ARCH5	(FL_FOR_ARCH4 | FL_ARCH5)
-#define FL_FOR_ARCH5T	(FL_FOR_ARCH5 | FL_THUMB)
-#define FL_FOR_ARCH5E	(FL_FOR_ARCH5 | FL_ARCH5E)
-#define FL_FOR_ARCH5TE	(FL_FOR_ARCH5E | FL_THUMB)
-#define FL_FOR_ARCH5TEJ	FL_FOR_ARCH5TE
-#define FL_FOR_ARCH6	(FL_FOR_ARCH5TE | FL_ARCH6)
-#define FL_FOR_ARCH6J	FL_FOR_ARCH6
-#define FL_FOR_ARCH6K	(FL_FOR_ARCH6 | FL_ARCH6K)
-#define FL_FOR_ARCH6Z	FL_FOR_ARCH6
-#define FL_FOR_ARCH6ZK	FL_FOR_ARCH6K
-#define FL_FOR_ARCH6T2	(FL_FOR_ARCH6 | FL_THUMB2)
-#define FL_FOR_ARCH6M	(FL_FOR_ARCH6 & ~FL_NOTM)
-#define FL_FOR_ARCH7	((FL_FOR_ARCH6T2 & ~FL_NOTM) | FL_ARCH7)
-#define FL_FOR_ARCH7A	(FL_FOR_ARCH7 | FL_NOTM | FL_ARCH6K)
-#define FL_FOR_ARCH7VE	(FL_FOR_ARCH7A | FL_THUMB_DIV | FL_ARM_DIV)
-#define FL_FOR_ARCH7R	(FL_FOR_ARCH7A | FL_THUMB_DIV)
-#define FL_FOR_ARCH7M	(FL_FOR_ARCH7 | FL_THUMB_DIV)
-#define FL_FOR_ARCH7EM  (FL_FOR_ARCH7M | FL_ARCH7EM)
-#define FL_FOR_ARCH8A	(FL_FOR_ARCH7VE | FL_ARCH8)
-
 /* The bits in this mask specify which
    instructions we are allowed to generate.  */
 extern unsigned long insn_flags;
diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index 8c10ea3..3119957 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -2394,13 +2394,18 @@ extern const char *arm_rewrite_mcpu (int argc, const char **argv);
    "   :%{march=*:-march=%*}}"					\
    BIG_LITTLE_SPEC
 
+extern const char *arm_is_target_thumb_only (int argc, const char **argv);
+#define MODE_SET_SPEC_FUNCTIONS						\
+  { "target_mode_check", arm_is_target_thumb_only },
+
 /* -mcpu=native handling only makes sense with compiler running on
    an ARM chip.  */
 #if defined(__arm__)
 extern const char *host_detect_local_cpu (int argc, const char **argv);
 # define EXTRA_SPEC_FUNCTIONS						\
   { "local_cpu_detect", host_detect_local_cpu },			\
-  BIG_LITTLE_CPU_SPEC_FUNCTIONS
+  BIG_LITTLE_CPU_SPEC_FUNCTIONS						\
+  MODE_SET_SPEC_FUNCTIONS
 
 # define MCPU_MTUNE_NATIVE_SPECS					\
    " %{march=native:%<march=native %:local_cpu_detect(arch)}"		\
@@ -2408,9 +2413,19 @@ extern const char *host_detect_local_cpu (int argc, const char **argv);
    " %{mtune=native:%<mtune=native %:local_cpu_detect(tune)}"
 #else
 # define MCPU_MTUNE_NATIVE_SPECS ""
-# define EXTRA_SPEC_FUNCTIONS BIG_LITTLE_CPU_SPEC_FUNCTIONS
+# define EXTRA_SPEC_FUNCTIONS						\
+  BIG_LITTLE_CPU_SPEC_FUNCTIONS						\
+  MODE_SET_SPEC_FUNCTIONS
 #endif
 
-#define DRIVER_SELF_SPECS MCPU_MTUNE_NATIVE_SPECS
+/* Automatically add -mthumb for thumb-only target if mode isn't specified
+   via configuration option --with-mode and command line.
+   If -march present, we collect just -march options.  Otherwise we
+   collect just -mcpu options.  The last one of them will be used to
+   decide target mode.  */
+#define MODE_SET_SPECS							\
+  " %{!marm:%{!mthumb:%:target_mode_check(%{march=*:%*;mcpu=*:%*})}}"
+
+#define DRIVER_SELF_SPECS MCPU_MTUNE_NATIVE_SPECS MODE_SET_SPECS
 #define TARGET_SUPPORTS_WIDE_INT 1
 #endif /* ! GCC_ARM_H */

Re: [PATCH][ARM]Automatically add -mthumb for thumb-only target when mode isn't specified

[Maxim Kuvyrkov]

> On Mar 4, 2015, at 5:46 AM, Terry Guo <flameroc@gmail.com> wrote:
> 
> On Wed, Mar 4, 2015 at 10:44 AM, Terry Guo <flameroc@gmail.com> wrote:
>> On Mon, Mar 2, 2015 at 9:08 PM, Maxim Kuvyrkov
>> <maxim.kuvyrkov@linaro.org> wrote:
>>>> On Mar 2, 2015, at 4:44 AM, Terry Guo <terry.guo@arm.com> wrote:
>>>> 
>>>> Hi there,
>>>> 
>>>> If target mode isn't specified via either gcc configuration option
>>>> --with-mode or command line, this patch intends to improve gcc driver to
>>>> automatically add option -mthumb for thumb-only target. Tested with gcc
>>>> regression test for various arm targets, no regression. Is it OK?
>>>> 
>>>> BR,
>>>> Terry
>>>> 
>>>> gcc/ChangeLog:
>>>> 
>>>> 2015-03-02  Terry Guo  <terry.guo@arm.com>
>>>> 
>>>>       * common/config/arm/arm-common.c (arm_is_target_thumb_only): New
>>>> function.
>>>>       * config/arm/arm-protos.h (FL_ Macros): Move to ...
>>>>       * config/arm/arm-opts.h (FL_ Macros): ... here.
>>>>       (struct arm_arch_core_flag): New struct.
>>>>       (arm_arch_core_flags): New array for arch/core and flag map.
>>>>       * config/arm/arm.h (MODE_SET_SPEC_FUNCTIONS): Define new SPEC
>>>> function.
>>>>       (EXTRA_SPEC_FUNCTIONS): Include new SPEC function.
>>>>       (MODE_SET_SPECS): New SPEC.
>>>>       (DRIVER_SELF_SPECS): Include new SPEC.<gcc-mthumb-option-v5.txt>
>>> 
>>> Did you consider approach of implementing this purely inside cc1 rather than driver?
>>> 
>>> We do not seem to need to pass -mthumb to assembler or linker since those will pick up ARM-ness / Thumb-ness from function annotations.  Therefore we need to handle -marm / -mthumb for cc1 only.  What am I missing?
>>> 
>>> Also, what's the significance of moving FL_* flags to arm-opts.h?  If you had to separate FL_* definitions from the rest of arm-protos.h, then a new dedicated file (e.g., arm-fl.h) would be a better choice for new home of FL_* definitions.
>>> 
>> 
>> Please find my answers in another email. The attached patch tries to
>> follow your idea that puts those FL_* into separate file named
>> arm-flags.h. Does it look good to you?

Thanks Terry (and everyone else) for explaining why we want to do this in the driver.  The substance of the patch looks good to me, and below are some comments and nit-picks.  (Also, I'm not an ARM maintainer, so this is a review, not an approval to commit).

Please make sure to update changelog before committing.


> diff --git a/gcc/common/config/arm/arm-common.c b/gcc/common/config/arm/arm-common.c
> index 86673b7..e17ee03 100644
> --- a/gcc/common/config/arm/arm-common.c
> +++ b/gcc/common/config/arm/arm-common.c
> @@ -97,6 +97,28 @@ arm_rewrite_mcpu (int argc, const char **argv)
>    return arm_rewrite_selected_cpu (argv[argc - 1]);
>  }
>  
> +/* Called by driver to check whether the target denoted by current
> +   command line options is thumb-only target.  If -march present,
> +   check the last -march option.  If no -march, check the last -mcpu
> +   option.  */
> +const char *
> +arm_is_target_thumb_only (int argc, const char **argv)

Consider renaming this to arm_target_thumb_only.  The "is" part of the name make it sound like a predicate function, which it is not.


> +{
> +  unsigned int opt;
> +
> +  if (argc)
> +    {
> +      for (opt = 0; opt < (ARRAY_SIZE (arm_arch_core_flags) - 1); opt++)
> +	if ((strcmp (argv[argc - 1], arm_arch_core_flags[opt].name) == 0)
> +	    && ((arm_arch_core_flags[opt].flags & FL_NOTM) == 0))
> +	  return "-mthumb";
> +
> +      return NULL;
> +    }
> +  else
> +    return NULL;
> +}
> +
>  #undef ARM_CPU_NAME_LENGTH
>  
>  
> diff --git a/gcc/config/arm/arm-flags.h b/gcc/config/arm/arm-flags.h
> new file mode 100644
> index 0000000..fe3a723
> --- /dev/null
> +++ b/gcc/config/arm/arm-flags.h
> @@ -0,0 +1,92 @@
> +/* Flags used to identify the presence of processor capabilities. 
> +
> +   Copyright (C) 2015 Free Software Foundation, Inc.
> +   Contributed by ARM Ltd.
> +
> +   This file is part of GCC.
> +
> +   GCC is free software; you can redistribute it and/or modify it
> +   under the terms of the GNU General Public License as published
> +   by the Free Software Foundation; either version 3, or (at your
> +   option) any later version.
> +
> +   GCC is distributed in the hope that it will be useful, but WITHOUT
> +   ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
> +   or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> +   License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with GCC; see the file COPYING3.  If not see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#ifndef GCC_ARM_FLAGS_H
> +#define GCC_ARM_FLAGS_H
> +
> +/* Bit values used to identify processor capabilities.  */
> +#define FL_CO_PROC    (1 << 0)        /* Has external co-processor bus */
> +#define FL_ARCH3M     (1 << 1)        /* Extended multiply */
> +#define FL_MODE26     (1 << 2)        /* 26-bit mode support */
> +#define FL_MODE32     (1 << 3)        /* 32-bit mode support */
> +#define FL_ARCH4      (1 << 4)        /* Architecture rel 4 */
> +#define FL_ARCH5      (1 << 5)        /* Architecture rel 5 */
> +#define FL_THUMB      (1 << 6)        /* Thumb aware */
> +#define FL_LDSCHED    (1 << 7)	      /* Load scheduling necessary */
> +#define FL_STRONG     (1 << 8)	      /* StrongARM */
> +#define FL_ARCH5E     (1 << 9)        /* DSP extensions to v5 */
> +#define FL_XSCALE     (1 << 10)	      /* XScale */
> +/* spare	      (1 << 11)	*/
> +#define FL_ARCH6      (1 << 12)       /* Architecture rel 6.  Adds
> +					 media instructions.  */
> +#define FL_VFPV2      (1 << 13)       /* Vector Floating Point V2.  */
> +#define FL_WBUF	      (1 << 14)	      /* Schedule for write buffer ops.
> +					 Note: ARM6 & 7 derivatives only.  */
> +#define FL_ARCH6K     (1 << 15)       /* Architecture rel 6 K extensions.  */
> +#define FL_THUMB2     (1 << 16)	      /* Thumb-2.  */
> +#define FL_NOTM	      (1 << 17)	      /* Instructions not present in the 'M'
> +					 profile.  */
> +#define FL_THUMB_DIV  (1 << 18)	      /* Hardware divide (Thumb mode).  */
> +#define FL_VFPV3      (1 << 19)       /* Vector Floating Point V3.  */
> +#define FL_NEON       (1 << 20)       /* Neon instructions.  */
> +#define FL_ARCH7EM    (1 << 21)	      /* Instructions present in the ARMv7E-M
> +					 architecture.  */
> +#define FL_ARCH7      (1 << 22)       /* Architecture 7.  */
> +#define FL_ARM_DIV    (1 << 23)	      /* Hardware divide (ARM mode).  */
> +#define FL_ARCH8      (1 << 24)       /* Architecture 8.  */
> +#define FL_CRC32      (1 << 25)	      /* ARMv8 CRC32 instructions.  */
> +
> +#define FL_SMALLMUL   (1 << 26)       /* Small multiply supported.  */
> +#define FL_NO_VOLATILE_CE   (1 << 27) /* No volatile memory in IT block.  */
> +
> +#define FL_IWMMXT     (1 << 29)	      /* XScale v2 or "Intel Wireless MMX technology".  */
> +#define FL_IWMMXT2    (1 << 30)       /* "Intel Wireless MMX2 technology".  */
> +
> +/* Flags that only effect tuning, not available instructions.  */
> +#define FL_TUNE		(FL_WBUF | FL_VFPV2 | FL_STRONG | FL_LDSCHED \
> +			 | FL_CO_PROC)
> +
> +#define FL_FOR_ARCH2	FL_NOTM
> +#define FL_FOR_ARCH3	(FL_FOR_ARCH2 | FL_MODE32)
> +#define FL_FOR_ARCH3M	(FL_FOR_ARCH3 | FL_ARCH3M)
> +#define FL_FOR_ARCH4	(FL_FOR_ARCH3M | FL_ARCH4)
> +#define FL_FOR_ARCH4T	(FL_FOR_ARCH4 | FL_THUMB)
> +#define FL_FOR_ARCH5	(FL_FOR_ARCH4 | FL_ARCH5)
> +#define FL_FOR_ARCH5T	(FL_FOR_ARCH5 | FL_THUMB)
> +#define FL_FOR_ARCH5E	(FL_FOR_ARCH5 | FL_ARCH5E)
> +#define FL_FOR_ARCH5TE	(FL_FOR_ARCH5E | FL_THUMB)
> +#define FL_FOR_ARCH5TEJ	FL_FOR_ARCH5TE
> +#define FL_FOR_ARCH6	(FL_FOR_ARCH5TE | FL_ARCH6)
> +#define FL_FOR_ARCH6J	FL_FOR_ARCH6
> +#define FL_FOR_ARCH6K	(FL_FOR_ARCH6 | FL_ARCH6K)
> +#define FL_FOR_ARCH6Z	FL_FOR_ARCH6
> +#define FL_FOR_ARCH6ZK	FL_FOR_ARCH6K
> +#define FL_FOR_ARCH6T2	(FL_FOR_ARCH6 | FL_THUMB2)
> +#define FL_FOR_ARCH6M	(FL_FOR_ARCH6 & ~FL_NOTM)
> +#define FL_FOR_ARCH7	((FL_FOR_ARCH6T2 & ~FL_NOTM) | FL_ARCH7)
> +#define FL_FOR_ARCH7A	(FL_FOR_ARCH7 | FL_NOTM | FL_ARCH6K)
> +#define FL_FOR_ARCH7VE	(FL_FOR_ARCH7A | FL_THUMB_DIV | FL_ARM_DIV)
> +#define FL_FOR_ARCH7R	(FL_FOR_ARCH7A | FL_THUMB_DIV)
> +#define FL_FOR_ARCH7M	(FL_FOR_ARCH7 | FL_THUMB_DIV)
> +#define FL_FOR_ARCH7EM  (FL_FOR_ARCH7M | FL_ARCH7EM)
> +#define FL_FOR_ARCH8A	(FL_FOR_ARCH7VE | FL_ARCH8)
> +
> +#endif /* GCC_ARM_FLAGS_H */
> diff --git a/gcc/config/arm/arm-opts.h b/gcc/config/arm/arm-opts.h
> index 039e333..dd727cf 100644
> --- a/gcc/config/arm/arm-opts.h
> +++ b/gcc/config/arm/arm-opts.h
> @@ -25,6 +25,8 @@
>  #ifndef ARM_OPTS_H
>  #define ARM_OPTS_H
>  
> +#include "arm-flags.h"
> +
>  /* The various ARM cores.  */
>  enum processor_type
>  {
> @@ -77,4 +79,25 @@ enum arm_tls_type {
>    TLS_GNU,
>    TLS_GNU2
>  };
> +
> +struct arm_arch_core_flag
> +{
> +  const char *const name;
> +  const unsigned long flags;
> +};
> +
> +static const struct arm_arch_core_flag arm_arch_core_flags[] =
> +{
> +#undef ARM_CORE
> +#define ARM_CORE(NAME, X, IDENT, ARCH, FLAGS, COSTS) \
> +  {NAME, FLAGS | FL_FOR_ARCH##ARCH},
> +#include "arm-cores.def"
> +#undef ARM_CORE
> +#undef ARM_ARCH
> +#define ARM_ARCH(NAME, CORE, ARCH, FLAGS) \
> +  {NAME, FLAGS},
> +#include "arm-arches.def"
> +#undef ARM_ARCH
> +  {NULL, 0}
> +};

Did you consider implications from mixing ARCHes and CPUs in the same array?  It should not be a problem, but would you please double-check that cases like "-march=cortex-a15" are properly caught as errors elsewhere in the driver?

>  #endif
> diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
> index 28ffe52..325a81c 100644
> --- a/gcc/config/arm/arm-protos.h
> +++ b/gcc/config/arm/arm-protos.h
> @@ -325,75 +325,6 @@ extern const char *arm_rewrite_selected_cpu (const char *name);
>  
>  extern bool arm_is_constant_pool_ref (rtx);
>  
> -/* Flags used to identify the presence of processor capabilities.  */

You've lost this comment in the new file.  Was it intentional?

> -
> -/* Bit values used to identify processor capabilities.  */
> -#define FL_CO_PROC    (1 << 0)        /* Has external co-processor bus */
> -#define FL_ARCH3M     (1 << 1)        /* Extended multiply */
> -#define FL_MODE26     (1 << 2)        /* 26-bit mode support */
> -#define FL_MODE32     (1 << 3)        /* 32-bit mode support */
> -#define FL_ARCH4      (1 << 4)        /* Architecture rel 4 */
> -#define FL_ARCH5      (1 << 5)        /* Architecture rel 5 */
> -#define FL_THUMB      (1 << 6)        /* Thumb aware */
> -#define FL_LDSCHED    (1 << 7)	      /* Load scheduling necessary */
> -#define FL_STRONG     (1 << 8)	      /* StrongARM */
> -#define FL_ARCH5E     (1 << 9)        /* DSP extensions to v5 */
> -#define FL_XSCALE     (1 << 10)	      /* XScale */
> -/* spare	      (1 << 11)	*/
> -#define FL_ARCH6      (1 << 12)       /* Architecture rel 6.  Adds
> -					 media instructions.  */
> -#define FL_VFPV2      (1 << 13)       /* Vector Floating Point V2.  */
> -#define FL_WBUF	      (1 << 14)	      /* Schedule for write buffer ops.
> -					 Note: ARM6 & 7 derivatives only.  */
> -#define FL_ARCH6K     (1 << 15)       /* Architecture rel 6 K extensions.  */
> -#define FL_THUMB2     (1 << 16)	      /* Thumb-2.  */
> -#define FL_NOTM	      (1 << 17)	      /* Instructions not present in the 'M'
> -					 profile.  */
> -#define FL_THUMB_DIV  (1 << 18)	      /* Hardware divide (Thumb mode).  */
> -#define FL_VFPV3      (1 << 19)       /* Vector Floating Point V3.  */
> -#define FL_NEON       (1 << 20)       /* Neon instructions.  */
> -#define FL_ARCH7EM    (1 << 21)	      /* Instructions present in the ARMv7E-M
> -					 architecture.  */
> -#define FL_ARCH7      (1 << 22)       /* Architecture 7.  */
> -#define FL_ARM_DIV    (1 << 23)	      /* Hardware divide (ARM mode).  */
> -#define FL_ARCH8      (1 << 24)       /* Architecture 8.  */
> -#define FL_CRC32      (1 << 25)	      /* ARMv8 CRC32 instructions.  */
> -
> -#define FL_SMALLMUL   (1 << 26)       /* Small multiply supported.  */
> -#define FL_NO_VOLATILE_CE   (1 << 27) /* No volatile memory in IT block.  */
> -
> -#define FL_IWMMXT     (1 << 29)	      /* XScale v2 or "Intel Wireless MMX technology".  */
> -#define FL_IWMMXT2    (1 << 30)       /* "Intel Wireless MMX2 technology".  */
> -
> -/* Flags that only effect tuning, not available instructions.  */
> -#define FL_TUNE		(FL_WBUF | FL_VFPV2 | FL_STRONG | FL_LDSCHED \
> -			 | FL_CO_PROC)
> -
> -#define FL_FOR_ARCH2	FL_NOTM
> -#define FL_FOR_ARCH3	(FL_FOR_ARCH2 | FL_MODE32)
> -#define FL_FOR_ARCH3M	(FL_FOR_ARCH3 | FL_ARCH3M)
> -#define FL_FOR_ARCH4	(FL_FOR_ARCH3M | FL_ARCH4)
> -#define FL_FOR_ARCH4T	(FL_FOR_ARCH4 | FL_THUMB)
> -#define FL_FOR_ARCH5	(FL_FOR_ARCH4 | FL_ARCH5)
> -#define FL_FOR_ARCH5T	(FL_FOR_ARCH5 | FL_THUMB)
> -#define FL_FOR_ARCH5E	(FL_FOR_ARCH5 | FL_ARCH5E)
> -#define FL_FOR_ARCH5TE	(FL_FOR_ARCH5E | FL_THUMB)
> -#define FL_FOR_ARCH5TEJ	FL_FOR_ARCH5TE
> -#define FL_FOR_ARCH6	(FL_FOR_ARCH5TE | FL_ARCH6)
> -#define FL_FOR_ARCH6J	FL_FOR_ARCH6
> -#define FL_FOR_ARCH6K	(FL_FOR_ARCH6 | FL_ARCH6K)
> -#define FL_FOR_ARCH6Z	FL_FOR_ARCH6
> -#define FL_FOR_ARCH6ZK	FL_FOR_ARCH6K
> -#define FL_FOR_ARCH6T2	(FL_FOR_ARCH6 | FL_THUMB2)
> -#define FL_FOR_ARCH6M	(FL_FOR_ARCH6 & ~FL_NOTM)
> -#define FL_FOR_ARCH7	((FL_FOR_ARCH6T2 & ~FL_NOTM) | FL_ARCH7)
> -#define FL_FOR_ARCH7A	(FL_FOR_ARCH7 | FL_NOTM | FL_ARCH6K)
> -#define FL_FOR_ARCH7VE	(FL_FOR_ARCH7A | FL_THUMB_DIV | FL_ARM_DIV)
> -#define FL_FOR_ARCH7R	(FL_FOR_ARCH7A | FL_THUMB_DIV)
> -#define FL_FOR_ARCH7M	(FL_FOR_ARCH7 | FL_THUMB_DIV)
> -#define FL_FOR_ARCH7EM  (FL_FOR_ARCH7M | FL_ARCH7EM)
> -#define FL_FOR_ARCH8A	(FL_FOR_ARCH7VE | FL_ARCH8)
> -

Would you please explicitly include "arm-flags.h" into arm-protos.h?  There is an effort underway to flatten header files (make sure that no header includes another header), so it is best to minimize implicit includes.

>  /* The bits in this mask specify which
>     instructions we are allowed to generate.  */
>  extern unsigned long insn_flags;
> diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
> index 8c10ea3..3119957 100644
> --- a/gcc/config/arm/arm.h
> +++ b/gcc/config/arm/arm.h
> @@ -2394,13 +2394,18 @@ extern const char *arm_rewrite_mcpu (int argc, const char **argv);
>     "   :%{march=*:-march=%*}}"					\
>     BIG_LITTLE_SPEC
>  
> +extern const char *arm_is_target_thumb_only (int argc, const char **argv);
> +#define MODE_SET_SPEC_FUNCTIONS						\
> +  { "target_mode_check", arm_is_target_thumb_only },

Consider renaming to TARGET_MODE_SPEC_FUNCTIONS or THUMBONLY_SPEC_FUNCTIONS.  Current name sounds like "set spec functions for a given mode", while what it does is "these are spec functions for setting mode".

> +
>  /* -mcpu=native handling only makes sense with compiler running on
>     an ARM chip.  */
>  #if defined(__arm__)
>  extern const char *host_detect_local_cpu (int argc, const char **argv);
>  # define EXTRA_SPEC_FUNCTIONS						\
>    { "local_cpu_detect", host_detect_local_cpu },			\
> -  BIG_LITTLE_CPU_SPEC_FUNCTIONS
> +  BIG_LITTLE_CPU_SPEC_FUNCTIONS						\
> +  MODE_SET_SPEC_FUNCTIONS
>  
>  # define MCPU_MTUNE_NATIVE_SPECS					\
>     " %{march=native:%<march=native %:local_cpu_detect(arch)}"		\
> @@ -2408,9 +2413,19 @@ extern const char *host_detect_local_cpu (int argc, const char **argv);
>     " %{mtune=native:%<mtune=native %:local_cpu_detect(tune)}"
>  #else
>  # define MCPU_MTUNE_NATIVE_SPECS ""
> -# define EXTRA_SPEC_FUNCTIONS BIG_LITTLE_CPU_SPEC_FUNCTIONS
> +# define EXTRA_SPEC_FUNCTIONS						\
> +  BIG_LITTLE_CPU_SPEC_FUNCTIONS						\
> +  MODE_SET_SPEC_FUNCTIONS
>  #endif
>  
> -#define DRIVER_SELF_SPECS MCPU_MTUNE_NATIVE_SPECS
> +/* Automatically add -mthumb for thumb-only target if mode isn't specified
> +   via configuration option --with-mode and command line.
> +   If -march present, we collect just -march options.  Otherwise we
> +   collect just -mcpu options.  The last one of them will be used to
> +   decide target mode.  */
> +#define MODE_SET_SPECS							\
> +  " %{!marm:%{!mthumb:%:target_mode_check(%{march=*:%*;mcpu=*:%*})}}"

Again, consider renaming to "TARGET_MODE_SPECS" or THUMBONLY_SPEC_FUNCTIONS.

> +
> +#define DRIVER_SELF_SPECS MCPU_MTUNE_NATIVE_SPECS MODE_SET_SPECS
>  #define TARGET_SUPPORTS_WIDE_INT 1
>  #endif /* ! GCC_ARM_H */
> 

Thanks,

--
Maxim Kuvyrkov
www.linaro.org

Re: [PATCH][ARM]Automatically add -mthumb for thumb-only target when mode isn't specified

[Terry Guo]

[-- Attachment #1: Type: text/plain, Size: 2910 bytes --]

>
> Thanks Terry (and everyone else) for explaining why we want to do this in the driver.  The substance of the patch looks good to me, and below are some comments and nit-picks.  (Also, I'm not an ARM maintainer, so this is a review, not an approval to commit).
>
> Please make sure to update changelog before committing.
>

Thanks Maxim, your comments are great. I accepted all of them and
commented two of them that I am not clear.

<<snapped>>
>> +
>> +struct arm_arch_core_flag
>> +{
>> +  const char *const name;
>> +  const unsigned long flags;
>> +};
>> +
>> +static const struct arm_arch_core_flag arm_arch_core_flags[] =
>> +{
>> +#undef ARM_CORE
>> +#define ARM_CORE(NAME, X, IDENT, ARCH, FLAGS, COSTS) \
>> +  {NAME, FLAGS | FL_FOR_ARCH##ARCH},
>> +#include "arm-cores.def"
>> +#undef ARM_CORE
>> +#undef ARM_ARCH
>> +#define ARM_ARCH(NAME, CORE, ARCH, FLAGS) \
>> +  {NAME, FLAGS},
>> +#include "arm-arches.def"
>> +#undef ARM_ARCH
>> +  {NULL, 0}
>> +};
>
> Did you consider implications from mixing ARCHes and CPUs in the same array?  It should not be a problem, but would you please double-check that cases like "-march=cortex-a15" are properly caught as errors elsewhere in the driver?
>

Not sure I follow you correctly here. This array is just used for my
new arm_target_thumb_only function. It isn't used by any another code
in gcc. So I don't think mixing them will break gcc option check
mechanism. I tried below command and I can get error message:

$ ./install-native/bin/arm-none-eabi-gcc -march=cortex-a15 x.c -S -mthumb
arm-none-eabi-gcc: error: unrecognized argument in option '-march=cortex-a15'

>>  #endif
>> diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
>> index 28ffe52..325a81c 100644
>> --- a/gcc/config/arm/arm-protos.h
>> +++ b/gcc/config/arm/arm-protos.h
>> @@ -325,75 +325,6 @@ extern const char *arm_rewrite_selected_cpu (const char *name);
>>
>>  extern bool arm_is_constant_pool_ref (rtx);
>>
>> -/* Flags used to identify the presence of processor capabilities.  */
>
> You've lost this comment in the new file.  Was it intentional?
>

The line is used as first line in new file arm-flags.h.

BR,
Terry

Here is updated ChangeLog:

gcc/ChangeLog:

2015-03-05  Terry Guo  <terry.guo@arm.com>

      * common/config/arm/arm-common.c (arm_target_thumb_only): New function.
      * config/arm/arm-protos.h: Move FL_* stuff into below file and
then include it.
      * config/arm/arm-flags.h: New file for FL_* stuff.
      * config/arm/arm-opts.h (struct arm_arch_core_flag): New struct.
      (arm_arch_core_flags): New array for arch/core and flag map.
      * config/arm/arm.h (TARGET_MODE_SPEC_FUNCTIONS): New SPEC function.
      (EXTRA_SPEC_FUNCTIONS): Include new SPEC function.
      (TARGET_MODE_SPECS): New SPEC.
      (DRIVER_SELF_SPECS): Include new SPEC.

[-- Attachment #2: gcc-mthumb-option-v7.txt --]
[-- Type: text/plain, Size: 12672 bytes --]

diff --git a/gcc/common/config/arm/arm-common.c b/gcc/common/config/arm/arm-common.c
index 86673b7..5efb55e 100644
--- a/gcc/common/config/arm/arm-common.c
+++ b/gcc/common/config/arm/arm-common.c
@@ -97,6 +97,28 @@ arm_rewrite_mcpu (int argc, const char **argv)
   return arm_rewrite_selected_cpu (argv[argc - 1]);
 }
 
+/* Called by driver to check whether the target denoted by current
+   command line options is thumb-only target.  If -march present,
+   check the last -march option.  If no -march, check the last -mcpu
+   option.  */
+const char *
+arm_target_thumb_only (int argc, const char **argv)
+{
+  unsigned int opt;
+
+  if (argc)
+    {
+      for (opt = 0; opt < (ARRAY_SIZE (arm_arch_core_flags) - 1); opt++)
+	if ((strcmp (argv[argc - 1], arm_arch_core_flags[opt].name) == 0)
+	    && ((arm_arch_core_flags[opt].flags & FL_NOTM) == 0))
+	  return "-mthumb";
+
+      return NULL;
+    }
+  else
+    return NULL;
+}
+
 #undef ARM_CPU_NAME_LENGTH
 
 
diff --git a/gcc/config/arm/arm-flags.h b/gcc/config/arm/arm-flags.h
new file mode 100644
index 0000000..e206ad1
--- /dev/null
+++ b/gcc/config/arm/arm-flags.h
@@ -0,0 +1,92 @@
+/* Flags used to identify the presence of processor capabilities.
+
+   Copyright (C) 2015 Free Software Foundation, Inc.
+   Contributed by ARM Ltd.
+
+   This file is part of GCC.
+
+   GCC is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published
+   by the Free Software Foundation; either version 3, or (at your
+   option) any later version.
+
+   GCC is distributed in the hope that it will be useful, but WITHOUT
+   ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+   or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
+   License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with GCC; see the file COPYING3.  If not see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef GCC_ARM_FLAGS_H
+#define GCC_ARM_FLAGS_H
+
+/* Bit values used to identify processor capabilities.  */
+#define FL_CO_PROC    (1 << 0)        /* Has external co-processor bus */
+#define FL_ARCH3M     (1 << 1)        /* Extended multiply */
+#define FL_MODE26     (1 << 2)        /* 26-bit mode support */
+#define FL_MODE32     (1 << 3)        /* 32-bit mode support */
+#define FL_ARCH4      (1 << 4)        /* Architecture rel 4 */
+#define FL_ARCH5      (1 << 5)        /* Architecture rel 5 */
+#define FL_THUMB      (1 << 6)        /* Thumb aware */
+#define FL_LDSCHED    (1 << 7)	      /* Load scheduling necessary */
+#define FL_STRONG     (1 << 8)	      /* StrongARM */
+#define FL_ARCH5E     (1 << 9)        /* DSP extensions to v5 */
+#define FL_XSCALE     (1 << 10)	      /* XScale */
+/* spare	      (1 << 11)	*/
+#define FL_ARCH6      (1 << 12)       /* Architecture rel 6.  Adds
+					 media instructions.  */
+#define FL_VFPV2      (1 << 13)       /* Vector Floating Point V2.  */
+#define FL_WBUF	      (1 << 14)	      /* Schedule for write buffer ops.
+					 Note: ARM6 & 7 derivatives only.  */
+#define FL_ARCH6K     (1 << 15)       /* Architecture rel 6 K extensions.  */
+#define FL_THUMB2     (1 << 16)	      /* Thumb-2.  */
+#define FL_NOTM	      (1 << 17)	      /* Instructions not present in the 'M'
+					 profile.  */
+#define FL_THUMB_DIV  (1 << 18)	      /* Hardware divide (Thumb mode).  */
+#define FL_VFPV3      (1 << 19)       /* Vector Floating Point V3.  */
+#define FL_NEON       (1 << 20)       /* Neon instructions.  */
+#define FL_ARCH7EM    (1 << 21)	      /* Instructions present in the ARMv7E-M
+					 architecture.  */
+#define FL_ARCH7      (1 << 22)       /* Architecture 7.  */
+#define FL_ARM_DIV    (1 << 23)	      /* Hardware divide (ARM mode).  */
+#define FL_ARCH8      (1 << 24)       /* Architecture 8.  */
+#define FL_CRC32      (1 << 25)	      /* ARMv8 CRC32 instructions.  */
+
+#define FL_SMALLMUL   (1 << 26)       /* Small multiply supported.  */
+#define FL_NO_VOLATILE_CE   (1 << 27) /* No volatile memory in IT block.  */
+
+#define FL_IWMMXT     (1 << 29)	      /* XScale v2 or "Intel Wireless MMX technology".  */
+#define FL_IWMMXT2    (1 << 30)       /* "Intel Wireless MMX2 technology".  */
+
+/* Flags that only effect tuning, not available instructions.  */
+#define FL_TUNE		(FL_WBUF | FL_VFPV2 | FL_STRONG | FL_LDSCHED \
+			 | FL_CO_PROC)
+
+#define FL_FOR_ARCH2	FL_NOTM
+#define FL_FOR_ARCH3	(FL_FOR_ARCH2 | FL_MODE32)
+#define FL_FOR_ARCH3M	(FL_FOR_ARCH3 | FL_ARCH3M)
+#define FL_FOR_ARCH4	(FL_FOR_ARCH3M | FL_ARCH4)
+#define FL_FOR_ARCH4T	(FL_FOR_ARCH4 | FL_THUMB)
+#define FL_FOR_ARCH5	(FL_FOR_ARCH4 | FL_ARCH5)
+#define FL_FOR_ARCH5T	(FL_FOR_ARCH5 | FL_THUMB)
+#define FL_FOR_ARCH5E	(FL_FOR_ARCH5 | FL_ARCH5E)
+#define FL_FOR_ARCH5TE	(FL_FOR_ARCH5E | FL_THUMB)
+#define FL_FOR_ARCH5TEJ	FL_FOR_ARCH5TE
+#define FL_FOR_ARCH6	(FL_FOR_ARCH5TE | FL_ARCH6)
+#define FL_FOR_ARCH6J	FL_FOR_ARCH6
+#define FL_FOR_ARCH6K	(FL_FOR_ARCH6 | FL_ARCH6K)
+#define FL_FOR_ARCH6Z	FL_FOR_ARCH6
+#define FL_FOR_ARCH6ZK	FL_FOR_ARCH6K
+#define FL_FOR_ARCH6T2	(FL_FOR_ARCH6 | FL_THUMB2)
+#define FL_FOR_ARCH6M	(FL_FOR_ARCH6 & ~FL_NOTM)
+#define FL_FOR_ARCH7	((FL_FOR_ARCH6T2 & ~FL_NOTM) | FL_ARCH7)
+#define FL_FOR_ARCH7A	(FL_FOR_ARCH7 | FL_NOTM | FL_ARCH6K)
+#define FL_FOR_ARCH7VE	(FL_FOR_ARCH7A | FL_THUMB_DIV | FL_ARM_DIV)
+#define FL_FOR_ARCH7R	(FL_FOR_ARCH7A | FL_THUMB_DIV)
+#define FL_FOR_ARCH7M	(FL_FOR_ARCH7 | FL_THUMB_DIV)
+#define FL_FOR_ARCH7EM  (FL_FOR_ARCH7M | FL_ARCH7EM)
+#define FL_FOR_ARCH8A	(FL_FOR_ARCH7VE | FL_ARCH8)
+
+#endif /* GCC_ARM_FLAGS_H */
diff --git a/gcc/config/arm/arm-opts.h b/gcc/config/arm/arm-opts.h
index 039e333..dd727cf 100644
--- a/gcc/config/arm/arm-opts.h
+++ b/gcc/config/arm/arm-opts.h
@@ -25,6 +25,8 @@
 #ifndef ARM_OPTS_H
 #define ARM_OPTS_H
 
+#include "arm-flags.h"
+
 /* The various ARM cores.  */
 enum processor_type
 {
@@ -77,4 +79,25 @@ enum arm_tls_type {
   TLS_GNU,
   TLS_GNU2
 };
+
+struct arm_arch_core_flag
+{
+  const char *const name;
+  const unsigned long flags;
+};
+
+static const struct arm_arch_core_flag arm_arch_core_flags[] =
+{
+#undef ARM_CORE
+#define ARM_CORE(NAME, X, IDENT, ARCH, FLAGS, COSTS) \
+  {NAME, FLAGS | FL_FOR_ARCH##ARCH},
+#include "arm-cores.def"
+#undef ARM_CORE
+#undef ARM_ARCH
+#define ARM_ARCH(NAME, CORE, ARCH, FLAGS) \
+  {NAME, FLAGS},
+#include "arm-arches.def"
+#undef ARM_ARCH
+  {NULL, 0}
+};
 #endif
diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
index 28ffe52..0d5f61f 100644
--- a/gcc/config/arm/arm-protos.h
+++ b/gcc/config/arm/arm-protos.h
@@ -22,6 +22,8 @@
 #ifndef GCC_ARM_PROTOS_H
 #define GCC_ARM_PROTOS_H
 
+#include "arm-flags.h"
+
 extern enum unwind_info_type arm_except_unwind_info (struct gcc_options *);
 extern int use_return_insn (int, rtx);
 extern bool use_simple_return_p (void);
@@ -325,75 +327,6 @@ extern const char *arm_rewrite_selected_cpu (const char *name);
 
 extern bool arm_is_constant_pool_ref (rtx);
 
-/* Flags used to identify the presence of processor capabilities.  */
-
-/* Bit values used to identify processor capabilities.  */
-#define FL_CO_PROC    (1 << 0)        /* Has external co-processor bus */
-#define FL_ARCH3M     (1 << 1)        /* Extended multiply */
-#define FL_MODE26     (1 << 2)        /* 26-bit mode support */
-#define FL_MODE32     (1 << 3)        /* 32-bit mode support */
-#define FL_ARCH4      (1 << 4)        /* Architecture rel 4 */
-#define FL_ARCH5      (1 << 5)        /* Architecture rel 5 */
-#define FL_THUMB      (1 << 6)        /* Thumb aware */
-#define FL_LDSCHED    (1 << 7)	      /* Load scheduling necessary */
-#define FL_STRONG     (1 << 8)	      /* StrongARM */
-#define FL_ARCH5E     (1 << 9)        /* DSP extensions to v5 */
-#define FL_XSCALE     (1 << 10)	      /* XScale */
-/* spare	      (1 << 11)	*/
-#define FL_ARCH6      (1 << 12)       /* Architecture rel 6.  Adds
-					 media instructions.  */
-#define FL_VFPV2      (1 << 13)       /* Vector Floating Point V2.  */
-#define FL_WBUF	      (1 << 14)	      /* Schedule for write buffer ops.
-					 Note: ARM6 & 7 derivatives only.  */
-#define FL_ARCH6K     (1 << 15)       /* Architecture rel 6 K extensions.  */
-#define FL_THUMB2     (1 << 16)	      /* Thumb-2.  */
-#define FL_NOTM	      (1 << 17)	      /* Instructions not present in the 'M'
-					 profile.  */
-#define FL_THUMB_DIV  (1 << 18)	      /* Hardware divide (Thumb mode).  */
-#define FL_VFPV3      (1 << 19)       /* Vector Floating Point V3.  */
-#define FL_NEON       (1 << 20)       /* Neon instructions.  */
-#define FL_ARCH7EM    (1 << 21)	      /* Instructions present in the ARMv7E-M
-					 architecture.  */
-#define FL_ARCH7      (1 << 22)       /* Architecture 7.  */
-#define FL_ARM_DIV    (1 << 23)	      /* Hardware divide (ARM mode).  */
-#define FL_ARCH8      (1 << 24)       /* Architecture 8.  */
-#define FL_CRC32      (1 << 25)	      /* ARMv8 CRC32 instructions.  */
-
-#define FL_SMALLMUL   (1 << 26)       /* Small multiply supported.  */
-#define FL_NO_VOLATILE_CE   (1 << 27) /* No volatile memory in IT block.  */
-
-#define FL_IWMMXT     (1 << 29)	      /* XScale v2 or "Intel Wireless MMX technology".  */
-#define FL_IWMMXT2    (1 << 30)       /* "Intel Wireless MMX2 technology".  */
-
-/* Flags that only effect tuning, not available instructions.  */
-#define FL_TUNE		(FL_WBUF | FL_VFPV2 | FL_STRONG | FL_LDSCHED \
-			 | FL_CO_PROC)
-
-#define FL_FOR_ARCH2	FL_NOTM
-#define FL_FOR_ARCH3	(FL_FOR_ARCH2 | FL_MODE32)
-#define FL_FOR_ARCH3M	(FL_FOR_ARCH3 | FL_ARCH3M)
-#define FL_FOR_ARCH4	(FL_FOR_ARCH3M | FL_ARCH4)
-#define FL_FOR_ARCH4T	(FL_FOR_ARCH4 | FL_THUMB)
-#define FL_FOR_ARCH5	(FL_FOR_ARCH4 | FL_ARCH5)
-#define FL_FOR_ARCH5T	(FL_FOR_ARCH5 | FL_THUMB)
-#define FL_FOR_ARCH5E	(FL_FOR_ARCH5 | FL_ARCH5E)
-#define FL_FOR_ARCH5TE	(FL_FOR_ARCH5E | FL_THUMB)
-#define FL_FOR_ARCH5TEJ	FL_FOR_ARCH5TE
-#define FL_FOR_ARCH6	(FL_FOR_ARCH5TE | FL_ARCH6)
-#define FL_FOR_ARCH6J	FL_FOR_ARCH6
-#define FL_FOR_ARCH6K	(FL_FOR_ARCH6 | FL_ARCH6K)
-#define FL_FOR_ARCH6Z	FL_FOR_ARCH6
-#define FL_FOR_ARCH6ZK	FL_FOR_ARCH6K
-#define FL_FOR_ARCH6T2	(FL_FOR_ARCH6 | FL_THUMB2)
-#define FL_FOR_ARCH6M	(FL_FOR_ARCH6 & ~FL_NOTM)
-#define FL_FOR_ARCH7	((FL_FOR_ARCH6T2 & ~FL_NOTM) | FL_ARCH7)
-#define FL_FOR_ARCH7A	(FL_FOR_ARCH7 | FL_NOTM | FL_ARCH6K)
-#define FL_FOR_ARCH7VE	(FL_FOR_ARCH7A | FL_THUMB_DIV | FL_ARM_DIV)
-#define FL_FOR_ARCH7R	(FL_FOR_ARCH7A | FL_THUMB_DIV)
-#define FL_FOR_ARCH7M	(FL_FOR_ARCH7 | FL_THUMB_DIV)
-#define FL_FOR_ARCH7EM  (FL_FOR_ARCH7M | FL_ARCH7EM)
-#define FL_FOR_ARCH8A	(FL_FOR_ARCH7VE | FL_ARCH8)
-
 /* The bits in this mask specify which
    instructions we are allowed to generate.  */
 extern unsigned long insn_flags;
diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index 8c10ea3..6a2dfaf 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -2394,13 +2394,18 @@ extern const char *arm_rewrite_mcpu (int argc, const char **argv);
    "   :%{march=*:-march=%*}}"					\
    BIG_LITTLE_SPEC
 
+extern const char *arm_target_thumb_only (int argc, const char **argv);
+#define TARGET_MODE_SPEC_FUNCTIONS					\
+  { "target_mode_check", arm_target_thumb_only },
+
 /* -mcpu=native handling only makes sense with compiler running on
    an ARM chip.  */
 #if defined(__arm__)
 extern const char *host_detect_local_cpu (int argc, const char **argv);
 # define EXTRA_SPEC_FUNCTIONS						\
   { "local_cpu_detect", host_detect_local_cpu },			\
-  BIG_LITTLE_CPU_SPEC_FUNCTIONS
+  BIG_LITTLE_CPU_SPEC_FUNCTIONS						\
+  TARGET_MODE_SPEC_FUNCTIONS
 
 # define MCPU_MTUNE_NATIVE_SPECS					\
    " %{march=native:%<march=native %:local_cpu_detect(arch)}"		\
@@ -2408,9 +2413,19 @@ extern const char *host_detect_local_cpu (int argc, const char **argv);
    " %{mtune=native:%<mtune=native %:local_cpu_detect(tune)}"
 #else
 # define MCPU_MTUNE_NATIVE_SPECS ""
-# define EXTRA_SPEC_FUNCTIONS BIG_LITTLE_CPU_SPEC_FUNCTIONS
+# define EXTRA_SPEC_FUNCTIONS						\
+  BIG_LITTLE_CPU_SPEC_FUNCTIONS						\
+  TARGET_MODE_SPEC_FUNCTIONS
 #endif
 
-#define DRIVER_SELF_SPECS MCPU_MTUNE_NATIVE_SPECS
+/* Automatically add -mthumb for thumb-only target if mode isn't specified
+   via configuration option --with-mode and command line.
+   If -march present, we collect just -march options.  Otherwise we
+   collect just -mcpu options.  The last one of them will be used to
+   decide target mode.  */
+#define TARGET_MODE_SPECS							\
+  " %{!marm:%{!mthumb:%:target_mode_check(%{march=*:%*;mcpu=*:%*})}}"
+
+#define DRIVER_SELF_SPECS MCPU_MTUNE_NATIVE_SPECS TARGET_MODE_SPECS
 #define TARGET_SUPPORTS_WIDE_INT 1
 #endif /* ! GCC_ARM_H */

Re: [PATCH][ARM]Automatically add -mthumb for thumb-only target when mode isn't specified

[Maxim Kuvyrkov]

> On Mar 5, 2015, at 9:14 AM, Terry Guo <flameroc@gmail.com> wrote:
> 
>> 
>> Thanks Terry (and everyone else) for explaining why we want to do this in the driver.  The substance of the patch looks good to me, and below are some comments and nit-picks.  (Also, I'm not an ARM maintainer, so this is a review, not an approval to commit).
>> 
>> Please make sure to update changelog before committing.
>> 
> 
> Thanks Maxim, your comments are great. I accepted all of them and
> commented two of them that I am not clear.
> 
> <<snapped>>
>>> +
>>> +struct arm_arch_core_flag
>>> +{
>>> +  const char *const name;
>>> +  const unsigned long flags;
>>> +};
>>> +
>>> +static const struct arm_arch_core_flag arm_arch_core_flags[] =
>>> +{
>>> +#undef ARM_CORE
>>> +#define ARM_CORE(NAME, X, IDENT, ARCH, FLAGS, COSTS) \
>>> +  {NAME, FLAGS | FL_FOR_ARCH##ARCH},
>>> +#include "arm-cores.def"
>>> +#undef ARM_CORE
>>> +#undef ARM_ARCH
>>> +#define ARM_ARCH(NAME, CORE, ARCH, FLAGS) \
>>> +  {NAME, FLAGS},
>>> +#include "arm-arches.def"
>>> +#undef ARM_ARCH
>>> +  {NULL, 0}
>>> +};
>> 
>> Did you consider implications from mixing ARCHes and CPUs in the same array?  It should not be a problem, but would you please double-check that cases like "-march=cortex-a15" are properly caught as errors elsewhere in the driver?
>> 
> 
> Not sure I follow you correctly here. This array is just used for my
> new arm_target_thumb_only function. It isn't used by any another code
> in gcc. So I don't think mixing them will break gcc option check
> mechanism. I tried below command and I can get error message:
> 
> $ ./install-native/bin/arm-none-eabi-gcc -march=cortex-a15 x.c -S -mthumb
> arm-none-eabi-gcc: error: unrecognized argument in option '-march=cortex-a15'

I'm just being paranoid here.  The above check you did is all I wanted.

> 
>>> #endif
>>> diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
>>> index 28ffe52..325a81c 100644
>>> --- a/gcc/config/arm/arm-protos.h
>>> +++ b/gcc/config/arm/arm-protos.h
>>> @@ -325,75 +325,6 @@ extern const char *arm_rewrite_selected_cpu (const char *name);
>>> 
>>> extern bool arm_is_constant_pool_ref (rtx);
>>> 
>>> -/* Flags used to identify the presence of processor capabilities.  */
>> 
>> You've lost this comment in the new file.  Was it intentional?
>> 
> 
> The line is used as first line in new file arm-flags.h.

Ack.

Thanks,

--
Maxim Kuvyrkov
www.linaro.org