[arm-embedded] [PATCH, GCC/ARM] Make arm_feature_set agree with type of FL_* macros

2016-11-21 Thread Thomas Preudhomme 

Hi,

We have decided to backport this patch to fix a type inconsistency for 
arm_feature_set to our embedded-6-branch.


*** gcc/ChangeLog.arm ***

2016-11-18  Thomas Preud'homme  

Backport from mainline
2016-11-18  Thomas Preud'homme  

* config/arm/arm-protos.h (FL_NONE, FL_ANY, FL_CO_PROC, FL_ARCH3M,
FL_MODE26, FL_MODE32, FL_ARCH4, FL_ARCH5, FL_THUMB, FL_LDSCHED,
FL_STRONG, FL_ARCH5E, FL_XSCALE, FL_ARCH6, FL_VFPV2, FL_WBUF,
FL_ARCH6K, FL_THUMB2, FL_NOTM, FL_THUMB_DIV, FL_VFPV3, FL_NEON,
FL_ARCH7EM, FL_ARCH7, FL_ARM_DIV, FL_ARCH8, FL_CRC32, FL_SMALLMUL,
FL_NO_VOLATILE_CE, FL_IWMMXT, FL_IWMMXT2, FL_ARCH6KZ, FL2_ARCH8_1,
FL2_ARCH8_2, FL2_FP16INST): Reindent comment, add final dot when
missing and make value unsigned.
(arm_feature_set): Use unsigned entries instead of unsigned long.
--- Begin Message ---

Hi,

I've rebased the patch to make arm_feature_set agree with type of FL_* macros on 
top of trunk rather than on top of the optional -mthumb patch. That involved 
doing the changes to gcc/config/arm/arm-protos.h rather than 
gcc/config/arm/arm-flags.h. I also took advantage of the fact that each line is 
changed to change the indentation to tabs and add dots in comments missing one.


For reference, please find below the original patch description:

Currently arm_feature_set is defined in gcc/config/arm/arm-flags as an array of 
2 unsigned long. However, the flags stored in these two entries are (signed) 
int, being combinations of bits set via expression of the form 1 << bitno. This 
creates 3 issues:


1) undefined behavior when setting the msb (1 << 31)
2) undefined behavior when storing a flag with msb set (negative int) into one 
of the unsigned array entries (positive int)

3) waste of space since the top 32 bits of each entry is not used

This patch changes the definition of FL_* macro to be unsigned int by using the 
form 1U << bitno instead and changes the definition of arm_feature_set to be an 
array of 2 unsigned (int) entries.


*** gcc/ChangeLog ***

2016-10-15  Thomas Preud'homme  

* config/arm/arm-protos.h (FL_NONE, FL_ANY, FL_CO_PROC, FL_ARCH3M,
FL_MODE26, FL_MODE32, FL_ARCH4, FL_ARCH5, FL_THUMB, FL_LDSCHED,
FL_STRONG, FL_ARCH5E, FL_XSCALE, FL_ARCH6, FL_VFPV2, FL_WBUF,
FL_ARCH6K, FL_THUMB2, FL_NOTM, FL_THUMB_DIV, FL_VFPV3, FL_NEON,
FL_ARCH7EM, FL_ARCH7, FL_ARM_DIV, FL_ARCH8, FL_CRC32, FL_SMALLMUL,
FL_NO_VOLATILE_CE, FL_IWMMXT, FL_IWMMXT2, FL_ARCH6KZ, FL2_ARCH8_1,
FL2_ARCH8_2, FL2_FP16INST): Reindent comment, add final dot when
missing and make value unsigned.
(arm_feature_set): Use unsigned entries instead of unsigned long.


Bootstrapped on arm-linux-gnueabihf targeting Thumb-2 state.

Is this ok for trunk?

Best regards,

Thomas

On 14/11/16 18:56, Thomas Preudhomme wrote:

My apologize, I realized when trying to apply the patch that I wrote it on top
of the optional -mthumb patch instead of the reverse. I'll rebase it to not
screw up bisect.

Best regards,

Thomas

On 14/11/16 14:47, Kyrill Tkachov wrote:


On 14/11/16 14:07, Thomas Preudhomme wrote:

Hi,

Currently arm_feature_set is defined in gcc/config/arm/arm-flags as an array
of 2 unsigned long. However, the flags stored in these two entries are
(signed) int, being combinations of bits set via expression of the form 1 <<
bitno. This creates 3 issues:

