[PATCH] D133700: [PowerPC] Exploit xxperm, check for dead vectors and substitute vperm with xxperm

Amy Kwan via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 7 08:37:38 PDT 2022


amyk added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:10281
+  if (V2HasXSWAPD) {
+    dl = SDLoc(V1->getOperand(0));
+    V2 = V2->getOperand(0)->getOperand(1);
----------------
Do we mean to get `V2->getOperand(0)` here?


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:10301
+  SDValue VPERMNode =
+      DAG.getNode(Opcode, dl, V1.getValueType(), V1, V2, VPermMask);
+
----------------
Isn't `V1.getValueType()` here just `ValType`?


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:10217
+
+  bool V1HasXSWAP = V1->getOperand(0)->getOpcode() == PPCISD::XXSWAPD;
+  bool V2HasXSWAP = V2->getOperand(0)->getOpcode() == PPCISD::XXSWAPD;
----------------
amyk wrote:
> nit: XSWAP -> XXSWAPD maybe to match the actual opcode? 
nit: Update the names?


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:10235
+     0-3     4-7     8-11   12-15         0-3     4-7     8-11   12-15
+  IE, index of A, B += 8, and index of C, D -= 8.
+  XXSWAPD on V2:
----------------
amyk wrote:
> nit: `IE`->`i.e.`
> Can we also put a space after each `i.e.` to view the illustration better?
nit: Could we add a space after each `i.e.` to view the illustration better?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133700



More information about the llvm-commits mailing list