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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to