1) undefined behavior when setting the msb (1 << 31)
2) undefined behavior when storing a flag with msb set (negative int) into one
of the unsigned array entries (positive int)
3) waste of space since the top 32 bits of each entry is not used

This patch changes the definition of FL_* macro to be unsigned int by using
the form 1U << bitno instead and changes the definition of arm_feature_set to
be an array of 2 unsigned (int) entries.

Bootstrapped on arm-linux-gnueabihf targeting Thumb-2 state.

Is this ok for trunk?



Ok.
Thanks,
Kyrill


Best regards,

Thomas


diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
index 95bae5ef57ba4c433c0cce8e0c197959abdf887b..5cee7718554886982f535da2e9baa5015da609e4 100644
--- a/gcc/config/arm/arm-protos.h
+++ b/gcc/config/arm/arm-protos.h
@@ -351,50 +351,51 @@ 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_NONE	  (0)	  /* No flags.  */
-#define FL_ANY	  (0x)/* All flags.  */
-#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

Re: [PATCH, GCC/ARM] Make arm_feature_set agree with type of FL_* macros

2016-11-18 Thread Kyrill Tkachov 


On 17/11/16 20:41, Thomas Preudhomme wrote:



On 17/11/16 09:10, Kyrill Tkachov wrote:


On 16/11/16 18:09, Thomas Preudhomme wrote:

Hi,

I've rebased the patch to make arm_feature_set agree with type of FL_* macros
on top of trunk rather than on top of the optional -mthumb patch. That
involved doing the changes to gcc/config/arm/arm-protos.h rather than
gcc/config/arm/arm-flags.h. I also took advantage of the fact that each line
is changed to change the indentation to tabs and add dots in comments missing
one.

For reference, please find below the original patch description:

Currently arm_feature_set is defined in gcc/config/arm/arm-flags as an array
of 2 unsigned long. However, the flags stored in these two entries are
(signed) int, being combinations of bits set via expression of the form 1 <<
bitno. This creates 3 issues:

1) undefined behavior when setting the msb (1 << 31)
2) undefined behavior when storing a flag with msb set (negative int) into one
of the unsigned array entries (positive int)
3) waste of space since the top 32 bits of each entry is not used

This patch changes the definition of FL_* macro to be unsigned int by using
the form 1U << bitno instead and changes the definition of arm_feature_set to
be an array of 2 unsigned (int) entries.

*** gcc/ChangeLog ***

2016-10-15  Thomas Preud'homme 

* config/arm/arm-protos.h (FL_NONE, FL_ANY, FL_CO_PROC, FL_ARCH3M,
FL_MODE26, FL_MODE32, FL_ARCH4, FL_ARCH5, FL_THUMB, FL_LDSCHED,
FL_STRONG, FL_ARCH5E, FL_XSCALE, FL_ARCH6, FL_VFPV2, FL_WBUF,
FL_ARCH6K, FL_THUMB2, FL_NOTM, FL_THUMB_DIV, FL_VFPV3, FL_NEON,
FL_ARCH7EM, FL_ARCH7, FL_ARM_DIV, FL_ARCH8, FL_CRC32, FL_SMALLMUL,
FL_NO_VOLATILE_CE, FL_IWMMXT, FL_IWMMXT2, FL_ARCH6KZ, FL2_ARCH8_1,
FL2_ARCH8_2, FL2_FP16INST): Reindent comment, add final dot when
missing and make value unsigned.
(arm_feature_set): Use unsigned entries instead of unsigned long.


Bootstrapped on arm-linux-gnueabihf targeting Thumb-2 state.

Is this ok for trunk?

Best regards,

Thomas

On 14/11/16 18:56, Thomas Preudhomme wrote:

My apologize, I realized when trying to apply the patch that I wrote it on top
of the optional -mthumb patch instead of the reverse. I'll rebase it to not
screw up bisect.

Best regards,

Thomas

On 14/11/16 14:47, Kyrill Tkachov wrote:


On 14/11/16 14:07, Thomas Preudhomme wrote:

Hi,

Currently arm_feature_set is defined in gcc/config/arm/arm-flags as an array
of 2 unsigned long. However, the flags stored in these two entries are
(signed) int, being combinations of bits set via expression of the form 1 <<
bitno. This creates 3 issues:

1) undefined behavior when setting the msb (1 << 31)
2) undefined behavior when storing a flag with msb set (negative int) into one
of the unsigned array entries (positive int)
3) waste of space since the top 32 bits of each entry is not used

