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

[Terry Guo <flameroc@gmail.com>]

[-- 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 */