> -----Original Message-----
> From: Richard Sandiford <richard.sandif...@arm.com>
> Sent: Monday, March 3, 2025 10:12 AM
> To: Tamar Christina <tamar.christ...@arm.com>
> Cc: gcc-patches@gcc.gnu.org; nd <n...@arm.com>; Richard Earnshaw
> <richard.earns...@arm.com>; ktkac...@gcc.gnu.org
> Subject: Re: [PATCH]AArch64: force operand to fresh register to avoid subreg
> issues [PR118892]
> 
> Tamar Christina <tamar.christ...@arm.com> writes:
> > Hi All,
> >
> > When the input is already a subreg and we try to make a paradoxical
> > subreg out of it for copysign this can fail if it violates the sugreg
> 
> subreg
> 
> > relationship.
> >
> > Use force_lowpart_subreg instead of lowpart_subreg to then force the
> > results to a register instead of ICEing.
> >
> > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
> >
> > Ok for master?
> >
> > Thanks,
> > Tamar
> >
> > gcc/ChangeLog:
> >
> >     PR target/118892
> >     * config/aarch64/aarch64.md (copysign<GPF:mode>3): Use
> >     force_lowpart_subreg instead of lowpart_subreg.
> >
> > gcc/testsuite/ChangeLog:
> >
> >     PR target/118892
> >     * gcc.target/aarch64/copysign-pr118892.c: New test.
> >
> > ---
> >
> > diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> > index
> cfe730f3732ce45c914b30a908851a4a7dd77c0f..62be9713cf417922b3c06e38f
> 12f401872751fa2 100644
> > --- a/gcc/config/aarch64/aarch64.md
> > +++ b/gcc/config/aarch64/aarch64.md
> > @@ -7479,8 +7479,8 @@ (define_expand "copysign<GPF:mode>3"
> >        && real_isneg (CONST_DOUBLE_REAL_VALUE (op2_elt)))
> >      {
> >        emit_insn (gen_ior<vq_int_equiv>3 (
> > -   lowpart_subreg (<VQ_INT_EQUIV>mode, operands[0], <MODE>mode),
> > -   lowpart_subreg (<VQ_INT_EQUIV>mode, operands[1], <MODE>mode),
> > +   force_lowpart_subreg (<VQ_INT_EQUIV>mode, operands[0],
> <MODE>mode),
> > +   force_lowpart_subreg (<VQ_INT_EQUIV>mode, operands[1],
> <MODE>mode),
> 
> force_lowpart_subreg conditionally forces the SUBREG_REG into a new temporary
> register and then takes the subreg of that.  It's therefore only appropriate
> for source operands, not destination operands.
> 
> It's true that the same problem could in principle occur for the
> destination, but that would need to be fixed in a different way.
> 

Ah, true. Should have thought about it a bit more.

> OK with just the operands[1] change, without the operands[0] change.
> 

I forgot to ask if OK for GCC 14 backport after some stew.

Thanks,
Tamar
> Thanks,
> Richard
> 
> 
> >     v_bitmask));
> >        DONE;
> >      }
> > diff --git a/gcc/testsuite/gcc.target/aarch64/copysign-pr118892.c
> b/gcc/testsuite/gcc.target/aarch64/copysign-pr118892.c
> > new file mode 100644
> > index
> 0000000000000000000000000000000000000000..adfa30dc3e2db895af4f205
> 7bdd1011fdb7d4537
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/aarch64/copysign-pr118892.c
> > @@ -0,0 +1,11 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-Ofast" } */
> > +
> > +double l();
> > +double f()
> > +{
> > +  double t6[2] = {l(), l()};
> > +  double t7[2];
> > +  __builtin_memcpy(&t7, &t6, sizeof(t6));
> > +  return -__builtin_fabs(t7[1]);
> > +}

Reply via email to