This patch changes the definition of FL_* macro to be unsigned int by using
the form 1U << bitno instead and changes the definition of arm_feature_set to
be an array of 2 unsigned (int) entries.

Bootstrapped on arm-linux-gnueabihf targeting Thumb-2 state.

Is this ok for trunk?



Ok.
Thanks,
Kyrill


Best regards,

Thomas




+#define FL_ARCH7  (1U << 22)/* Architecture 7. */
+#define FL_ARM_DIV(1U << 23)/* Hardware divide (ARM mode).  */
+#define FL_ARCH8  (1U << 24)/* Architecture 8. */
+#define FL_CRC32  (1U << 25)/* ARMv8 CRC32 instructions.  */
+#define FL_SMALLMUL   (1U << 26)/* Small multiply supported.  */
+#define FL_NO_VOLATILE_CE  (1U << 27)/* No volatile memory in IT block.  */
+
+#define FL_IWMMXT (1U << 29)/* XScale v2 or "Intel Wireless MMX
+   technology".  */
+#define FL_IWMMXT2(1U << 30)/* "Intel Wireless MMX2
+technology".  */
+#define FL_ARCH6KZ(1U << 31)/* ARMv6KZ architecture.  */
+
+#define FL2_ARCH8_1   (1U << 0)/* Architecture 8.1.  */
+#define FL2_ARCH8_2   (1U << 1)/* Architecture 8.2.  */
+#define FL2_FP16INST  (1U << 2)/* FP16 Instructions for ARMv8.2 and
+   later.  */

Since you're here can you please replace the "Architecture " by
"ARMv(7,8,8.1,8.2)-A"
in these entries.


That seems to make it less accurate though. For a start, most of them would also apply to the R profile. There is also a couple of them that would apply to the M profile: for instance FL_ARCH7 would be set for ARMv7-M and FL_ARCH8 would 
be set for ARMv8-M.




Fair point.
Ok as is then,
Kyrill


Best regards,

Thomas

Re: [PATCH, GCC/ARM] Make arm_feature_set agree with type of FL_* macros

2016-11-17 Thread Thomas Preudhomme 



On 17/11/16 09:10, Kyrill Tkachov wrote:


On 16/11/16 18:09, Thomas Preudhomme wrote:

Hi,

I've rebased the patch to make arm_feature_set agree with type of FL_* macros
on top of trunk rather than on top of the optional -mthumb patch. That
involved doing the changes to gcc/config/arm/arm-protos.h rather than
gcc/config/arm/arm-flags.h. I also took advantage of the fact that each line
is changed to change the indentation to tabs and add dots in comments missing
one.

For reference, please find below the original patch description:

Currently arm_feature_set is defined in gcc/config/arm/arm-flags as an array
of 2 unsigned long. However, the flags stored in these two entries are
(signed) int, being combinations of bits set via expression of the form 1 <<
bitno. This creates 3 issues:

1) undefined behavior when setting the msb (1 << 31)
2) undefined behavior when storing a flag with msb set (negative int) into one
of the unsigned array entries (positive int)
3) waste of space since the top 32 bits of each entry is not used

This patch changes the definition of FL_* macro to be unsigned int by using
the form 1U << bitno instead and changes the definition of arm_feature_set to
be an array of 2 unsigned (int) entries.

*** gcc/ChangeLog ***

2016-10-15  Thomas Preud'homme  

* config/arm/arm-protos.h (FL_NONE, FL_ANY, FL_CO_PROC, FL_ARCH3M,
FL_MODE26, FL_MODE32, FL_ARCH4, FL_ARCH5, FL_THUMB, FL_LDSCHED,
FL_STRONG, FL_ARCH5E, FL_XSCALE, FL_ARCH6, FL_VFPV2, FL_WBUF,
FL_ARCH6K, FL_THUMB2, FL_NOTM, FL_THUMB_DIV, FL_VFPV3, FL_NEON,
FL_ARCH7EM, FL_ARCH7, FL_ARM_DIV, FL_ARCH8, FL_CRC32, FL_SMALLMUL,
FL_NO_VOLATILE_CE, FL_IWMMXT, FL_IWMMXT2, FL_ARCH6KZ, FL2_ARCH8_1,
FL2_ARCH8_2, FL2_FP16INST): Reindent comment, add final dot when
missing and make value unsigned.
(arm_feature_set): Use unsigned entries instead of unsigned long.


