if C1 and C3 are pow2 and Log2(C3)+C2 < BitWidth:
((C1 << X) >> C2) & C3 -> X == (Log2(C3)+C2-Log2(C1)) ? C3 : 0;
Differential D126617
[InstCombine] Optimize shl+lshr+and conversion pattern bcl5980 on May 29 2022, 9:46 AM. Authored by
Details if C1 and C3 are pow2 and Log2(C3)+C2 < BitWidth: ((C1 << X) >> C2) & C3 -> X == (Log2(C3)+C2-Log2(C1)) ? C3 : 0;
Diff Detail
Unit Tests Event Timeline
Comment Actions
Comment Actions I still think we should split this patch up as 2 independent transforms. The opposite shifts transform doesn't seem like it should be a power-of-2-mask transform. Can we handle that using demanded bits instead? Double-check (you can pre-commit more tests as needed), but I don't think this patch will handle these related folds: Comment Actions Thanks for the mention. Is this transform you want ? Comment Actions Yes, the first pre-condition looks correct. We don't actually care what the final instruction in the sequence is - it just has to remove demand of the high bits. The last instruction could be a trunc for example, so we should have tests with that too: We already look for that pattern in InstCombine's demanded bits. So I think we just need to add a transform like this: diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp index 278db05f65d1..c0d92fc27bb6 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp @@ -630,6 +630,18 @@ Value *InstCombinerImpl::SimplifyDemandedUseBits(Value *V, APInt DemandedMask, ComputeNumSignBits(I->getOperand(0), Depth + 1, CxtI); if (SignBits >= NumHiDemandedBits) return I->getOperand(0); + + // If we can pre-shift a left-shifted constant to the right without + // losing any low bits (we already know we don't demand the high bits): + // (C << X) >> SA --> (C >> SA) << X + Value *X; + const APInt *C; + if (match(I->getOperand(0), m_Shl(m_APInt(C), m_Value(X))) && + C->countTrailingZeros() >= ShiftAmt) { + Constant *ShiftC = ConstantInt::get(I->getType(), C->lshr(ShiftAmt)); + Instruction *Shl = BinaryOperator::CreateShl(ShiftC, X); + return InsertNewInstWith(Shl, *I); + } } // Unsigned shift right.
Comment Actions Sorry for the late response. Based on the discussion I have some questions:
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp index 278db05f65d1..c0d92fc27bb6 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp @@ -630,6 +630,18 @@ Value *InstCombinerImpl::SimplifyDemandedUseBits(Value *V, APInt DemandedMask, ComputeNumSignBits(I->getOperand(0), Depth + 1, CxtI); if (SignBits >= NumHiDemandedBits) return I->getOperand(0); + + // If we can pre-shift a left-shifted constant to the right without + // losing any low bits (we already know we don't demand the high bits): + // (C << X) >> SA --> (C >> SA) << X + Value *X; + const APInt *C; + if (match(I->getOperand(0), m_Shl(m_APInt(C), m_Value(X))) && + C->countTrailingZeros() >= ShiftAmt) { + Constant *ShiftC = ConstantInt::get(I->getType(), C->lshr(ShiftAmt)); + Instruction *Shl = BinaryOperator::CreateShl(ShiftC, X); + return InsertNewInstWith(Shl, *I); + } }
iff (C1 is pow2) & ((C2 & ~(C1-1)) + C1) is pow2) & (C1 < C2): ((C1 << X) & C2) == 0 -> X >= (Log2(C2+C1) - Log2(C1)); https://alive2.llvm.org/ce/z/JQYFnn ((C1 << X) & C2) != 0 -> X < (Log2(C2+C1) - Log2(C1)); https://alive2.llvm.org/ce/z/BnyEmk
Comment Actions Ok, let's try to make improvements in small steps; one part for demanded bits is here:
Comment Actions
Comment Actions Update the Alive2 proof in the patch description, so it matches the new code.
Comment Actions LGTM If I'm seeing it correctly, this will alter D126591 or possibly make it unnecessary. I recommend implementing the symmetric TODO pattern for this patch as the next patch, and then we can see what remains. |
What happens if we reduce the pattern to:
https://alive2.llvm.org/ce/z/7snGRd
That's the same transform that I suggested in D126591, but invert the shift direction (lshr instead of shl).