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.preudho...@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.preudho...@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
