This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
RE: [PATCH, ARM, ping2] Backport fix for PR59593 (minipool of small values on big endian targets)
- From: "Thomas Preud'homme" <thomas dot preudhomme at arm dot com>
- To: <gcc-patches at gcc dot gnu dot org>, "Richard Earnshaw" <Richard dot Earnshaw at arm dot com>, "Ramana Radhakrishnan" <Ramana dot Radhakrishnan at arm dot com>, "Marcus Shawcroft" <Marcus dot Shawcroft at arm dot com>
- Date: Tue, 17 Feb 2015 16:06:17 +0800
- Subject: RE: [PATCH, ARM, ping2] Backport fix for PR59593 (minipool of small values on big endian targets)
- Authentication-results: sourceware.org; auth=none
Ping?
> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Thomas Preud'homme
> Sent: Tuesday, February 10, 2015 4:47 PM
> To: gcc-patches@gcc.gnu.org; Richard Earnshaw; Ramana Radhakrishnan;
> Marcus Shawcroft
> Subject: RE: [PATCH, ARM] Backport fix for PR59593 (minipool of small
> values on big endian targets)
>
> Ping?
>
> > -----Original Message-----
> > From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> > owner@gcc.gnu.org] On Behalf Of Thomas Preud'homme
> > Sent: Tuesday, January 20, 2015 1:06 PM
> > To: gcc-patches@gcc.gnu.org; Richard Earnshaw; Ramana
> Radhakrishnan;
> > Marcus Shawcroft
> > Subject: [PATCH, ARM] Backport fix for PR59593 (minipool of small
> values
> > on big endian targets)
> >
> > 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.preudhomme@arm.com>
> >
> > Backport from mainline
> > 2014-11-27 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): 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.preudhomme@arm.com>
> >
> > Backport from mainline
> > 2014-11-27 Thomas Preud'homme <thomas.preudhomme@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.preudhomme@arm.com>
> > +
> > + Backport from mainline
> > + 2014-11-27 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): 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 <polacek@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.preudhomme@arm.com>
> > +
> > + Backport from mainline
> > + 2014-11-27 Thomas Preud'homme
> > <thomas.preudhomme@arm.com>
> > +
> > + PR target/59593
> > + * gcc.target/arm/constant-pool.c: New test.
> > +
> > 2015-01-14 Marek Polacek <polacek@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?
> >
> > Best regards,
> >
> > Thomas
> >
> >
> >
>
>
>
>