[PATCH] D101383: [PowerPC] Enable safe for 32bit vins* P10 instructions

[Zarko Todorovski via Phabricator via llvm-commits]

ZarkoCA marked 2 inline comments as done.
ZarkoCA added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:10344
     // integer vectors.
     if (VT == MVT::v16i8 || VT == MVT::v8i16 || VT == MVT::v4i32 ||
         VT == MVT::v2i64)
----------------
nemanjai wrote:
> ZarkoCA wrote:
> > nemanjai wrote:
> > > It would seem that all we need to change this condition and the one below to not emit `PPCISD::VECINSERT` for 64-bit element widths (`v2i64, v2f64`). Why do we need to disable this lowering on 32-bit targets altogether?
> > It looks like all of the pattern matches for VINS* in `PPCInstrPrefix.td` hardcode `i64`:
> > eg:
> > ```
> >   def : Pat<(v16i8 (PPCvecinsertelt v16i8:$vDi, i32:$rA, i64:$rB)),
> >             (VINSBLX $vDi, InsertEltShift.Sub32Left0, $rA)>;
> > ...
> >  foreach i = [0, 1] in
> >     def : Pat<(v2i64 (PPCvecinsertelt v2i64:$vDi, i64:$rA, (i64 i))),
> >               (VINSD $vDi, !mul(i, 8), $rA)>;
> > }
> > ```
> > So we can't emit the VECINSERT safely in 32bit mode due to this. 
> Sure, so those won't match. You might be able to change `i64` to `iPTR` (I'm not sure about that) or provide patterns with `i32` instead of `i64`.
Thanks for the suggestion and help.  It's much better to emit these when we can in 32bit mode.


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrPrefix.td:2758
 
-let Predicates = [IsISA3_1, HasVSX, IsBigEndian] in {
+let Predicates = [IsISA3_1, HasVSX, IsBigEndian, IsPPC32] in {
+  // Indexed vector insert element
----------------
I preferred to split the 32/64bit implementations mainly to keep 64bit as is.  I noticed that there were no other predicate definitions in this file and they can be moved if that's preferred. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101383