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 <thomas.preudho...@arm.com>
* 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 <number>" by
"ARMv(7,8,8.1,8.2)-A"
in these entries.
Ok with that change.
Thanks,
Kyrill
