> From: Ramana Radhakrishnan [mailto:ramana....@googlemail.com] > Sent: Tuesday, February 17, 2015 4:08 PM > To: Thomas Preud'homme > Cc: gcc-patches; Richard Earnshaw; Ramana Radhakrishnan; Marcus > Shawcroft; Richard Biener; Jakub Jelinek > Subject: Re: [PATCH, ARM] Backport fix for PR59593 (minipool of small > values on big endian targets) > > On Tue, Jan 20, 2015 at 5:06 AM, Thomas Preud'homme > <thomas.preudho...@arm.com> wrote: > > Currently on GCC 4.8 and 4.9, 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 backport of commit r218118. > > > > *** gcc/ChangeLog *** > > > > 2015-01-14 Thomas Preud'homme <thomas.preudho...@arm.com> > > > > Backport from mainline > > 2014-11-27 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): Make it TARGET_EITHER. > > (consttable_2): Make it TARGET_EITHER and move HFmode handling > from > > consttable_4 to it. > > (consttable_4): Move HFmode handling to consttable_2 pattern. > > > > > > *** gcc/testsuite/ChangeLog *** > > > > 2015-01-14 Thomas Preud'homme <thomas.preudho...@arm.com> > > > > Backport from mainline > > 2014-11-27 Thomas Preud'homme <thomas.preudho...@arm.com> > > > > PR target/59593 > > * gcc.target/arm/constant-pool.c: New test. > > > > > > diff --git a/gcc/ChangeLog b/gcc/ChangeLog > > index 85372e5..eeaece8 100644 > > --- a/gcc/ChangeLog > > +++ b/gcc/ChangeLog > > @@ -1,3 +1,16 @@ > > +2015-01-14 Thomas Preud'homme > <thomas.preudho...@arm.com> > > + > > + Backport from mainline > > + 2014-11-27 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): Make it TARGET_EITHER. > > + (consttable_2): Make it TARGET_EITHER and move HFmode > handling from > > + consttable_4 to it. > > + (consttable_4): Move HFmode handling to consttable_2 pattern. > > + > > 2015-01-14 Marek Polacek <pola...@redhat.com> > > > > Backport from mainline > > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c > > index 5e2571c..67ef80b 100644 > > --- a/gcc/config/arm/arm.c > > +++ b/gcc/config/arm/arm.c > > @@ -16274,7 +16274,7 @@ dump_minipool (rtx 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 8119387..93b25e9 100644 > > --- a/gcc/config/arm/arm.md > > +++ b/gcc/config/arm/arm.md > > @@ -12224,7 +12224,7 @@ > > > > (define_insn "consttable_1" > > [(unspec_volatile [(match_operand 0 "" "")] VUNSPEC_POOL_1)] > > - "TARGET_THUMB1" > > + "TARGET_EITHER" > > "* > > making_const_table = TRUE; > > assemble_integer (operands[0], 1, BITS_PER_WORD, 1); > > @@ -12237,14 +12237,23 @@ > > > > (define_insn "consttable_2" > > [(unspec_volatile [(match_operand 0 "" "")] VUNSPEC_POOL_2)] > > - "TARGET_THUMB1" > > + "TARGET_EITHER" > > "* > > - 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 \"\"; > > - " > > + { > > + 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")] > > ) > > @@ -12259,15 +12268,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/testsuite/ChangeLog b/gcc/testsuite/ChangeLog > > index 222716d..49d1a73 100644 > > --- a/gcc/testsuite/ChangeLog > > +++ b/gcc/testsuite/ChangeLog > > @@ -1,3 +1,11 @@ > > +2015-01-14 Thomas Preud'homme > <thomas.preudho...@arm.com> > > + > > + Backport from mainline > > + 2014-11-27 Thomas Preud'homme > <thomas.preudho...@arm.com> > > + > > + PR target/59593 > > + * gcc.target/arm/constant-pool.c: New test. > > + > > 2015-01-14 Marek Polacek <pola...@redhat.com> > > > > Backport from mainline > > 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..8427dfb > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/arm/constant-pool.c > > @@ -0,0 +1,27 @@ > > +/* { dg-do run } */ > > +/* { 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 this ok for 4.8 and 4.9 branches? > > Ok if no regressions and no RM objects to this in 24 hours. I think > it's baked sufficiently on trunk to backport now.
Done. Best regards, Thomas
