[PATCH] IBM Z: Fix testcase vcond-shift.c

Stefan Schulze Frielinghaus stefansf@linux.ibm.com
Tue Mar 2 07:42:29 GMT 2021


On Tue, Mar 02, 2021 at 08:08:14AM +0100, Andreas Krebbel wrote:
> On 3/1/21 5:00 PM, Stefan Schulze Frielinghaus wrote:
> > As of commit 3a6e3ad38a17a03ee0139b49a0946e7b9ded1eb1 expressions
> > x CMP y ? -1 : 0 are fold into x CMP y.  Due to this we do not see
> > shifts anymore after expand in our testcases but comparisons.  Thus
> > replace instructions vesraX by corresponding vchX.  Keep testcases
> > vchX_{lt,gt} where only a relational comparison is done and no shift in
> > order to keep test coverage for vectorization.
> 
> The vcond-shift optimization verified by the testcase is currently implemented in s390_expand_vcond
> but due to the common code change we go the vec_cmp route now. So we probably should do the same
> also in s390_expand_vec_compare now. Perhaps like this ... it appears to fix the testcase for me:
> 
> diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
> index 9d2cee950d0b..9d9f5a0f6f4e 100644
> --- a/gcc/config/s390/s390.c
> +++ b/gcc/config/s390/s390.c
> @@ -6562,6 +6562,7 @@ s390_expand_vec_compare (rtx target, enum rtx_code cond,
> 
>    if (GET_MODE_CLASS (GET_MODE (cmp_op1)) == MODE_VECTOR_FLOAT)
>      {
> +      cmp_op2 = force_operand (cmp_op2, 0);
>        switch (cond)
>         {
>           /* NE a != b -> !(a == b) */
> @@ -6600,6 +6601,19 @@ s390_expand_vec_compare (rtx target, enum rtx_code cond,
>      }
>    else
>      {
> +      /* Turn x < 0 into x >> (bits - )  */
> +      if (cond == LT && cmp_op2 == CONST0_RTX (mode))
> +       {
> +         int shift = GET_MODE_BITSIZE (GET_MODE_INNER (mode)) - 1;
> +         rtx res = expand_simple_binop (mode, ASHIFTRT, cmp_op1,
> +                                        GEN_INT (shift), target,
> +                                        0, OPTAB_DIRECT);
> +         if (res != target)
> +           emit_move_insn (target, res);
> +         return;
> +       }
> +      cmp_op2 = force_operand (cmp_op2, 0);
> +
>        switch (cond)
>         {
>           /* NE: a != b -> !(a == b) */
> diff --git a/gcc/config/s390/vector.md b/gcc/config/s390/vector.md
> index bc52211c55e5..c80d582a300d 100644
> --- a/gcc/config/s390/vector.md
> +++ b/gcc/config/s390/vector.md
> @@ -1589,7 +1589,7 @@
>    [(set (match_operand:<TOINTVEC>  0 "register_operand" "")
>         (match_operator:<TOINTVEC> 1 "vcond_comparison_operator"
>           [(match_operand:V_HW     2 "register_operand" "")
> -          (match_operand:V_HW     3 "register_operand" "")]))]
> +          (match_operand:V_HW     3 "nonmemory_operand" "")]))]
>    "TARGET_VX"
>  {
>    s390_expand_vec_compare (operands[0], GET_CODE(operands[1]), operands[2], operands[3]);

Sounds great to me.  Also eliminates the extra vzero :)

Cheers,
Stefan

> 
> Andreas
> 
> 
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* gcc.target/s390/vector/vcond-shift.c: Replace vesraX
> > 	instructions by corresponding vchX instructions.
> > ---
> >  .../gcc.target/s390/vector/vcond-shift.c      | 31 ++++++++++---------
> >  1 file changed, 17 insertions(+), 14 deletions(-)
> > 
> > diff --git a/gcc/testsuite/gcc.target/s390/vector/vcond-shift.c b/gcc/testsuite/gcc.target/s390/vector/vcond-shift.c
> > index a6b4e97aa50..9e472aef960 100644
> > --- a/gcc/testsuite/gcc.target/s390/vector/vcond-shift.c
> > +++ b/gcc/testsuite/gcc.target/s390/vector/vcond-shift.c
> > @@ -3,10 +3,13 @@
> >  /* { dg-do compile { target { s390*-*-* } } } */
> >  /* { dg-options "-O3 -march=z13 -mzarch" } */
> >  
> > -/* { dg-final { scan-assembler-times "vesraf\t%v.?,%v.?,31" 6 } } */
> > -/* { dg-final { scan-assembler-times "vesrah\t%v.?,%v.?,15" 6 } } */
> > -/* { dg-final { scan-assembler-times "vesrab\t%v.?,%v.?,7" 6 } } */
> > -/* { dg-final { scan-assembler-not "vzero\t*" } } */
> > +/* { dg-final { scan-assembler-times "vzero\t" 9 } } */
> > +/* { dg-final { scan-assembler-times "vchf\t" 6 } } */
> > +/* { dg-final { scan-assembler-times "vesraf\t%v.?,%v.?,1" 2 } } */
> > +/* { dg-final { scan-assembler-times "vchh\t" 6 } } */
> > +/* { dg-final { scan-assembler-times "vesrah\t%v.?,%v.?,1" 2 } } */
> > +/* { dg-final { scan-assembler-times "vchb\t" 6 } } */
> > +/* { dg-final { scan-assembler-times "vesrab\t%v.?,%v.?,1" 2 } } */
> >  /* { dg-final { scan-assembler-times "vesrlf\t%v.?,%v.?,31" 4 } } */
> >  /* { dg-final { scan-assembler-times "vesrlh\t%v.?,%v.?,15" 4 } } */
> >  /* { dg-final { scan-assembler-times "vesrlb\t%v.?,%v.?,7" 4 } } */
> > @@ -15,19 +18,19 @@
> >  #define ITER(X) (2 * (16 / sizeof (X[1])))
> >  
> >  void
> > -vesraf_div (int *x)
> > +vchf_vesraf_div (int *x)
> >  {
> >    int i;
> >    int *xx = __builtin_assume_aligned (x, 8);
> >  
> >    /* Should expand to (xx + (xx < 0 ? 1 : 0)) >> 1
> > -     which in turn should get simplified to (xx + (xx >> 31)) >> 1.  */
> > +     which in turn should get simplified to (xx - (xx < 0)) >> 1.  */
> >    for (i = 0; i < ITER (xx); i++)
> >      xx[i] = xx[i] / 2;
> >  }
> >  
> >  void
> > -vesrah_div (short *x)
> > +vchh_vesrah_div (short *x)
> >  {
> >    int i;
> >    short *xx = __builtin_assume_aligned (x, 8);
> > @@ -38,7 +41,7 @@ vesrah_div (short *x)
> >  
> >  
> >  void
> > -vesrab_div (signed char *x)
> > +vchb_vesrab_div (signed char *x)
> >  {
> >    int i;
> >    signed char *xx = __builtin_assume_aligned (x, 8);
> > @@ -50,7 +53,7 @@ vesrab_div (signed char *x)
> >  
> >  
> >  int
> > -vesraf_lt (int *x)
> > +vchf_lt (int *x)
> >  {
> >    int i;
> >    int *xx = __builtin_assume_aligned (x, 8);
> > @@ -60,7 +63,7 @@ vesraf_lt (int *x)
> >  }
> >  
> >  int
> > -vesrah_lt (short *x)
> > +vchh_lt (short *x)
> >  {
> >    int i;
> >    short *xx = __builtin_assume_aligned (x, 8);
> > @@ -70,7 +73,7 @@ vesrah_lt (short *x)
> >  }
> >  
> >  int
> > -vesrab_lt (signed char *x)
> > +vchb_lt (signed char *x)
> >  {
> >    int i;
> >    signed char *xx = __builtin_assume_aligned (x, 8);
> > @@ -82,7 +85,7 @@ vesrab_lt (signed char *x)
> >  
> >  
> >  int
> > -vesraf_ge (int *x)
> > +vchf_ge (int *x)
> >  {
> >    int i;
> >    int *xx = __builtin_assume_aligned (x, 8);
> > @@ -92,7 +95,7 @@ vesraf_ge (int *x)
> >  }
> >  
> >  int
> > -vesrah_ge (short *x)
> > +vchh_ge (short *x)
> >  {
> >    int i;
> >    short *xx = __builtin_assume_aligned (x, 8);
> > @@ -102,7 +105,7 @@ vesrah_ge (short *x)
> >  }
> >  
> >  int
> > -vesrab_ge (signed char *x)
> > +vchb_ge (signed char *x)
> >  {
> >    int i;
> >    signed char *xx = __builtin_assume_aligned (x, 8);
> > 
> 


More information about the Gcc-patches mailing list