On Fri, Dec 18, 2015 at 12:13:31PM +0000, James Greenhalgh wrote:
>
> On Mon, Dec 14, 2015 at 11:49:26AM +0000, Marcus Shawcroft wrote:
> > On 14 December 2015 at 11:01, James Greenhalgh <james.greenha...@arm.com>
> > wrote:
> > > On Wed, Dec 09, 2015 at 01:13:20PM +0000, Marcus Shawcroft wrote:
> > >> On 27 November 2015 at 13:01, James Greenhalgh
> > >> <james.greenha...@arm.com> wrote:
> > >>
> > >> > 2015-11-27 James Greenhalgh <james.greenha...@arm.com>
> > >> >
> > >> > * config/aarch64/aarch64-protos.h
> > >> > (aarch64_cannot_change_mode_class): Bring back.
> > >> > * config/aarch64/aarch64.c
> > >> > (aarch64_cannot_change_mode_class): Likewise.
> > >> > * config/aarch64/aarch64.h (CANNOT_CHANGE_MODE_CLASS):
> > >> > Likewise.
> > >> > * config/aarch64/aarch64.md (aarch64_movdi_<mode>low): Use
> > >> > zero_extract rather than truncate.
> > >> > (aarch64_movdi_<mode>high): Likewise.
> > >> >
> > >> > 2015-11-27 James Greenhalgh <james.greenha...@arm.com>
> > >> >
> > >> > * gcc.dg/torture/pr67609.c: New.
> > >> >
> > >>
> > >> + detailed dicussion. In all other cases, we want to be premissive
> > >>
> > >> s/premissive/permissive/
> > >>
> > >> OK /Marcus
> > >
> > > Thanks.
> > >
> > > This has had a week or so to soak on trunk now, is it OK to backport to
> > > GCC
> > > 5 and 4.9?
> > >
> > > The patch applies as-good-as clean, with only a little bit to fix up in
> > > aarch64-protos.h to keep alphabetical order, and I've bootstrapped and
> > > tested
> > > the backports with no issue.
> >
> > OK /Marcus
> >
>
> Looking back at the patch just before I hit commit, the 4.9 backport was
> a little different (as we still have a CANNOT_CHANGE_MODE_CLASS there).
> We can drop the aarch64-protos.h and aarch64.h changes, and we need to
> change the sense of the new check, such that we can return true for the
> case added by this patch, and false for the limited number of other safe
> cases in 4.9.
*ping*
Thanks,
James
>
> Bootstrapped on aarch64-none-linux-gnu.
>
> OK?
>
> Thanks,
> James
>
> ---
> gcc/
>
> 2015-12-14 James Greenhalgh <james.greenha...@arm.com>
>
> Backport from mainline.
> 2015-12-09 James Greenhalgh <james.greenha...@arm.com>
>
> PR rtl-optimization/67609
> * config/aarch64/aarch64.c
> (aarch64_cannot_change_mode_class): Don't permit word_mode
> subregs of full vector modes.
> * config/aarch64/aarch64.md (aarch64_movdi_<mode>low): Use
> zero_extract rather than truncate.
> (aarch64_movdi_<mode>high): Likewise.
>
> gcc/testsuite/
>
> 2015-12-14 James Greenhalgh <james.greenha...@arm.com>
>
> Backport from mainline.
> 2015-12-09 James Greenhalgh <james.greenha...@arm.com>
>
> PR rtl-optimization/67609
> * gcc.dg/torture/pr67609.c: New.
>
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 8153997..5ca38b6 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -8405,6 +8405,18 @@ aarch64_cannot_change_mode_class (enum machine_mode
> from,
> enum machine_mode to,
> enum reg_class rclass)
> {
> + /* We cannot allow word_mode subregs of full vector modes.
> + Otherwise the middle-end will assume it's ok to store to
> + (subreg:DI (reg:TI 100) 0) in order to modify only the low 64 bits
> + of the 128-bit register. However, after reload the subreg will
> + be dropped leaving a plain DImode store. See PR67609 for a more
> + detailed dicussion. In some other cases we can be permissive and
> + return false. */
> + if (reg_classes_intersect_p (FP_REGS, rclass)
> + && GET_MODE_SIZE (to) == UNITS_PER_WORD
> + && GET_MODE_SIZE (from) > UNITS_PER_WORD)
> + return true;
> +
> /* Full-reg subregs are allowed on general regs or any class if they are
> the same size. */
> if (GET_MODE_SIZE (from) == GET_MODE_SIZE (to)
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index 24bb029..d6c6b1e 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -3454,7 +3454,8 @@
>
> (define_insn "aarch64_movdi_<mode>low"
> [(set (match_operand:DI 0 "register_operand" "=r")
> - (truncate:DI (match_operand:TX 1 "register_operand" "w")))]
> + (zero_extract:DI (match_operand:TX 1 "register_operand" "w")
> + (const_int 64) (const_int 0)))]
> "reload_completed || reload_in_progress"
> "fmov\\t%x0, %d1"
> [(set_attr "type" "f_mrc")
> @@ -3463,9 +3464,8 @@
>
> (define_insn "aarch64_movdi_<mode>high"
> [(set (match_operand:DI 0 "register_operand" "=r")
> - (truncate:DI
> - (lshiftrt:TX (match_operand:TX 1 "register_operand" "w")
> - (const_int 64))))]
> + (zero_extract:DI (match_operand:TX 1 "register_operand" "w")
> + (const_int 64) (const_int 64)))]
> "reload_completed || reload_in_progress"
> "fmov\\t%x0, %1.d[1]"
> [(set_attr "type" "f_mrc")
> diff --git a/gcc/testsuite/gcc.dg/torture/pr67609.c
> b/gcc/testsuite/gcc.dg/torture/pr67609.c
> new file mode 100644
> index 0000000..817857d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/torture/pr67609.c
> @@ -0,0 +1,33 @@
> +/* { dg-do run } */
> +
> +typedef union
> +{
> + double v[2];
> + double s __attribute__ ((vector_size (16)));
> +} data;
> +
> +data reg;
> +
> +void __attribute__ ((noinline))
> +set_lower (double b)
> +{
> + data stack_var;
> + double __attribute__ ((vector_size (16))) one = { 1.0, 1.0 };
> + stack_var.s = reg.s;
> + stack_var.s += one;
> + stack_var.v[0] += b;
> + reg.s = stack_var.s;
> +}
> +
> +int
> +main (int argc, char ** argv)
> +{
> + reg.v[0] = 1.0;
> + reg.v[1] = 1.0;
> + /* reg should contain { 1.0, 1.0 }. */
> + set_lower (2.0);
> + /* reg should contain { 4.0, 2.0 }. */
> + if ((int) reg.v[0] != 4 || (int) reg.v[1] != 2)
> + __builtin_abort ();
> + return 0;
> +}
