[PATCH] D149083: [PowerPC] Optimize VPERM and fix code order for swapping vector operands on LE
Stefan Pintilie via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri May 5 13:25:50 PDT 2023
stefanp added a comment.
Overall I think that this makes sense to me. I do have a couple of comments but mostly nits.
================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:10238
NeedSwap = !NeedSwap;
}
}
----------------
Now that the swap has been moved down should this also be moved down? Or at least part of it?
I ask because I've noticed that there are a number of tests where a new `vmr` has been introduced and I'm wondering if that's why.
================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:10310
+
+ if (V1HasXXSWAPD || V2HasXXSWAPD || Opcode == PPCISD::XXPERM) {
if (isPPC64 && ValType != MVT::v2f64)
----------------
nit:
Since this guard is only around a smaller section of code I think you can take the `isPPC64` part out and make it part of this guard.
Like:
```
if (isPPC64 && (V1HasXXSWAPD || V2HasXXSWAPD || Opcode == PPCISD::XXPERM))
```
That way if we are not PPC64 we won't need to check any of the rest.
================
Comment at: llvm/test/CodeGen/PowerPC/xxperm-swap.ll:20
; CHECK-LE-P9-NEXT: addis r3, r2, .LCPI0_0 at toc@ha
+; CHECK-LE-P9-NEXT: vmr v2, v3
; CHECK-LE-P9-NEXT: addi r3, r3, .LCPI0_0 at toc@l
----------------
This is the kind of extra copy that I mean.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D149083/new/
https://reviews.llvm.org/D149083
More information about the llvm-commits
mailing list