Bootstrapped on arm-linux-gnueabihf targeting Thumb-2 state.

Is this ok for trunk?

Best regards,

Thomas

On 14/11/16 18:56, Thomas Preudhomme wrote:

My apologize, I realized when trying to apply the patch that I wrote it on top
of the optional -mthumb patch instead of the reverse. I'll rebase it to not
screw up bisect.

Best regards,

Thomas

On 14/11/16 14:47, Kyrill Tkachov wrote:


On 14/11/16 14:07, Thomas Preudhomme wrote:

Hi,

Currently arm_feature_set is defined in gcc/config/arm/arm-flags as an array
of 2 unsigned long. However, the flags stored in these two entries are
(signed) int, being combinations of bits set via expression of the form 1 <<
bitno. This creates 3 issues:

1) undefined behavior when setting the msb (1 << 31)
2) undefined behavior when storing a flag with msb set (negative int) into one
of the unsigned array entries (positive int)
3) waste of space since the top 32 bits of each entry is not used

This patch changes the definition of FL_* macro to be unsigned int by using
the form 1U << bitno instead and changes the definition of arm_feature_set to
be an array of 2 unsigned (int) entries.

Bootstrapped on arm-linux-gnueabihf targeting Thumb-2 state.

Is this ok for trunk?



Ok.
Thanks,
Kyrill


Best regards,

Thomas




+#define FL_ARCH7  (1U << 22)/* Architecture 7.  */
+#define FL_ARM_DIV(1U << 23)/* Hardware divide (ARM mode).  */
+#define FL_ARCH8  (1U << 24)/* Architecture 8.  */
+#define FL_CRC32  (1U << 25)/* ARMv8 CRC32 instructions.  */
+#define FL_SMALLMUL   (1U << 26)/* Small multiply supported.  */
+#define FL_NO_VOLATILE_CE  (1U << 27)/* No volatile memory in IT block.  */
+
+#define FL_IWMMXT (1U << 29)/* XScale v2 or "Intel Wireless MMX
+   technology".  */
+#define FL_IWMMXT2(1U << 30)/* "Intel Wireless MMX2
+technology".  */
+#define FL_ARCH6KZ(1U << 31)/* ARMv6KZ architecture.  */
+
+#define FL2_ARCH8_1   (1U << 0)/* Architecture 8.1.  */
+#define FL2_ARCH8_2   (1U << 1)/* Architecture 8.2.  */
+#define FL2_FP16INST  (1U << 2)/* FP16 Instructions for ARMv8.2 and
+   later.  */

Since you're here can you please replace the "Architecture " by
"ARMv(7,8,8.1,8.2)-A"
in these entries.


That seems to make it less accurate though. For a start, most of them would also 
apply to the R profile. There is also a couple of them that would apply to the M 
profile: for instance FL_ARCH7 would be set for ARMv7-M and FL_ARCH8 would be 
set for ARMv8-M.


Best regards,

Thomas

Re: [PATCH, GCC/ARM] Make arm_feature_set agree with type of FL_* macros

2016-11-17 Thread Kyrill Tkachov 


On 16/11/16 18:09, Thomas Preudhomme wrote:

Hi,

I've rebased the patch to make arm_feature_set agree with type of FL_* macros on top of trunk rather than on top of the optional -mthumb patch. That involved doing the changes to gcc/config/arm/arm-protos.h rather than 
gcc/config/arm/arm-flags.h. I also took advantage of the fact that each line is changed to change the indentation to tabs and add dots in comments missing one.


For reference, please find below the original patch description:

Currently arm_feature_set is defined in gcc/config/arm/arm-flags as an array of 2 unsigned long. However, the flags stored in these two entries are (signed) int, being combinations of bits set via expression of the form 1 << bitno. This 
creates 3 issues:


1) undefined behavior when setting the msb (1 << 31)
2) undefined behavior when storing a flag with msb set (negative int) into one 
of the unsigned array entries (positive int)
3) waste of space since the top 32 bits of each entry is not used

This patch changes the definition of FL_* macro to be unsigned int by using the form 
1U << bitno instead and changes the definition of arm_feature_set to be an 
array of 2 unsigned (int) entries.

*** gcc/ChangeLog ***

2016-10-15  Thomas Preud'homme  

