[PATCH, ARM] Fix incorrect alignment of small values in minipool

Thomas Preud'homme thomas.preudhomme@arm.com
Mon Aug 11 07:49:00 GMT 2014


Being 32-bit wide in size, constant pool entries are always filled as 32-bit
quantities. This works fine for little endian system but gives some incorrect
results for big endian system when the value is *accessed* with a mode smaller
than 32-bit in size. Suppose the case of the value 0x1234 that is accessed as an
HImode value. It would be output as 0x0 0x0 0x12 0x34 in a constant pool entry
and the 16-bit load that would be done would lead to the value 0x0 in register.
The approach here is to transform the value so that it is output correctly by
shifting the value left so that the highest byte in its mode ends up in the
highest byte of the 32-bit value being output.

The patch was tested by running the testsuite with three different builds of GCC:
1) bootstrap of GCC on x86_64-linux-gnu
2) arm-none-eabi cross compiler (defaulting to little endian) with testsuite
run under qemu emulqting a Cortex M4
3) arm-none-eabi cross compiler (defaulting to big endian, thanks to patch at [1])
with testsuite run under qemu emulating a Cortex M3. Due to the processor used,
the test constant-minipool was not run as part of the testsuite but manually with
-cpu=cortex-r4

[1] https://sourceware.org/ml/binutils/2014-08/msg00014.html

The ChangeLog is as follows:

*** gcc/ChangeLog ***

2014-07-14  Thomas Preud'homme  <thomas.preudhomme@arm.com>

	* config/arm/arm.c (dump_minipool): Fix alignment in minipool of
	values whose size is less than MINIPOOL_FIX_SIZE on big endian target.

*** gcc/testsuite/ChangeLog ***

2014-07-14  Thomas Preud'homme  <thomas.preudhomme@arm.com>

	* gcc.target/arm/constant-pool.c: New test.


diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 0146fe8..e4e0ef4 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -16507,6 +16507,15 @@ dump_minipool (rtx scan)
 	      fputc ('\n', dump_file);
 	    }
 
+	  /* On big-endian target, make sure that padding for values whose
+	     mode size is smaller than MINIPOOL_FIX_SIZE comes after.  */
+	  if (BYTES_BIG_ENDIAN && CONST_INT_P (mp->value))
+	    {
+	      int byte_disp = mp->fix_size - GET_MODE_SIZE (mp->mode);
+	      HOST_WIDE_INT val = INTVAL (mp->value);
+	      mp->value = GEN_INT (val << (byte_disp * BITS_PER_UNIT));
+	    }
+
 	  switch (mp->fix_size)
 	    {
 #ifdef HAVE_consttable_1
diff --git a/gcc/testsuite/gcc.target/arm/constant-pool.c b/gcc/testsuite/gcc.target/arm/constant-pool.c
new file mode 100644
index 0000000..46a1534
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/constant-pool.c
@@ -0,0 +1,28 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target arm_arm_ok } */
+/* { dg-options "-O1 -marm -mbig-endian" } */
+
+unsigned short v = 0x5678;
+int i;
+int j = 0;
+int *ptr = &j;
+
+int
+func (void)
+{
+  for (i = 0; i < 1; ++i)
+    {
+      *ptr = -1;
+      v = 0x1234;
+    }
+  return v;
+}
+
+int
+main (void)
+{
+  func ();
+  return v - 0x1234;
+}
+
+/* { dg-final { scan-assembler-not ".word 4660" } } */


Is this ok for trunk?

Best regards,

Thomas




More information about the Gcc-patches mailing list