Tamar Christina <tamar.christ...@arm.com> writes:
>> -----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.
Yeah, ok for GCC 14 too. The force_lowpart_subreg function hasn't been
backported to GCC 14 yet, but I think it should be (as part of this patch).
Other backportable fixes rely on it too.
Thanks,
Richard
>
> 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]);
>> > +}