* config/arm/arm-protos.h (FL_NONE, FL_ANY, FL_CO_PROC, FL_ARCH3M,
FL_MODE26, FL_MODE32, FL_ARCH4, FL_ARCH5, FL_THUMB, FL_LDSCHED,
FL_STRONG, FL_ARCH5E, FL_XSCALE, FL_ARCH6, FL_VFPV2, FL_WBUF,
FL_ARCH6K, FL_THUMB2, FL_NOTM, FL_THUMB_DIV, FL_VFPV3, FL_NEON,
FL_ARCH7EM, FL_ARCH7, FL_ARM_DIV, FL_ARCH8, FL_CRC32, FL_SMALLMUL,
FL_NO_VOLATILE_CE, FL_IWMMXT, FL_IWMMXT2, FL_ARCH6KZ, FL2_ARCH8_1,
FL2_ARCH8_2, FL2_FP16INST): Reindent comment, add final dot when
missing and make value unsigned.
(arm_feature_set): Use unsigned entries instead of unsigned long.


Bootstrapped on arm-linux-gnueabihf targeting Thumb-2 state.

Is this ok for trunk?

Best regards,

Thomas

On 14/11/16 18:56, Thomas Preudhomme wrote:

My apologize, I realized when trying to apply the patch that I wrote it on top
of the optional -mthumb patch instead of the reverse. I'll rebase it to not
screw up bisect.

Best regards,

Thomas

On 14/11/16 14:47, Kyrill Tkachov wrote:


On 14/11/16 14:07, Thomas Preudhomme wrote:

Hi,

Currently arm_feature_set is defined in gcc/config/arm/arm-flags as an array
of 2 unsigned long. However, the flags stored in these two entries are
(signed) int, being combinations of bits set via expression of the form 1 <<
bitno. This creates 3 issues:

1) undefined behavior when setting the msb (1 << 31)
2) undefined behavior when storing a flag with msb set (negative int) into one
of the unsigned array entries (positive int)
3) waste of space since the top 32 bits of each entry is not used

This patch changes the definition of FL_* macro to be unsigned int by using
the form 1U << bitno instead and changes the definition of arm_feature_set to
be an array of 2 unsigned (int) entries.

Bootstrapped on arm-linux-gnueabihf targeting Thumb-2 state.

Is this ok for trunk?



Ok.
Thanks,
Kyrill


Best regards,

Thomas




+#define FL_ARCH7  (1U << 22) /* Architecture 7.  */
+#define FL_ARM_DIV(1U << 23) /* Hardware divide (ARM mode).  */
+#define FL_ARCH8  (1U << 24) /* Architecture 8.  */
+#define FL_CRC32  (1U << 25) /* ARMv8 CRC32 instructions.  */
+#define FL_SMALLMUL   (1U << 26) /* Small multiply supported.  */
+#define FL_NO_VOLATILE_CE  (1U << 27)/* No volatile memory in IT block.  */
+
+#define FL_IWMMXT (1U << 29) /* XScale v2 or "Intel Wireless MMX
+  technology".  */
+#define FL_IWMMXT2(1U << 30) /* "Intel Wireless MMX2
+   technology".  */
+#define FL_ARCH6KZ(1U << 31) /* ARMv6KZ architecture.  */
+
+#define FL2_ARCH8_1   (1U << 0)  /* Architecture 8.1.  */
+#define FL2_ARCH8_2   (1U << 1)  /* Architecture 8.2.  */
+#define FL2_FP16INST  (1U << 2)  /* FP16 Instructions for ARMv8.2 and
+  later.  */

Since you're here can you please replace the "Architecture " by 
"ARMv(7,8,8.1,8.2)-A"
in these entries.

Ok with that change.
Thanks,
Kyrill

Re: [PATCH, GCC/ARM] Make arm_feature_set agree with type of FL_* macros

2016-11-16 Thread Thomas Preudhomme 

Hi,

I've rebased the patch to make arm_feature_set agree with type of FL_* macros on 
top of trunk rather than on top of the optional -mthumb patch. That involved 
doing the changes to gcc/config/arm/arm-protos.h rather than 
gcc/config/arm/arm-flags.h. I also took advantage of the fact that each line is 
changed to change the indentation to tabs and add dots in comments missing one.


For reference, please find below the original patch description:

Currently arm_feature_set is defined in gcc/config/arm/arm-flags as an array of 
2 unsigned long. However, the flags stored in these two entries are (signed) 
int, being combinations of bits set via expression of the form 1 << bitno. This 
creates 3 issues:


