[PATCH] D45553: [Power9]Legalize and emit code for truncate and convert Quad-Precision to Double-Word

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai requested changes to this revision.
nemanjai added inline comments.
This revision now requires changes to proceed.


================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:6928
+
+  // It is legal to truncate and convert f128 to integer types.
+  if (EnableQuadPrecision && (Op->getOperand(0).getValueType() == MVT::f128))
----------------
This sentence makes it sound like this is legal in a judicial sense. Perhaps:
`// FP to INT conversions are legal for f128.`


================
Comment at: lib/Target/PowerPC/PPCInstrVSX.td:3154
+  // Truncate & Convert QP -> (Un)Signed DWord in memory
+  def : Pat<(store (i64 (fp_to_sint f128:$src)), xoaddr:$dst),
+            (STXSD (COPY_TO_REGCLASS (XSCVQPSDZ $src), VFRC), xoaddr:$dst)>;
----------------
This seems very wrong. The `STXSD` will need a reg and an immediate and `xoaddr` will force it to use reg+reg. We should have two versions here (`xaddr -> STXSDX` and `ixaddr -> STXSD`). And we should also have test cases for both store instructions.


================
Comment at: lib/Target/PowerPC/PPCInstrVSX.td:3157
 
+  // Truncate & Convert QP -> (Un)Signed DWord in memory
+  def : Pat<(store (i64 (fp_to_sint f128:$src)), xoaddr:$dst),
----------------
Don't forget periods on comments.


================
Comment at: test/CodeGen/PowerPC/f128-truncateNconv.ll:2
+; RUN: llc -mcpu=pwr9 -mtriple=powerpc64le-unknown-unknown \
+; RUN:   -enable-ppc-quad-precision -ppc-vsr-nums-as-vr < %s | FileCheck %s
+
----------------
Don't forget `-verify-machineinstrs` in test cases. We want to make sure we don't introduce new machine verifier failures (which I think this patch does).


https://reviews.llvm.org/D45553