[PATCH] D120192: [DAG] Attempt to fold bswap(shl(x,c)) -> zext(bswap(trunc(shl(x,c-bw/2))))
Sanjay Patel via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 22 10:04:28 PST 2022
spatel added inline comments.
================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:9594
+ unsigned BW = VT.getScalarSizeInBits();
+ if (BW >= 32 && N0.getOpcode() == ISD::SHL && N->isOnlyUserOf(N0.getNode())) {
+ auto *ShAmt = dyn_cast<ConstantSDNode>(N0.getOperand(1));
----------------
Is this intentionally different than the usual "N0.hasOneUse()"?
================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:9599
+ TLI.isTruncateFree(VT, HalfVT) &&
+ (!LegalOperations || hasOperation(ISD::BSWAP, HalfVT))) {
+ SDValue Res = N0.getOperand(0);
----------------
If I'm seeing it correctly, we need to either tighten this clause by removing !LegalOperations or add the "BW % 16" requirement.
As-is (and we probably want a test either way), we could end up worse. For example on AArch64:
```
define i32 @test_bswap32_shift17(i32 %a0) {
%s = shl i32 %a0, 17
%b = call i32 @llvm.bswap.i32(i32 %s)
ret i32 %b
}
```
is:
```
lsl w8, w0, #17
rev w0, w8
```
but this patch would make it:
```
lsl w8, w0, #1
rev w8, w8
lsr w0, w8, #16
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120192/new/
https://reviews.llvm.org/D120192
More information about the llvm-commits
mailing list