1) undefined behavior when setting the msb (1 << 31)
2) undefined behavior when storing a flag with msb set (negative int) into one 
of the unsigned array entries (positive int)

3) waste of space since the top 32 bits of each entry is not used

This patch changes the definition of FL_* macro to be unsigned int by using the 
form 1U << bitno instead and changes the definition of arm_feature_set to be an 
array of 2 unsigned (int) entries.


*** gcc/ChangeLog ***

2016-10-15  Thomas Preud'homme  

* config/arm/arm-protos.h (FL_NONE, FL_ANY, FL_CO_PROC, FL_ARCH3M,
FL_MODE26, FL_MODE32, FL_ARCH4, FL_ARCH5, FL_THUMB, FL_LDSCHED,
FL_STRONG, FL_ARCH5E, FL_XSCALE, FL_ARCH6, FL_VFPV2, FL_WBUF,
FL_ARCH6K, FL_THUMB2, FL_NOTM, FL_THUMB_DIV, FL_VFPV3, FL_NEON,
FL_ARCH7EM, FL_ARCH7, FL_ARM_DIV, FL_ARCH8, FL_CRC32, FL_SMALLMUL,
FL_NO_VOLATILE_CE, FL_IWMMXT, FL_IWMMXT2, FL_ARCH6KZ, FL2_ARCH8_1,
FL2_ARCH8_2, FL2_FP16INST): Reindent comment, add final dot when
missing and make value unsigned.
(arm_feature_set): Use unsigned entries instead of unsigned long.


Bootstrapped on arm-linux-gnueabihf targeting Thumb-2 state.

Is this ok for trunk?

Best regards,

Thomas

On 14/11/16 18:56, Thomas Preudhomme wrote:

My apologize, I realized when trying to apply the patch that I wrote it on top
of the optional -mthumb patch instead of the reverse. I'll rebase it to not
screw up bisect.

Best regards,

Thomas

On 14/11/16 14:47, Kyrill Tkachov wrote:


On 14/11/16 14:07, Thomas Preudhomme wrote:

Hi,

Currently arm_feature_set is defined in gcc/config/arm/arm-flags as an array
of 2 unsigned long. However, the flags stored in these two entries are
(signed) int, being combinations of bits set via expression of the form 1 <<
bitno. This creates 3 issues:

1) undefined behavior when setting the msb (1 << 31)
2) undefined behavior when storing a flag with msb set (negative int) into one
of the unsigned array entries (positive int)
3) waste of space since the top 32 bits of each entry is not used

This patch changes the definition of FL_* macro to be unsigned int by using
the form 1U << bitno instead and changes the definition of arm_feature_set to
be an array of 2 unsigned (int) entries.

Bootstrapped on arm-linux-gnueabihf targeting Thumb-2 state.

Is this ok for trunk?



Ok.
Thanks,
Kyrill


Best regards,

Thomas


diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
index 95bae5ef57ba4c433c0cce8e0c197959abdf887b..5cee7718554886982f535da2e9baa5015da609e4 100644
--- a/gcc/config/arm/arm-protos.h
+++ b/gcc/config/arm/arm-protos.h
@@ -351,50 +351,51 @@ 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_NONE	  (0)	  /* No flags.  */
-#define FL_ANY	  (0x)/* All flags.  */
-#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

Re: [PATCH, GCC/ARM] Make arm_feature_set agree with type of FL_* macros

2016-11-14 Thread Thomas Preudhomme 
My apologize, I realized when trying to apply the patch that I wrote it on top 
of the optional -mthumb patch instead of the reverse. I'll rebase it to not 
screw up bisect.


Best regards,

Thomas

On 14/11/16 14:47, Kyrill Tkachov wrote:


On 14/11/16 14:07, Thomas Preudhomme wrote:

Hi,

Currently arm_feature_set is defined in gcc/config/arm/arm-flags as an array
of 2 unsigned long. However, the flags stored in these two entries are
(signed) int, being combinations of bits set via expression of the form 1 <<
bitno. This creates 3 issues:

1) undefined behavior when setting the msb (1 << 31)
2) undefined behavior when storing a flag with msb set (negative int) into one
of the unsigned array entries (positive int)
3) waste of space since the top 32 bits of each entry is not used

This patch changes the definition of FL_* macro to be unsigned int by using
the form 1U << bitno instead and changes the definition of arm_feature_set to
be an array of 2 unsigned (int) entries.

