> -----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]); > > +}