[PATCH][ARM] Fix PR59593/PR63742: arm *movhi_insn_arch4 pattern for big-endian

["Thomas Preud'homme" <thomas dot preudhomme at arm dot com>]

Currently, constant pool entries for QImode, HImode and SImode values are 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.

This patch makes the consttable_1 and consttable_2 pattern available to ARM as well so that values are output according to their mode. This is a different approach than the one proposed by Felix Yang at [1].

[1] https://gcc.gnu.org/ml/gcc-patches/2014-11/msg00258.html

ChangeLog entries are as follows:

*** gcc/ChangeLog ***

2014-09-03  Thomas Preud'homme  <thomas.preudhomme@arm.com>

	PR target/59593
	* config/arm/arm.c (dump_minipool): dispatch to consttable pattern
	based on mode size.
	* config/arm/arm.md (consttable_1): Move from config/arm/thumb1.md and
	make it TARGET_EITHER.
	(consttable_2): Move from config/arm/thumb1.md, make it TARGET_EITHER
	and move HFmode handling from consttable_4 to it.
	(consttable_4): Move HFmode handling to consttable_2 pattern.
	* config/arm/thumb1.md (consttable_1): Move to config/arm/arm.md.
	(consttable_2): Ditto.

*** gcc/testsuite/ChangeLog ***

2014-10-08  Thomas Preud'homme  <thomas.preudhomme@arm.com>

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


diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index d8bfda3..63babd2 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -16608,7 +16608,7 @@ dump_minipool (rtx_insn *scan)
 	      fputc ('\n', dump_file);
 	    }
 
-	  switch (mp->fix_size)
+	  switch (GET_MODE_SIZE (mp->mode))
 	    {
 #ifdef HAVE_consttable_1
 	    case 1:
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index cd9ab6c..8ca88b6 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -10567,6 +10567,42 @@
   [(set_attr "type" "no_insn")]
 )
 
+(define_insn "consttable_1"
+  [(unspec_volatile [(match_operand 0 "" "")] VUNSPEC_POOL_1)]
+  "TARGET_EITHER"
+  "*
+  making_const_table = TRUE;
+  assemble_integer (operands[0], 1, BITS_PER_WORD, 1);
+  assemble_zeros (3);
+  return \"\";
+  "
+  [(set_attr "length" "4")
+   (set_attr "type" "no_insn")]
+)
+
+(define_insn "consttable_2"
+  [(unspec_volatile [(match_operand 0 "" "")] VUNSPEC_POOL_2)]
+  "TARGET_EITHER"
+  "*
+  {
+    rtx x = operands[0];
+    making_const_table = TRUE;
+    switch (GET_MODE_CLASS (GET_MODE (x)))
+      {
+      case MODE_FLOAT:
+	arm_emit_fp16_const (x);
+	break;
+      default:
+	assemble_integer (operands[0], 2, BITS_PER_WORD, 1);
+	assemble_zeros (2);
+	break;
+      }
+    return \"\";
+  }"
+  [(set_attr "length" "4")
+   (set_attr "type" "no_insn")]
+)
+
 (define_insn "consttable_4"
   [(unspec_volatile [(match_operand 0 "" "")] VUNSPEC_POOL_4)]
   "TARGET_EITHER"
@@ -10577,15 +10613,12 @@
     switch (GET_MODE_CLASS (GET_MODE (x)))
       {
       case MODE_FLOAT:
- 	if (GET_MODE (x) == HFmode)
- 	  arm_emit_fp16_const (x);
- 	else
- 	  {
- 	    REAL_VALUE_TYPE r;
- 	    REAL_VALUE_FROM_CONST_DOUBLE (r, x);
- 	    assemble_real (r, GET_MODE (x), BITS_PER_WORD);
- 	  }
- 	break;
+	{
+	  REAL_VALUE_TYPE r;
+	  REAL_VALUE_FROM_CONST_DOUBLE (r, x);
+	  assemble_real (r, GET_MODE (x), BITS_PER_WORD);
+	  break;
+	}
       default:
 	/* XXX: Sometimes gcc does something really dumb and ends up with
 	   a HIGH in a constant pool entry, usually because it's trying to
diff --git a/gcc/config/arm/thumb1.md b/gcc/config/arm/thumb1.md
index 020d83b..80d470b 100644
--- a/gcc/config/arm/thumb1.md
+++ b/gcc/config/arm/thumb1.md
@@ -1735,33 +1735,6 @@
    (set_attr "conds" "clob")]
 )
 
-(define_insn "consttable_1"
-  [(unspec_volatile [(match_operand 0 "" "")] VUNSPEC_POOL_1)]
-  "TARGET_THUMB1"
-  "*
-  making_const_table = TRUE;
-  assemble_integer (operands[0], 1, BITS_PER_WORD, 1);
-  assemble_zeros (3);
-  return \"\";
-  "
-  [(set_attr "length" "4")
-   (set_attr "type" "no_insn")]
-)
-
-(define_insn "consttable_2"
-  [(unspec_volatile [(match_operand 0 "" "")] VUNSPEC_POOL_2)]
-  "TARGET_THUMB1"
-  "*
-  making_const_table = TRUE;
-  gcc_assert (GET_MODE_CLASS (GET_MODE (operands[0])) != MODE_FLOAT);
-  assemble_integer (operands[0], 2, BITS_PER_WORD, 1);
-  assemble_zeros (2);
-  return \"\";
-  "
-  [(set_attr "length" "4")
-   (set_attr "type" "no_insn")]
-)
-
 ;; Miscellaneous Thumb patterns
 (define_expand "tablejump"
   [(parallel [(set (pc) (match_operand:SI 0 "register_operand" ""))
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..e8209f6
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/constant-pool.c
@@ -0,0 +1,27 @@
+/* { dg-do run { target armeb-*-*eabi* } } */
+/* { dg-options "-O1" } */
+
+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 ();
+  if (v != 0x1234)
+    __builtin_abort ();
+  return 0;
+}


Is it ok for trunk?

Best regards,

Thomas