[PATCH] D90173: [PowerPC] Exploit splat instruction xxsplti32dx in Power10

Nemanja Ivanovic via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Dec 11 07:06:05 PST 2020


nemanjai requested changes to this revision.
nemanjai added a comment.
This revision now requires changes to proceed.

This is not functionally correct.



================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:9345
+      return DAG.getBitcast(Op.getValueType(), SplatNode);
+    } else { // we may lose precision, so we have to use XXSPLTI32DX.
+
----------------
Nit: full sentences in comments. Capitalize the first word.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:9349
+          (uint32_t)((APSplatBits.getZExtValue() & 0xFFFFFFFF00000000LL) >> 32);
+      uint32_t Lo =
+          (uint32_t)(APSplatBits.getZExtValue() & 0xFFFFFFFF00000000LL);
----------------
Won't this just produce a zero?


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:9351
+          (uint32_t)(APSplatBits.getZExtValue() & 0xFFFFFFFF00000000LL);
+      SDValue SplatNode;
+
----------------
Initialize to `undef` to simplify the logic below.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:9359
+        SplatNode =
+            DAG.getNode(PPCISD::XXSPLTI32DX, !Hi ? SDLoc(SplatNode) : dl,
+                        MVT::v2i64, !Hi ? SplatNode : DAG.getUNDEF(MVT::v2i64),
----------------
The debug location comes from the original node (i.e. `dl` in this case). Just use that - no ternary operator.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:9361
+                        MVT::v2i64, !Hi ? SplatNode : DAG.getUNDEF(MVT::v2i64),
+                        DAG.getTargetConstant(isLE ? 0 : 1, dl, MVT::i32),
+                        DAG.getTargetConstant(Lo, dl, MVT::i32));
----------------
Why does the index change depending on endianness? You are trying to produce a 64-bit pattern that is in no way dependent on endianness.


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

https://reviews.llvm.org/D90173



More information about the cfe-commits mailing list