> -----Original Message-----
> From: Andrea Pinski <[email protected]>
> Sent: 24 June 2026 18:28
> To: Tamar Christina <[email protected]>
> Cc: Artemiy Volkov <[email protected]>; [email protected];
> Wilco Dijkstra <[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 9:28 AM Tamar Christina
> <[email protected]> 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.
> 
> Note `fmov hN, wN` only exists if FEAT_FP16 . So there might need more
> work done in this case.

Good point, I forgot that unlike loads and stores, h fmov isn't in base Armv8-a.

Thanks,
Tamar

> 
> Thanks,
> Andrea
> 
> >
> > Thanks,
> > Tamar
> >
> > >       [w , Dz ; neon_move        , simd ] movi\t%0.2d, #0
> > >       [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
> >

Reply via email to