> -----Original Message-----
> From: Artemiy Volkov <[email protected]>
> Sent: 24 June 2026 18:44
> To: Tamar Christina <[email protected]>
> Cc: [email protected]; Wilco Dijkstra <[email protected]>;
> [email protected]; Richard Earnshaw
> <[email protected]>; [email protected]; Alice Carlotti
> <[email protected]>; Alex Coplan <[email protected]>
> Subject: Re: [PATCH] aarch64: define r->w moves for partial AdvSIMD vectors
> [PR125716]
>
> On Wed, Jun 24, 2026 at 05:27:33PM +0100, Tamar Christina wrote:
> > > -----Original Message-----
> > > From: Artemiy Volkov <[email protected]>
> > > Sent: 24 June 2026 17:00
> > > To: [email protected]
> > > Cc: Tamar Christina <[email protected]>; Wilco Dijkstra
> > > <[email protected]>; [email protected]; Richard
> > > Earnshaw <[email protected]>; [email protected]; Alice
> > > Carlotti <[email protected]>; Alex Coplan <[email protected]>;
> > > Artemiy Volkov <[email protected]>
> > > Subject: [PATCH] aarch64: define r->w moves for partial AdvSIMD vectors
> > > [PR125716]
> > >
> > > This is a fix for PR125716, where under specific circumstances we end
> > > up emitting an insn like:
> > >
> > > (insn 32 26 27 2 (set (reg:V2HI 63 v31 [orig:130 _13 ] [130])
> > > (reg:V2HI 0 x0 [orig:115 _13 ] [115])) "testcase.c":3:17 1392
> > > {*aarch64_simd_movv2hi}
> > > (nil))
> > >
> > > for which aarch64_simd_mov instruction variant taking sub-64-bit vector
> > > operands there is no (w,r) alternative and things blow up later during
> > > reload.
> > >
> > > This happens specifically in the case of reg-reg transfers and only
> > > when transfer cost between relevant register classes is 2, which, as I've
> > > learned thanks to Andrea's writeup at [0] (and as is confirmed by e.g.
> > > gcc/lra-constraints.cc:4245), is a magic value that tells LRA/reload to
> > > not
> > > try different reloads which would have helped here.
> > >
> > > The testcase added is a blend between those provided in PR125716 and in
> > > PR125947. As stable as affected tuning models may be, I've opted to
> > > create a dedicated JSON file for this test to guarantee a GP2FP cost of 2.
> > >
> > > Bootstrapped and regtested on aarch64-linux-gnu with and without
> > > -mtune=octeontx81.
> > >
> > > [0] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=125947#c1
> > >
> > > PR target/125716
> > >
> > > gcc/ChangeLog:
> > >
> > > * config/aarch64/aarch64-simd.md (*aarch64_simd_mov<mode>):
> > > Add
> > > (?w,r) alternative for VSUB64 types.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > > * gcc.target/aarch64/aarch64-json-tunings/gp2fp-2.json: New JSON
> > > tuning file.
> > > * gcc.target/aarch64/simd/pr125716.c: New test.
> > > ---
> > > gcc/config/aarch64/aarch64-simd.md | 1 +
> > > .../gcc.target/aarch64/aarch64-json-tunings/gp2fp-2.json | 7 +++++++
> > > gcc/testsuite/gcc.target/aarch64/simd/pr125716.c | 7 +++++++
> > > 3 files changed, 15 insertions(+)
> > > create mode 100644 gcc/testsuite/gcc.target/aarch64/aarch64-json-
> > > tunings/gp2fp-2.json
> > > create mode 100644 gcc/testsuite/gcc.target/aarch64/simd/pr125716.c
> > >
> > > diff --git a/gcc/config/aarch64/aarch64-simd.md
> > > b/gcc/config/aarch64/aarch64-simd.md
> > > index b2e8fe3f6a9..ec7bebff0bd 100644
> > > --- a/gcc/config/aarch64/aarch64-simd.md
> > > +++ b/gcc/config/aarch64/aarch64-simd.md
> > > @@ -304,6 +304,7 @@
> > > [r , m ; load_4 , * ] ldr<size>\t%w0, %1
> > > [w , w ; neon_logic , simd ] mov\t%0.8b, %1.8b
> > > [w , m ; neon_load1_1reg , simd ] ldr\t%<vstype>0, %1
> > > + [?w, r ; f_mcr , * ] fmov\t%d0, %1
> >
> > I'm not sure we can count on the value of the top bits to be correct
> > here. Should this instead be something like
> >
> > fmov\t%<vstype>0, %<single_wx>1
> >
> > so that for something like a V2QI we only transfer the actual 16-bits
> > of data? That'll be safer if the GPR register was created by something
> > like a paradoxical subreg.
>
> You are right that this is an oversight, but:
>
> (a) The second operand should be %w1 rather than %<single_wx>1 as we
> are limited to 32 bits,
Ah yeah, I thought VSUB64 was inclusive. But it's indeed exclusive.
> (b) The above still won't work when <vstype> is "h" for non-FP16 cores, so
> "*" should be changed to "fp16" and a fallback line added.
>
Yeah I guess without FP16 it'll end up spilling it which is likely better than
doing an AND + FMOV.
> Instead of all that, I think the easiest is to split here, since all the
> complexity is already handled elsewhere. So it looks like that line
> should just be
>
> + [?w, r ; f_mcr , * ] #
>
> Do you agree, and if so, should I respin or is it OK like that?
The above is OK if it passes regtest.
Thanks,
Tamar
>
> Thanks,
> Artemiy
>
> >
> > Thanks,
> > Tamar
> >
> > > [w , Dz ; neon_move , simd ] movi\t%0.2d, #
> > > [m , rZ ; store_4 , * ] str<size>\t%w1, %0
> > > [m , w ; neon_store1_1reg , simd ] str\t%<vstype>1, %0
> > > diff --git a/gcc/testsuite/gcc.target/aarch64/aarch64-json-tunings/gp2fp-
> > > 2.json b/gcc/testsuite/gcc.target/aarch64/aarch64-json-tunings/gp2fp-
> > > 2.json
> > > new file mode 100644
> > > index 00000000000..529efa84626
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.target/aarch64/aarch64-json-tunings/gp2fp-
> 2.json
> > > @@ -0,0 +1,7 @@
> > > +{
> > > + "tune_params": {
> > > + "regmove_cost": {
> > > + "GP2FP": 2
> > > + }
> > > + }
> > > +}
> > > diff --git a/gcc/testsuite/gcc.target/aarch64/simd/pr125716.c
> > > b/gcc/testsuite/gcc.target/aarch64/simd/pr125716.c
> > > new file mode 100644
> > > index 00000000000..d38290cbbe9
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.target/aarch64/simd/pr125716.c
> > > @@ -0,0 +1,7 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-options "-muser-provided-
> > > CPU=${srcdir}/gcc.target/aarch64/aarch64-json-tunings/gp2fp-2.json -O2
> -
> > > fno-vect-cost-model -fweb" } */
> > > +/* { dg-warning "JSON tuning file does not contain version information"
> > > ""
> {
> > > target *-*-* } 0 } */
> > > +
> > > +double d;
> > > +_Complex short cs;
> > > +void foo() { cs *= d; }
> > > --
> > > 2.43.0
> >
