Hi Jeevitha, Have a few suggestions on this patch and a query
On Thu, 2025-08-21 at 18:26 +0530, jeevitha wrote: > > Hi All, > > The following patch has been bootstrapped and regtested on > powerpc64le-linux. > > Disable easy_vector_constant_msb and easy_vector_constant_add_self > splitters on > POWER10. On POWER9, bypass these splitters when xxspltib can > directly generate > 8-bit signed constants, for avoiding redundant vaddubm/vslb > sequences. For > word constants on POWER9, retain the splitters since xxspltiw is not > available. > > 2025-07-21 Jeevitha Palanisamy <[email protected]> > > gcc/ > PR target/118480 > * config/rs6000/altivec.md (easy_vector_constant_msb > splitter): Disable > on POWER10, and on POWER9 when xxspltib can generate the > constant. > (easy_vector_constant_add_self splitter): Likewise > > gcc/testsuite/ > PR target/118480 > * gcc.target/powerpc/pr118480-1.c: New > * gcc.target/powerpc/pr118480-2.c: New > * gcc.target/powerpc/vec-splat-constant-v4sf.c: Adjust test > to handle > failed case by msb splitter. > > > diff --git a/gcc/config/rs6000/altivec.md > b/gcc/config/rs6000/altivec.md > index 7edc288a656..509681c4e05 100644 > --- a/gcc/config/rs6000/altivec.md > +++ b/gcc/config/rs6000/altivec.md > @@ -283,9 +283,14 @@ > (define_split > [(set (match_operand:VM 0 "altivec_register_operand") > (match_operand:VM 1 "easy_vector_constant_msb"))] > - "VECTOR_UNIT_ALTIVEC_OR_VSX_P (<MODE>mode) && reload_completed" > + "VECTOR_UNIT_ALTIVEC_OR_VSX_P (<MODE>mode) && reload_completed > + && !TARGET_POWER10" > [(const_int 0)] > { > + int value = 256; > + int num_insns = -1; > + if (xxspltib_constant_p (operands[1], <MODE>mode, &num_insns, > &value)) > + DONE; > rtx dest = operands[0]; > machine_mode mode; > rtvec v; > @@ -321,10 +326,15 @@ > (define_split > [(set (match_operand:VM 0 "altivec_register_operand") > (match_operand:VM 1 "easy_vector_constant_add_self"))] > - "VECTOR_UNIT_ALTIVEC_OR_VSX_P (<MODE>mode) && reload_completed" > + "VECTOR_UNIT_ALTIVEC_OR_VSX_P (<MODE>mode) && reload_completed > + && !TARGET_POWER10" > [(set (match_dup 0) (match_dup 3)) > (set (match_dup 0) (match_dup 4))] > { > + int value = 256; > + int num_insns = -1; > + if (xxspltib_constant_p (operands[1], <MODE>mode, &num_insns, > &value)) > + DONE; Since you are adding a check for the operands[1] explicitly, I think we can directly express it in the operand predicate something like: (define_predicate "easy_vector_constant_add_self" (and (match_code "const_vector") (and (match_test "TARGET_ALTIVEC") (and (match_test "easy_altivec_constant (op, mode)") (match_test "!xxspltib_constant_nosplit (op,mode)")))) It would apply to both patterns. Also I do not understand why disable these patterns for power10? A testcase to demonstrate it would be helpful, where if we had this pattern enabled for power10, it generates bad code specially for this particular split pattern? I see that the test case in vec-splat- constant-v4sf.c test case covers the case for prior split pattern. Thanks and regards, Avinash Jayakar > rtx dup = gen_easy_altivec_constant (operands[1]); > rtx const_vec; > machine_mode op_mode = <MODE>mode; > diff --git a/gcc/testsuite/gcc.target/powerpc/pr118480-1.c > b/gcc/testsuite/gcc.target/powerpc/pr118480-1.c > new file mode 100644 > index 00000000000..6e1d7b45fc1 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/pr118480-1.c > @@ -0,0 +1,18 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -mdejagnu-cpu=power9 -mvsx" } */ > + > +/* On P9, vec_splats(18) should use xxspltib directly, > + not the add-self sequence with vaddubm. */ > + > +#include <altivec.h> > + > +typedef vector unsigned char vui8_t; > + > +vui8_t test_splat7_char_18(void) > +{ > + return vec_splats((unsigned char)18); > +} > + > +/* { dg-final { scan-assembler "xxspltib" } } */ > +/* { dg-final { scan-assembler-not "vaddubm" } } */ > + > diff --git a/gcc/testsuite/gcc.target/powerpc/pr118480-2.c > b/gcc/testsuite/gcc.target/powerpc/pr118480-2.c > new file mode 100644 > index 00000000000..8c858231848 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/pr118480-2.c > @@ -0,0 +1,17 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -mdejagnu-cpu=power9 -mvsx" } */ > + > +/* On P9, vec_splats(-128) should be a single xxspltib, > + not the msb-splitter sequence with vslb. */ > + > +#include <altivec.h> > + > +typedef vector signed char vui8_t; > + > +vui8_t test_msb(void) > +{ > + return vec_splats((signed char)-128); > +} > + > + /* { dg-final { scan-assembler "xxspltib" } } */ > + /* { dg-final { scan-assembler-not "vslb" } } */ > diff --git a/gcc/testsuite/gcc.target/powerpc/vec-splat-constant- > v4sf.c b/gcc/testsuite/gcc.target/powerpc/vec-splat-constant-v4sf.c > index 1f0475cf47a..8f742df2f9f 100644 > --- a/gcc/testsuite/gcc.target/powerpc/vec-splat-constant-v4sf.c > +++ b/gcc/testsuite/gcc.target/powerpc/vec-splat-constant-v4sf.c > @@ -33,7 +33,7 @@ v4sf_const_inf (void) > vector float > v4sf_const_m0 (void) > { > - return (vector float) { -0.0f, -0.0f, -0.0f, -0.0f }; /* > XXSPLTIB/VSLW. */ > + return (vector float) { -0.0f, -0.0f, -0.0f, -0.0f }; /* > XXSPLTIW */ > } > > vector float > @@ -57,11 +57,9 @@ v4sf_splats_inf (void) > vector float > v8hi_splats_m0 (void) > { > - return vec_splats (-0.0f); /* > XXSPLTIB/VSLW. */ > + return vec_splats (-0.0f); /* XXSPLTIW > */ > } > > -/* { dg-final { scan-assembler-times {\mxxspltiw\M} 6 } } */ > -/* { dg-final { scan-assembler-times {\mxxspltib\M} 2 } } */ > -/* { dg-final { scan-assembler-times {\mvslw\M} 2 } } */ > +/* { dg-final { scan-assembler-times {\mxxspltiw\M} 8 } } */ > /* { dg-final { scan-assembler-not {\mlxvx?\M} } } */ > /* { dg-final { scan-assembler-not {\mplxv\M} } } */
