| Message ID | 6455763.xNPUBGU9Kt@e108577-lin |
|---|---|
| State | New |
| Headers | show |
Ping? Best regards, Thomas On Tuesday 10 May 2016 14:26:04 Thomas Preudhomme wrote: > Hi, > > ARM_ARCH_ISA_THUMB is currently set to 1 when compiling for armv5 despite > armv5 not supporting Thumb instructions (armv5t does): > > arm-none-eabi-gcc -dM -march=armv5 -E - < /dev/null | grep ISA_THUMB > #define __ARM_ARCH_ISA_THUMB 1 > > The reason is TARGET_ARM_ARCH_ISA_THUMB being set to 1 if target does not > support Thumb-2 and is ARMv4T, ARMv5 or later. This patch replaces that > logic by checking whether the given architecture has the right feature bit > (FL_THUMB). > > ChangeLog entry is as follows: > > > *** gcc/ChangeLog *** > > 2016-05-06 Thomas Preud'homme <thomas.preudhomme@arm.com> > > * config/arm/arm-protos.h (arm_arch_thumb): Declare. > * config/arm/arm.c (arm_arch_thumb): Define. > (arm_option_override): Initialize arm_arch_thumb. > * config/arm/arm.h (TARGET_ARM_ARCH_ISA_THUMB): Use arm_arch_thumb > to determine if target support Thumb-1 ISA. > > > diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h > index > d8179c441bb53dced94d2ebf497aad093e4ac600..4d11c91133ff1b875afcbf58abc4491c2c > 93768e 100644 > --- a/gcc/config/arm/arm-protos.h > +++ b/gcc/config/arm/arm-protos.h > @@ -603,6 +603,9 @@ extern int arm_tune_cortex_a9; > interworking clean. */ > extern int arm_cpp_interwork; > > +/* Nonzero if chip supports Thumb. */ > +extern int arm_arch_thumb; > + > /* Nonzero if chip supports Thumb 2. */ > extern int arm_arch_thumb2; > > diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h > index > ad123dde991a3e4c4b9563ee6ebb84981767988f..f64e8caa8bc08b7aff9fe385567de9936a > 964004 100644 > --- a/gcc/config/arm/arm.h > +++ b/gcc/config/arm/arm.h > @@ -2191,9 +2191,8 @@ extern int making_const_table; > #define TARGET_ARM_V7M (!arm_arch_notm && arm_arch_thumb2) > > /* The highest Thumb instruction set version supported by the chip. */ > -#define TARGET_ARM_ARCH_ISA_THUMB \ > - (arm_arch_thumb2 ? 2 \ > - : ((TARGET_ARM_ARCH >= 5 || arm_arch4t) ? 1 : 0)) > +#define TARGET_ARM_ARCH_ISA_THUMB \ > + (arm_arch_thumb2 ? 2 : (arm_arch_thumb ? 1 : 0)) > > /* Expands to an upper-case char of the target's architectural > profile. */ > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c > index > 71b51439dc7ba5be67671e9fb4c3f18040cce58f..de1c2d4600529518a92ed44815cff05308 > baa31c 100644 > --- a/gcc/config/arm/arm.c > +++ b/gcc/config/arm/arm.c > @@ -852,6 +852,9 @@ int arm_tune_cortex_a9 = 0; > interworking clean. */ > int arm_cpp_interwork = 0; > > +/* Nonzero if chip supports Thumb. */ > +int arm_arch_thumb; > + > /* Nonzero if chip supports Thumb 2. */ > int arm_arch_thumb2; > > @@ -3170,6 +3173,7 @@ arm_option_override (void) > arm_arch7em = ARM_FSET_HAS_CPU1 (insn_flags, FL_ARCH7EM); > arm_arch8 = ARM_FSET_HAS_CPU1 (insn_flags, FL_ARCH8); > arm_arch8_1 = ARM_FSET_HAS_CPU2 (insn_flags, FL2_ARCH8_1); > + arm_arch_thumb = ARM_FSET_HAS_CPU1 (insn_flags, FL_THUMB); > arm_arch_thumb2 = ARM_FSET_HAS_CPU1 (insn_flags, FL_THUMB2); > arm_arch_xscale = ARM_FSET_HAS_CPU1 (insn_flags, FL_XSCALE); > > > > Before patch: > > % arm-none-eabi-gcc -dM -march=armv4 -E - < /dev/null | grep ISA_THUMB > cc1: warning: target CPU does not support THUMB instructions > % arm-none-eabi-gcc -dM -march=armv4t -E - < /dev/null | grep ISA_THUMB > #define __ARM_ARCH_ISA_THUMB 1 > % arm-none-eabi-gcc -dM -march=armv5 -E - < /dev/null | grep ISA_THUMB > cc1: warning: target CPU does not support THUMB instructions > #define __ARM_ARCH_ISA_THUMB 1 > % arm-none-eabi-gcc -dM -march=armv5t -E - < /dev/null | grep ISA_THUMB > #define __ARM_ARCH_ISA_THUMB 1 > > After patch: > > % arm-none-eabi-gcc -dM -march=armv5 -E - < /dev/null | grep ISA_THUMB > cc1: warning: target CPU does not support THUMB instructions > % arm-none-eabi-gcc -dM -march=armv5t -E - < /dev/null | grep ISA_THUMB > #define __ARM_ARCH_ISA_THUMB 1 > % arm-none-eabi-gcc -dM -march=armv4 -E - < /dev/null | grep ISA_THUMB > cc1: warning: target CPU does not support THUMB instructions > % arm-none-eabi-gcc -dM -march=armv4t -E - < /dev/null | grep ISA_THUMB > #define __ARM_ARCH_ISA_THUMB 1
Hi Thomas, On 10/05/16 14:26, Thomas Preudhomme wrote: > Hi, > > ARM_ARCH_ISA_THUMB is currently set to 1 when compiling for armv5 despite > armv5 not supporting Thumb instructions (armv5t does): > > arm-none-eabi-gcc -dM -march=armv5 -E - < /dev/null | grep ISA_THUMB > #define __ARM_ARCH_ISA_THUMB 1 > > The reason is TARGET_ARM_ARCH_ISA_THUMB being set to 1 if target does not > support Thumb-2 and is ARMv4T, ARMv5 or later. This patch replaces that logic > by checking whether the given architecture has the right feature bit > (FL_THUMB). > > ChangeLog entry is as follows: > > > *** gcc/ChangeLog *** > > 2016-05-06 Thomas Preud'homme <thomas.preudhomme@arm.com> > > * config/arm/arm-protos.h (arm_arch_thumb): Declare. > * config/arm/arm.c (arm_arch_thumb): Define. > (arm_option_override): Initialize arm_arch_thumb. > * config/arm/arm.h (TARGET_ARM_ARCH_ISA_THUMB): Use arm_arch_thumb to > determine if target support Thumb-1 ISA. > > > diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h > index > d8179c441bb53dced94d2ebf497aad093e4ac600..4d11c91133ff1b875afcbf58abc4491c2c93768e > 100644 > --- a/gcc/config/arm/arm-protos.h > +++ b/gcc/config/arm/arm-protos.h > @@ -603,6 +603,9 @@ extern int arm_tune_cortex_a9; > interworking clean. */ > extern int arm_cpp_interwork; > > +/* Nonzero if chip supports Thumb. */ > +extern int arm_arch_thumb; > + Bit of bikeshedding really, but I think a better name would be arm_arch_thumb1. This is because we also have the macros TARGET_THUMB and TARGET_THUMB2 where TARGET_THUMB2 means either Thumb-1 or Thumb-2 and a casual reader might think that arm_arch_thumb means that there is support for either. Also, please add a simple test that compiles something with -march=armv5 (plus -marm) and checks that __ARM_ARCH_ISA_THUMB is not defined. Ok with that change and the test. Thanks, Kyrill P.S. I think your mailer sometimes mangles long lines in the patches (for example the git hash headers). Can you please send your patches as attachments? That will also make it easier for me to extract and apply them to my tree without having to manually select the inlined patch from the message. > /* Nonzero if chip supports Thumb 2. */ > extern int arm_arch_thumb2; > > diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h > index > ad123dde991a3e4c4b9563ee6ebb84981767988f..f64e8caa8bc08b7aff9fe385567de9936a964004 > 100644 > --- a/gcc/config/arm/arm.h > +++ b/gcc/config/arm/arm.h > @@ -2191,9 +2191,8 @@ extern int making_const_table; > #define TARGET_ARM_V7M (!arm_arch_notm && arm_arch_thumb2) > > /* The highest Thumb instruction set version supported by the chip. */ > -#define TARGET_ARM_ARCH_ISA_THUMB \ > - (arm_arch_thumb2 ? 2 \ > - : ((TARGET_ARM_ARCH >= 5 || arm_arch4t) ? 1 : 0)) > +#define TARGET_ARM_ARCH_ISA_THUMB \ > + (arm_arch_thumb2 ? 2 : (arm_arch_thumb ? 1 : 0)) > > /* Expands to an upper-case char of the target's architectural > profile. */ > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c > index > 71b51439dc7ba5be67671e9fb4c3f18040cce58f..de1c2d4600529518a92ed44815cff05308baa31c > 100644 > --- a/gcc/config/arm/arm.c > +++ b/gcc/config/arm/arm.c > @@ -852,6 +852,9 @@ int arm_tune_cortex_a9 = 0; > interworking clean. */ > int arm_cpp_interwork = 0; > > +/* Nonzero if chip supports Thumb. */ > +int arm_arch_thumb; > + > /* Nonzero if chip supports Thumb 2. */ > int arm_arch_thumb2; > > @@ -3170,6 +3173,7 @@ arm_option_override (void) > arm_arch7em = ARM_FSET_HAS_CPU1 (insn_flags, FL_ARCH7EM); > arm_arch8 = ARM_FSET_HAS_CPU1 (insn_flags, FL_ARCH8); > arm_arch8_1 = ARM_FSET_HAS_CPU2 (insn_flags, FL2_ARCH8_1); > + arm_arch_thumb = ARM_FSET_HAS_CPU1 (insn_flags, FL_THUMB); > arm_arch_thumb2 = ARM_FSET_HAS_CPU1 (insn_flags, FL_THUMB2); > arm_arch_xscale = ARM_FSET_HAS_CPU1 (insn_flags, FL_XSCALE); > > > > Before patch: > > % arm-none-eabi-gcc -dM -march=armv4 -E - < /dev/null | grep ISA_THUMB > cc1: warning: target CPU does not support THUMB instructions > % arm-none-eabi-gcc -dM -march=armv4t -E - < /dev/null | grep ISA_THUMB > #define __ARM_ARCH_ISA_THUMB 1 > % arm-none-eabi-gcc -dM -march=armv5 -E - < /dev/null | grep ISA_THUMB > cc1: warning: target CPU does not support THUMB instructions > #define __ARM_ARCH_ISA_THUMB 1 > % arm-none-eabi-gcc -dM -march=armv5t -E - < /dev/null | grep ISA_THUMB > #define __ARM_ARCH_ISA_THUMB 1 > > After patch: > > % arm-none-eabi-gcc -dM -march=armv5 -E - < /dev/null | grep ISA_THUMB > cc1: warning: target CPU does not support THUMB instructions > % arm-none-eabi-gcc -dM -march=armv5t -E - < /dev/null | grep ISA_THUMB > #define __ARM_ARCH_ISA_THUMB 1 > % arm-none-eabi-gcc -dM -march=armv4 -E - < /dev/null | grep ISA_THUMB > cc1: warning: target CPU does not support THUMB instructions > % arm-none-eabi-gcc -dM -march=armv4t -E - < /dev/null | grep ISA_THUMB > #define __ARM_ARCH_ISA_THUMB 1 >
On Tuesday 24 May 2016 18:00:27 Kyrill Tkachov wrote: > Hi Thomas, > > On 10/05/16 14:26, Thomas Preudhomme wrote: > > Hi, > > > > ARM_ARCH_ISA_THUMB is currently set to 1 when compiling for armv5 despite > > armv5 not supporting Thumb instructions (armv5t does): > > > > arm-none-eabi-gcc -dM -march=armv5 -E - < /dev/null | grep ISA_THUMB > > #define __ARM_ARCH_ISA_THUMB 1 > > > > The reason is TARGET_ARM_ARCH_ISA_THUMB being set to 1 if target does not > > support Thumb-2 and is ARMv4T, ARMv5 or later. This patch replaces that > > logic by checking whether the given architecture has the right feature > > bit (FL_THUMB). > > > > ChangeLog entry is as follows: > > > > > > *** gcc/ChangeLog *** > > > > 2016-05-06 Thomas Preud'homme <thomas.preudhomme@arm.com> > > > > * config/arm/arm-protos.h (arm_arch_thumb): Declare. > > * config/arm/arm.c (arm_arch_thumb): Define. > > (arm_option_override): Initialize arm_arch_thumb. > > * config/arm/arm.h (TARGET_ARM_ARCH_ISA_THUMB): Use > > arm_arch_thumb to > > determine if target support Thumb-1 ISA. > > > > diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h > > index > > d8179c441bb53dced94d2ebf497aad093e4ac600..4d11c91133ff1b875afcbf58abc4491c > > 2c93768e 100644 > > --- a/gcc/config/arm/arm-protos.h > > +++ b/gcc/config/arm/arm-protos.h > > @@ -603,6 +603,9 @@ extern int arm_tune_cortex_a9; > > > > interworking clean. */ > > > > extern int arm_cpp_interwork; > > > > +/* Nonzero if chip supports Thumb. */ > > +extern int arm_arch_thumb; > > + > > Bit of bikeshedding really, but I think a better name would be > arm_arch_thumb1. > This is because we also have the macros TARGET_THUMB and TARGET_THUMB2 > where TARGET_THUMB2 means either Thumb-1 or Thumb-2 and a casual reader > might think that arm_arch_thumb means that there is support for either. It kind of is in the sense that Thumb-2 also includes Thumb-1 instructions so an architecture with FL_THUMB2 would also have FL_THUMB. > > Also, please add a simple test that compiles something with -march=armv5 > (plus -marm) and checks that __ARM_ARCH_ISA_THUMB is not defined. Ack. > > Ok with that change and the test. > > Thanks, > Kyrill > > P.S. I think your mailer sometimes mangles long lines in the patches > (for example the git hash headers). Can you please send your patches as > attachments? That will also make it easier for me to extract and apply > them to my tree without having to manually select the inlined patch > from the message. Will do. I thought inline was preferred to easily do inline review. Best regards, Thomas
diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h index d8179c441bb53dced94d2ebf497aad093e4ac600..4d11c91133ff1b875afcbf58abc4491c2c93768e 100644 --- a/gcc/config/arm/arm-protos.h +++ b/gcc/config/arm/arm-protos.h @@ -603,6 +603,9 @@ extern int arm_tune_cortex_a9; interworking clean. */ extern int arm_cpp_interwork; +/* Nonzero if chip supports Thumb. */ +extern int arm_arch_thumb; + /* Nonzero if chip supports Thumb 2. */ extern int arm_arch_thumb2; diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h index ad123dde991a3e4c4b9563ee6ebb84981767988f..f64e8caa8bc08b7aff9fe385567de9936a964004 100644 --- a/gcc/config/arm/arm.h +++ b/gcc/config/arm/arm.h @@ -2191,9 +2191,8 @@ extern int making_const_table; #define TARGET_ARM_V7M (!arm_arch_notm && arm_arch_thumb2) /* The highest Thumb instruction set version supported by the chip. */ -#define TARGET_ARM_ARCH_ISA_THUMB \ - (arm_arch_thumb2 ? 2 \ - : ((TARGET_ARM_ARCH >= 5 || arm_arch4t) ? 1 : 0)) +#define TARGET_ARM_ARCH_ISA_THUMB \ + (arm_arch_thumb2 ? 2 : (arm_arch_thumb ? 1 : 0)) /* Expands to an upper-case char of the target's architectural profile. */ diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index 71b51439dc7ba5be67671e9fb4c3f18040cce58f..de1c2d4600529518a92ed44815cff05308baa31c 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -852,6 +852,9 @@ int arm_tune_cortex_a9 = 0; interworking clean. */ int arm_cpp_interwork = 0; +/* Nonzero if chip supports Thumb. */ +int arm_arch_thumb; + /* Nonzero if chip supports Thumb 2. */ int arm_arch_thumb2; @@ -3170,6 +3173,7 @@ arm_option_override (void) arm_arch7em = ARM_FSET_HAS_CPU1 (insn_flags, FL_ARCH7EM); arm_arch8 = ARM_FSET_HAS_CPU1 (insn_flags, FL_ARCH8); arm_arch8_1 = ARM_FSET_HAS_CPU2 (insn_flags, FL2_ARCH8_1); + arm_arch_thumb = ARM_FSET_HAS_CPU1 (insn_flags, FL_THUMB); arm_arch_thumb2 = ARM_FSET_HAS_CPU1 (insn_flags, FL_THUMB2); arm_arch_xscale = ARM_FSET_HAS_CPU1 (insn_flags, FL_XSCALE);
Hi, ARM_ARCH_ISA_THUMB is currently set to 1 when compiling for armv5 despite armv5 not supporting Thumb instructions (armv5t does): arm-none-eabi-gcc -dM -march=armv5 -E - < /dev/null | grep ISA_THUMB #define __ARM_ARCH_ISA_THUMB 1 The reason is TARGET_ARM_ARCH_ISA_THUMB being set to 1 if target does not support Thumb-2 and is ARMv4T, ARMv5 or later. This patch replaces that logic by checking whether the given architecture has the right feature bit (FL_THUMB). ChangeLog entry is as follows: *** gcc/ChangeLog *** 2016-05-06 Thomas Preud'homme <thomas.preudhomme@arm.com> * config/arm/arm-protos.h (arm_arch_thumb): Declare. * config/arm/arm.c (arm_arch_thumb): Define. (arm_option_override): Initialize arm_arch_thumb. * config/arm/arm.h (TARGET_ARM_ARCH_ISA_THUMB): Use arm_arch_thumb to determine if target support Thumb-1 ISA. Before patch: % arm-none-eabi-gcc -dM -march=armv4 -E - < /dev/null | grep ISA_THUMB cc1: warning: target CPU does not support THUMB instructions % arm-none-eabi-gcc -dM -march=armv4t -E - < /dev/null | grep ISA_THUMB #define __ARM_ARCH_ISA_THUMB 1 % arm-none-eabi-gcc -dM -march=armv5 -E - < /dev/null | grep ISA_THUMB cc1: warning: target CPU does not support THUMB instructions #define __ARM_ARCH_ISA_THUMB 1 % arm-none-eabi-gcc -dM -march=armv5t -E - < /dev/null | grep ISA_THUMB #define __ARM_ARCH_ISA_THUMB 1 After patch: % arm-none-eabi-gcc -dM -march=armv5 -E - < /dev/null | grep ISA_THUMB cc1: warning: target CPU does not support THUMB instructions % arm-none-eabi-gcc -dM -march=armv5t -E - < /dev/null | grep ISA_THUMB #define __ARM_ARCH_ISA_THUMB 1 % arm-none-eabi-gcc -dM -march=armv4 -E - < /dev/null | grep ISA_THUMB cc1: warning: target CPU does not support THUMB instructions % arm-none-eabi-gcc -dM -march=armv4t -E - < /dev/null | grep ISA_THUMB #define __ARM_ARCH_ISA_THUMB 1