Bootstrapped on arm-linux-gnueabihf targeting Thumb-2 state.

Is this ok for trunk?



Ok.
Thanks,
Kyrill


Best regards,

Thomas

Re: [PATCH, GCC/ARM] Make arm_feature_set agree with type of FL_* macros

2016-11-14 Thread Thomas Preudhomme 

Hi Christophe,

No, they were seen when bootstrapping on arm-linux-gnueabihf with the patch to 
to make -mthumb optional. The patch store flags in an arm_feature_set array and 
GCC -Wnarrowing complained about the difference of type.


Best regards,

Thomas

On 14/11/16 17:00, Christophe Lyon wrote:

Hi,


On 14 November 2016 at 15:07, Thomas Preudhomme
 wrote:

Hi,

Currently arm_feature_set is defined in gcc/config/arm/arm-flags as an array
of 2 unsigned long. However, the flags stored in these two entries are
(signed) int, being combinations of bits set via expression of the form 1 <<
bitno. This creates 3 issues:

1) undefined behavior when setting the msb (1 << 31)
2) undefined behavior when storing a flag with msb set (negative int) into
one of the unsigned array entries (positive int)


Just curious: are these problems seen when building with ubsan enabled?


3) waste of space since the top 32 bits of each entry is not used

This patch changes the definition of FL_* macro to be unsigned int by using
the form 1U << bitno instead and changes the definition of arm_feature_set
to be an array of 2 unsigned (int) entries.

Bootstrapped on arm-linux-gnueabihf targeting Thumb-2 state.

Is this ok for trunk?

Best regards,

Thomas

Re: [PATCH, GCC/ARM] Make arm_feature_set agree with type of FL_* macros

2016-11-14 Thread Christophe Lyon 
Hi,


On 14 November 2016 at 15:07, Thomas Preudhomme
 wrote:
> Hi,
>
> Currently arm_feature_set is defined in gcc/config/arm/arm-flags as an array
> of 2 unsigned long. However, the flags stored in these two entries are
> (signed) int, being combinations of bits set via expression of the form 1 <<
> bitno. This creates 3 issues:
>
> 1) undefined behavior when setting the msb (1 << 31)
> 2) undefined behavior when storing a flag with msb set (negative int) into
> one of the unsigned array entries (positive int)

Just curious: are these problems seen when building with ubsan enabled?

> 3) waste of space since the top 32 bits of each entry is not used
>
> This patch changes the definition of FL_* macro to be unsigned int by using
> the form 1U << bitno instead and changes the definition of arm_feature_set
> to be an array of 2 unsigned (int) entries.
>
> Bootstrapped on arm-linux-gnueabihf targeting Thumb-2 state.
>
> Is this ok for trunk?
>
> Best regards,
>
> Thomas

Re: [PATCH, GCC/ARM] Make arm_feature_set agree with type of FL_* macros

2016-11-14 Thread Kyrill Tkachov 


On 14/11/16 14:07, Thomas Preudhomme wrote:

Hi,

Currently arm_feature_set is defined in gcc/config/arm/arm-flags as an array of 2 unsigned long. However, the flags stored in these two entries are (signed) int, being combinations of bits set via expression of the form 1 << bitno. This 
creates 3 issues:


1) undefined behavior when setting the msb (1 << 31)
2) undefined behavior when storing a flag with msb set (negative int) into one 
of the unsigned array entries (positive int)
3) waste of space since the top 32 bits of each entry is not used

This patch changes the definition of FL_* macro to be unsigned int by using the form 
1U << bitno instead and changes the definition of arm_feature_set to be an 
array of 2 unsigned (int) entries.

Bootstrapped on arm-linux-gnueabihf targeting Thumb-2 state.

Is this ok for trunk?



Ok.
Thanks,
Kyrill


Best regards,

Thomas

Re: [PATCH, GCC/ARM] Make arm_feature_set agree with type of FL_* macros

2016-11-14 Thread Thomas Preudhomme 
I forgot to mention that this patch is needed for the optional -mthumb patch [1] 
to bootstrap.


[1] https://gcc.gnu.org/ml/gcc-patches/2016-11/msg00735.html

Best regards,

Thomas

On 14/11/16 14:07, Thomas Preudhomme wrote:

Hi,

Currently arm_feature_set is defined in gcc/config/arm/arm-flags as an array of
2 unsigned long. However, the flags stored in these two entries are (signed)
int, being combinations of bits set via expression of the form 1 << bitno. This
creates 3 issues:

