Fixed the subject. Best regards,
Thomas > -----Original Message----- > From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- > ow...@gcc.gnu.org] On Behalf Of Thomas Preud'homme > Sent: Tuesday, November 11, 2014 3:31 PM > To: gcc-patches@gcc.gnu.org; Ramana Radhakrishnan; Richard Earnshaw > Subject: [PATCH][ARM] Fix PR59593/PR63742: arm *movhi_insn_arch4 > pattern for big-endian > > 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 > > >
