[PATCH, ARM] Do not set ARM_ARCH_ISA_THUMB for armv5

[PATCH, ARM] Do not set ARM_ARCH_ISA_THUMB for armv5

[Thomas Preudhomme]

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;
+
 /* 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

Re: [PATCH, ARM, ping1] Do not set ARM_ARCH_ISA_THUMB for armv5

[Thomas Preudhomme]

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

Re: [PATCH, ARM] Do not set ARM_ARCH_ISA_THUMB for armv5

[Kyrill Tkachov]

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
>

Re: [PATCH, ARM] Do not set ARM_ARCH_ISA_THUMB for armv5

[Thomas Preudhomme]

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

Re: [PATCH, ARM] Do not set ARM_ARCH_ISA_THUMB for armv5

[Thomas Preudhomme]

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

On Tuesday 24 May 2016 18:00:27 Kyrill Tkachov wrote:
> Hi Thomas,

Hi Kyrill,


> > 
> > +/* 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.

Fixed.

> 
> Also, please add a simple test that compiles something with -march=armv5
> (plus -marm) and checks that __ARM_ARCH_ISA_THUMB is not defined.

Fixed too.

Please find the updated in attachment. ChangeLog entries are now:

*** gcc/ChangeLog ***

2016-05-26  Thomas Preud'homme  <thomas.preudhomme@arm.com>

        * config/arm/arm-protos.h (arm_arch_thumb1): Declare.
        * config/arm/arm.c (arm_arch_thumb1): Define.
        (arm_option_override): Initialize arm_arch_thumb1.
        * config/arm/arm.h (arm_arch_thumb1): Declare.
        (TARGET_ARM_ARCH_ISA_THUMB): Use arm_arch_thumb to determine if target
        support Thumb-1 ISA.


*** gcc/testsuite/ChangeLog ***

2016-05-26  Thomas Preud'homme  <thomas.preudhomme@arm.com>

        * gcc.target/arm/armv5_thumb_isa.c: New test.


Given the renaming I've redone the testing and confirmed that the patch still 
works as intended.

Best regards,

Thomas

[-- Attachment #2: fix_armv5_reported_thumb_isa.patch --]
[-- Type: text/x-patch, Size: 2809 bytes --]

diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
index d8179c441bb53dced94d2ebf497aad093e4ac600..34fd06a92d99cfcb7ece4da7f1a2957e0225e4fb 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 1.  */
+extern int arm_arch_thumb1;
+
 /* 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..1df676e7f844513c0b1b80be492965462557e25b 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -478,6 +478,9 @@ extern int arm_tune_cortex_a9;
    interworking clean.  */
 extern int arm_cpp_interwork;
 
+/* Nonzero if chip supports Thumb 1.  */
+extern int arm_arch_thumb1;
+
 /* Nonzero if chip supports Thumb 2.  */
 extern int arm_arch_thumb2;
 
@@ -2191,9 +2194,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_thumb1 ? 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..2ceee9071cb6c079c203e5876ea7e4749a255169 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 1.  */
+int arm_arch_thumb1;
+
 /* 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_thumb1 = 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);
 
diff --git a/gcc/testsuite/gcc.target/arm/armv5_thumb_isa.c b/gcc/testsuite/gcc.target/arm/armv5_thumb_isa.c
new file mode 100644
index 0000000000000000000000000000000000000000..80a00aec978778e848ea47d1eb00974fe7b0d3f5
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/armv5_thumb_isa.c
@@ -0,0 +1,8 @@
+/* { dg-require-effective-target arm_arch_v5_ok } */
+/* { dg-add-options arm_arch_v5 } */
+
+#if __ARM_ARCH_ISA_THUMB
+#error "__ARM_ARCH_ISA_THUMB defined for ARMv5"
+#endif
+
+int foo;

Re: [PATCH, ARM] Do not set ARM_ARCH_ISA_THUMB for armv5

[Kyrill Tkachov]

On 27/05/16 13:51, Thomas Preudhomme wrote:
> On Tuesday 24 May 2016 18:00:27 Kyrill Tkachov wrote:
>> Hi Thomas,
> Hi Kyrill,
>
>
>>> +/* 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.
> Fixed.
>
>> Also, please add a simple test that compiles something with -march=armv5
>> (plus -marm) and checks that __ARM_ARCH_ISA_THUMB is not defined.
> Fixed too.
>
> Please find the updated in attachment. ChangeLog entries are now:
>
> *** gcc/ChangeLog ***
>
> 2016-05-26  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>
>          * config/arm/arm-protos.h (arm_arch_thumb1): Declare.
>          * config/arm/arm.c (arm_arch_thumb1): Define.
>          (arm_option_override): Initialize arm_arch_thumb1.
>          * config/arm/arm.h (arm_arch_thumb1): Declare.
>          (TARGET_ARM_ARCH_ISA_THUMB): Use arm_arch_thumb to determine if target
>          support Thumb-1 ISA.
>
>
> *** gcc/testsuite/ChangeLog ***
>
> 2016-05-26  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>
>          * gcc.target/arm/armv5_thumb_isa.c: New test.
>

Ok.
Thanks,
Kyrill

> Given the renaming I've redone the testing and confirmed that the patch still
> works as intended.
>
> Best regards,
>
> Thomas

Re: [PATCH, ARM] Do not set ARM_ARCH_ISA_THUMB for armv5

[Thomas Preudhomme]

On Friday 27 May 2016 14:01:22 Kyrill Tkachov wrote:
> On 27/05/16 13:51, Thomas Preudhomme wrote:
> > On Tuesday 24 May 2016 18:00:27 Kyrill Tkachov wrote:
> >> Hi Thomas,
> > 
> > Hi Kyrill,
> > 
> >>> +/* 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.
> > 
> > Fixed.
> > 
> >> Also, please add a simple test that compiles something with -march=armv5
> >> (plus -marm) and checks that __ARM_ARCH_ISA_THUMB is not defined.
> > 
> > Fixed too.
> > 
> > Please find the updated in attachment. ChangeLog entries are now:
> > 
> > *** gcc/ChangeLog ***
> > 
> > 2016-05-26  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> > 
> >          * config/arm/arm-protos.h (arm_arch_thumb1): Declare.
> >          * config/arm/arm.c (arm_arch_thumb1): Define.
> >          (arm_option_override): Initialize arm_arch_thumb1.
> >          * config/arm/arm.h (arm_arch_thumb1): Declare.
> >          (TARGET_ARM_ARCH_ISA_THUMB): Use arm_arch_thumb to determine if
> >          target
> >          support Thumb-1 ISA.
> > 
> > *** gcc/testsuite/ChangeLog ***
> > 
> > 2016-05-26  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> > 
> >          * gcc.target/arm/armv5_thumb_isa.c: New test.
> 
> Ok.
> Thanks,
> Kyrill

Committed.

Best regards,

Thomas