1) undefined behavior when setting the msb (1 << 31)
2) undefined behavior when storing a flag with msb set (negative int) into one
of the unsigned array entries (positive int)
3) waste of space since the top 32 bits of each entry is not used

This patch changes the definition of FL_* macro to be unsigned int by using the
form 1U << bitno instead and changes the definition of arm_feature_set to be an
array of 2 unsigned (int) entries.

Bootstrapped on arm-linux-gnueabihf targeting Thumb-2 state.

Is this ok for trunk?

Best regards,

Thomas

[PATCH, GCC/ARM] Make arm_feature_set agree with type of FL_* macros

2016-11-14 Thread Thomas Preudhomme 

Hi,

Currently arm_feature_set is defined in gcc/config/arm/arm-flags as an array of 
2 unsigned long. However, the flags stored in these two entries are (signed) 
int, being combinations of bits set via expression of the form 1 << bitno. This 
creates 3 issues:


1) undefined behavior when setting the msb (1 << 31)
2) undefined behavior when storing a flag with msb set (negative int) into one 
of the unsigned array entries (positive int)

3) waste of space since the top 32 bits of each entry is not used

This patch changes the definition of FL_* macro to be unsigned int by using the 
form 1U << bitno instead and changes the definition of arm_feature_set to be an 
array of 2 unsigned (int) entries.


Bootstrapped on arm-linux-gnueabihf targeting Thumb-2 state.

Is this ok for trunk?

Best regards,

Thomas
diff --git a/gcc/config/arm/arm-flags.h b/gcc/config/arm/arm-flags.h
index 9a5991aa07a229a7741e526c2876e7e0e4749db4..136a36e403dd3207deb91adf8c36e568bc08fd9e 100644
--- a/gcc/config/arm/arm-flags.h
+++ b/gcc/config/arm/arm-flags.h
@@ -25,49 +25,49 @@
 /* Flags used to identify the presence of processor capabilities.  */
 
 /* Bit values used to identify processor capabilities.  */
-#define FL_NONE	  (0)	  /* No flags.  */
-#define FL_ANY	  (0x)/* All flags.  */
-#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
+#define FL_NONE	  (0U)	  /* No flags.  */
+#define FL_ANY	  (0xU)   /* All flags.  */
+#define FL_CO_PROC(1U << 0)   /* Has external co-processor bus */
+#define FL_ARCH3M (1U << 1)   /* Extended multiply */
+#define FL_MODE26 (1U << 2)   /* 26-bit mode support */
+#define FL_MODE32 (1U << 3)   /* 32-bit mode support */
+#define FL_ARCH4  (1U << 4)   /* Architecture rel 4 */
+#define FL_ARCH5  (1U << 5)   /* Architecture rel 5 */
+#define FL_THUMB  (1U << 6)   /* Thumb aware */
+#define FL_LDSCHED(1U << 7)	  /* Load scheduling necessary */
+#define FL_STRONG (1U << 8)	  /* StrongARM */
+#define FL_ARCH5E (1U << 9)   /* DSP extensions to v5 */
+#define FL_XSCALE (1U << 10)  /* XScale */
+/* spare	  (1U << 11)*/
+#define FL_ARCH6  (1U << 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.
+#define FL_VFPV2  (1U << 13)  /* Vector Floating Point V2.  */
+#define FL_WBUF	  (1U << 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'
+#define FL_ARCH6K (1U << 15)  /* Architecture rel 6 K extensions.  */
+#define FL_THUMB2 (1U << 16)  /* Thumb-2.  */
+#define FL_NOTM	  (1U << 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
+#define FL_THUMB_DIV  (1U << 18)  /* Hardware divide (Thumb mode).  */
+#define FL_VFPV3  (1U << 19)  /* Vector Floating Point V3.  */
+#define FL_NEON   (1U << 20)  /* Neon instructions.  */
+#define FL_ARCH7EM(1U << 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_ARCH7  (1U << 22)  /* Architecture 7.  */
+#define FL_ARM_DIV(1U << 23)  /* Hardware divide (ARM mode).  */
+#define FL_ARCH8  (1U << 24)  /* Architecture 8.  */
+#define FL_CRC32  (1U << 25)  /* ARMv8 CRC32 instructions.  */
 
-#define FL_SMALLMUL   (1 << 26)