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

Zarko Todorovski via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 7 08:03:11 PDT 2021


ZarkoCA marked an inline comment as done.
ZarkoCA added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:1250
 
-    if (Subtarget.isISA3_1())
+    if (Subtarget.isISA3_1() && Subtarget.isPPC64())
       setOperationAction(ISD::INSERT_VECTOR_ELT, MVT::v2i64, Custom);
----------------
nemanjai wrote:
> I don't really understand how we are custom lowering this on 32-bit targets now since you've added this. Where are the PPC-specific insert nodes coming from?
We don't need this now, I don't think. Forgot to remove it in the previous diff. 


================
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
----------------
nemanjai wrote:
> ZarkoCA wrote:
> > 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. 
> This is fine.
Thanks. 


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrPrefix.td:2771
+            (VINSWLX $vDi, $rB, (LWZ memri:$rA))>;
+  def : Pat<(v4f32 (PPCvecinsertelt v4f32:$vDi, (f32 (load iaddrX34:$rA)), i32:$rB)),
+            (VINSWLX $vDi, $rB, (PLWZ memri34:$rA))>;
----------------
nemanjai wrote:
> Nit: line too long (here and elsewhere).
I noticed that but also there are several lines in `IsISA3_1, HasVSX, IsLittleEndian` and `IsISA3_1, HasVSX, IsBigEndian, IsPPC64` that were too long as well.  Thought it may be ok but I fixed it now for this case. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101383



More information about the llvm-commits mailing list