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

[Victor Huang via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:9189
                            HasAnyUndefs, 0, !Subtarget.isLittleEndian());
+  bool LE = Subtarget.isLittleEndian();
 
----------------
please follow the naming convention in this file to use ` IsLE`


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:9204
+
+      uint32_t top = (uint32_t) ((APSplatBits.getZExtValue() & 0xFFFFFFFF00000000LL) >> 32);
+      uint32_t bot = (uint32_t) (APSplatBits.getZExtValue() & 0xFFFFFFFF00000000LL);
----------------
Agree with Amy it is better to use `Hi` and Lo` as the variable names. 
Also it is still taking more than 80 characters. Please double check if you properly `clang-format` it.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:9205
+      uint32_t top = (uint32_t) ((APSplatBits.getZExtValue() & 0xFFFFFFFF00000000LL) >> 32);
+      uint32_t bot = (uint32_t) (APSplatBits.getZExtValue() & 0xFFFFFFFF00000000LL);
+      SDValue SplatNode;
----------------
85 characters in this line, please double check you properly do `clang-format` 


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:9213
+
+      if (bot) {
+        SplatNode = DAG.getNode(
----------------
I do not quite understand the `if` and `else` logic here, current logic seems we can overwrite `SplatNode` after we execute `SplatNode = DAG.getTargetConstant(0, dl, MVT::v2i64);` 
Is that as expected? 


================
Comment at: llvm/test/CodeGen/PowerPC/p10-splatImm-CPload-pcrel.ll:17
 ; CHECK:       # %bb.0: # %entry
-; CHECK-NEXT:    plxv vs34, .LCPI0_0 at PCREL(0), 1
+; CHECK-NEXT:    xxlxor vs34, vs34, vs34
+; CHECK-LE-NEXT:    xxsplti32dx vs34, 1, 1081435463
----------------
alignment issue 


================
Comment at: llvm/test/CodeGen/PowerPC/p10-splatImm-CPload-pcrel.ll:19
+; CHECK-LE-NEXT:    xxsplti32dx vs34, 1, 1081435463
+; CHECK-BE-NEXT: xxsplti32dx vs34, 0, 1081435463
 ; CHECK-NEXT:    blr
----------------
alignment issue 


================
Comment at: llvm/test/CodeGen/PowerPC/p10-splatImm-CPload-pcrel.ll:43
 ; CHECK-NOPCREL:       # %bb.0: # %entry
-; CHECK-NOPCREL-NEXT:    addis r3, r2, .LCPI1_0 at toc@ha
-; CHECK-NOPCREL-NEXT:    addi r3, r3, .LCPI1_0 at toc@l
-; CHECK-NOPCREL-NEXT:    lxvx vs34, 0, r3
+; CHECK-NOPCREL-NEXT:    xxlxor vs34, vs34, vs34
+; CHECK-NOPCREL-LE-NEXT:    xxsplti32dx vs34, 1, 940259579
----------------
alignment issue 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90173