[PATCH] D141473: [PowerPC] Simplify fp-to-int store optimization

[ChenZheng via Phabricator via llvm-commits]

shchenz added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:8075
+  MVT DestTy = Op.getSimpleValueType();
+  if ((DestTy == MVT::i8 || DestTy == MVT::i16) && Subtarget.hasP9Vector())
+    DestTy = MVT::i32;
----------------
This change is not enough I think. You extend this function to be used before type legalizer and make this function depend on the caller that will not pass other illegal types except `i8` and `i16`. How about simple types like `i2` and `i4`?

We need a guard for the type in this function.



================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:8076
+  if ((DestTy == MVT::i8 || DestTy == MVT::i16) && Subtarget.hasP9Vector())
+    DestTy = MVT::i32;
   unsigned Opc = ISD::DELETED_NODE;
----------------

And a case please for `i16` or `i8`


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:8089
   }
+  EVT ConvTy = Src.getValueType() == MVT::f128 ? MVT::f128 : MVT::f64;
+  SDValue Conv;
----------------
If we plan to support type `MVT::f128`, we may need to change the PPCISD node names to generic ones. These names are 1:1 map to PPC instructions and these instructions assume the types are f64.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:14878
-                    dl, ResVT == MVT::f128 ? MVT::f128 : MVT::f64, Val);
-  DCI.AddToWorklist(Val.getNode());
 
----------------
We may need to following this logic and consider about adding the users back to the work list.


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrP10.td:1257
+            (PSTXSDpc $src, $dst, 0)>;
+  def : Pat<(PPCstore_scal_int_from_vsr f64:$src, (PPCmatpcreladdr PCRelForm:$dst), 8),
+            (PSTXSDpc (COPY_TO_REGCLASS $src, VFRC), $dst, 0)>;
----------------
These two patterns are same with below two patterns. We still need to distinguish f64 and f128 input for `PPCstore_scal_int_from_vsr`.


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrP10.td:2208
+            (PSTXSD $src, PDForm:$dst)>;
+  def : Pat<(PPCstore_scal_int_from_vsr f64:$src, PDForm:$dst, 8),
+            (PSTXSD (COPY_TO_REGCLASS $src, VFRC), PDForm:$dst)>;
----------------
Same like above, there are duplicated patterns now as f64 and f128 are not distinguished


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141473