[PATCH] D133700: [PowerPC] Exploit xxperm, check for dead vectors and substitute vperm with xxperm

[Stefan Pintilie via Phabricator via llvm-commits]

stefanp added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrVSX.td:91
+  SDTCisVT<0, v2f64>, SDTCisVT<1, v2f64>,
+  SDTCisVT<2, v2f64>]>;
 //--------------------------- Custom PPC nodes -------------------------------//
----------------
maryammo wrote:
> stefanp wrote:
> > maryammo wrote:
> > > stefanp wrote:
> > > > Is it possible to add a type constraint for the last operand here?  
> > > > ```
> > > > SDTCisVT<3, v4i32>
> > > > ```
> > > > Or is that going to cause issues elsewhere?
> > > The last one is  SDTCisVT<2, v2f64> that has a different type constraint, are you suggesting to change it?
> > No, I think that constraint is fine.
> > What I'm saying is that this `SDTypeProfile<1, 3,` has 1 output and 3 inputs. 
> > Currently, the output and the first 2 inputs have a constraint but the last input doesn't have a constraint.
> > 
> > So, what I'm thinking of is:
> > ```
> > --- a/llvm/lib/Target/PowerPC/PPCInstrVSX.td
> > +++ b/llvm/lib/Target/PowerPC/PPCInstrVSX.td
> > @@ -88,7 +88,7 @@ def SDT_PPCst_vec_be : SDTypeProfile<0, 2, [
> >  
> >  def SDT_PPCxxperm : SDTypeProfile<1, 3, [
> >    SDTCisVT<0, v2f64>, SDTCisVT<1, v2f64>,
> > -  SDTCisVT<2, v2f64>]>;
> > +  SDTCisVT<2, v2f64>, SDTCisVT<3, v4i32>]>;
> >  //--------------------------- Custom PPC nodes -------------------------------//
> >  def PPClxvd2x  : SDNode<"PPCISD::LXVD2X", SDT_PPClxvd2x,
> >                          [SDNPHasChain, SDNPMayLoad, SDNPMemOperand]>;
> > @@ -4151,8 +4151,8 @@ def : Pat<(v8i16 (PPCldsplat ForceXForm:$A)),
> >            (v8i16 (VSPLTHs 3, (LXSIHZX ForceXForm:$A)))>;
> >  def : Pat<(v16i8 (PPCldsplat ForceXForm:$A)),
> >            (v16i8 (VSPLTBs 7, (LXSIBZX ForceXForm:$A)))>;
> > -def : Pat<(v2f64 (PPCxxperm v2f64:$XT, v2f64:$XB, (v16i8 (bitconvert v4i32:$C)))),
> > -          (XXPERM v2f64:$XT, v2f64:$XB, v4i32:$C)>;
> >  def : Pat<(v2f64 (PPCxxperm v2f64:$XT, v2f64:$XB, v4i32:$C)),
> >            (XXPERM v2f64:$XT, v2f64:$XB, v4i32:$C)>;
> >  } // HasVSX, HasP9Vector
> > ```
> Such a change causes build failure which seems to be related to 
> def : Pat<(v2f64 (PPCxxperm v2f64:$XT, v2f64:$XB, (v16i8 (bitconvert v4i32:$C)))),
>                              (XXPERM v2f64:$XT, v2f64:$XB, v4i32:$C)>;
> 
Right which is why I don't think you need those two lines for that pattern.
The two patterns:
```
  def : Pat<(v2f64 (PPCxxperm v2f64:$XT, v2f64:$XB, (v16i8 (bitconvert v4i32:$C)))),
           (XXPERM v2f64:$XT, v2f64:$XB, v4i32:$C)>;
  def : Pat<(v2f64 (PPCxxperm v2f64:$XT, v2f64:$XB, v4i32:$C)),
           (XXPERM v2f64:$XT, v2f64:$XB, v4i32:$C)>;
```
do practically the same thing and I don't believe there is any use for the first one.

You can add the constraint and then remove the pattern that isn't used.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133700/new/

https://reviews.llvm.org/D133700