[PATCH] D138736: [PowerPC] Fix vperm codegen

[Stefan Pintilie via Phabricator via llvm-commits]

stefanp added a comment.

Thank you for fixing this.
I do have a couple of comments related to the fix.



================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:10204
+                          ? V2->getOperand(0)->getOpcode() == PPCISD::XXSWAPD
+                          : false;
+
----------------
I don't think you need to bother with this extra ternary operator. 
Everywhere you use `V1HasXXSWAPD` or `V2HasXXSWAPD` seems to already be guarded by `Opcode == PPCISD::XXPERM` so it doesn't matter what value those two bool values have in cases when you are not using the `XXPERM` oeprand.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:10261
+    }
+    if (V2HasXXSWAPD) {
+      dl = SDLoc(V2->getOperand(0));
----------------
Is it possible to have a swap on both sides?


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:10271
+    }
   }
 
----------------
For this section how about:
```
if (Opcode == PPCISD::XXPERM && (V1HasXXSWAPD || V2HasXXSWAPD)) {
  /// Same code as above but you don't need to check that (V1HasXXSWAPD || V2HasXXSWAPD)
}
```
You don't need to go into the `if` statement at all if there isn't at least one XXSWAPD. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138736