[llvm] 8378f1f - [InstCombine] Remove adjustMinMax() fold (PR62088)
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Tue May 30 07:06:47 PDT 2023
Author: Nikita Popov
Date: 2023-05-30T16:06:38+02:00
New Revision: 8378f1f4cdc8922e4f0409cabff25e0fef517bfa
URL: https://github.com/llvm/llvm-project/commit/8378f1f4cdc8922e4f0409cabff25e0fef517bfa
DIFF: https://github.com/llvm/llvm-project/commit/8378f1f4cdc8922e4f0409cabff25e0fef517bfa.diff
LOG: [InstCombine] Remove adjustMinMax() fold (PR62088)
This fold is buggy if the constant adjustment overflows.
Additionally, since we now canonicalize to min/max intrinsics,
the constants picked here don't actually matter, as long as SPF
still recognizes the pattern.
Fixes https://github.com/llvm/llvm-project/issues/62088.
Added:
Modified:
llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
llvm/test/Transforms/InstCombine/select.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
index 32b3c56dc9a2..7c93c2175aa9 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
@@ -1094,99 +1094,6 @@ static Value *foldSelectCttzCtlz(ICmpInst *ICI, Value *TrueVal, Value *FalseVal,
return nullptr;
}
-/// Return true if we find and adjust an icmp+select pattern where the compare
-/// is with a constant that can be incremented or decremented to match the
-/// minimum or maximum idiom.
-static bool adjustMinMax(SelectInst &Sel, ICmpInst &Cmp) {
- ICmpInst::Predicate Pred = Cmp.getPredicate();
- Value *CmpLHS = Cmp.getOperand(0);
- Value *CmpRHS = Cmp.getOperand(1);
- Value *TrueVal = Sel.getTrueValue();
- Value *FalseVal = Sel.getFalseValue();
-
- // We may move or edit the compare, so make sure the select is the only user.
- const APInt *CmpC;
- if (!Cmp.hasOneUse() || !match(CmpRHS, m_APInt(CmpC)))
- return false;
-
- // These transforms only work for selects of integers or vector selects of
- // integer vectors.
- Type *SelTy = Sel.getType();
- auto *SelEltTy = dyn_cast<IntegerType>(SelTy->getScalarType());
- if (!SelEltTy || SelTy->isVectorTy() != Cmp.getType()->isVectorTy())
- return false;
-
- Constant *AdjustedRHS;
- if (Pred == ICmpInst::ICMP_UGT || Pred == ICmpInst::ICMP_SGT)
- AdjustedRHS = ConstantInt::get(CmpRHS->getType(), *CmpC + 1);
- else if (Pred == ICmpInst::ICMP_ULT || Pred == ICmpInst::ICMP_SLT)
- AdjustedRHS = ConstantInt::get(CmpRHS->getType(), *CmpC - 1);
- else
- return false;
-
- // X > C ? X : C+1 --> X < C+1 ? C+1 : X
- // X < C ? X : C-1 --> X > C-1 ? C-1 : X
- if ((CmpLHS == TrueVal && AdjustedRHS == FalseVal) ||
- (CmpLHS == FalseVal && AdjustedRHS == TrueVal)) {
- ; // Nothing to do here. Values match without any sign/zero extension.
- }
- // Types do not match. Instead of calculating this with mixed types, promote
- // all to the larger type. This enables scalar evolution to analyze this
- // expression.
- else if (CmpRHS->getType()->getScalarSizeInBits() < SelEltTy->getBitWidth()) {
- Constant *SextRHS = ConstantExpr::getSExt(AdjustedRHS, SelTy);
-
- // X = sext x; x >s c ? X : C+1 --> X = sext x; X <s C+1 ? C+1 : X
- // X = sext x; x <s c ? X : C-1 --> X = sext x; X >s C-1 ? C-1 : X
- // X = sext x; x >u c ? X : C+1 --> X = sext x; X <u C+1 ? C+1 : X
- // X = sext x; x <u c ? X : C-1 --> X = sext x; X >u C-1 ? C-1 : X
- if (match(TrueVal, m_SExt(m_Specific(CmpLHS))) && SextRHS == FalseVal) {
- CmpLHS = TrueVal;
- AdjustedRHS = SextRHS;
- } else if (match(FalseVal, m_SExt(m_Specific(CmpLHS))) &&
- SextRHS == TrueVal) {
- CmpLHS = FalseVal;
- AdjustedRHS = SextRHS;
- } else if (Cmp.isUnsigned()) {
- Constant *ZextRHS = ConstantExpr::getZExt(AdjustedRHS, SelTy);
- // X = zext x; x >u c ? X : C+1 --> X = zext x; X <u C+1 ? C+1 : X
- // X = zext x; x <u c ? X : C-1 --> X = zext x; X >u C-1 ? C-1 : X
- // zext + signed compare cannot be changed:
- // 0xff <s 0x00, but 0x00ff >s 0x0000
- if (match(TrueVal, m_ZExt(m_Specific(CmpLHS))) && ZextRHS == FalseVal) {
- CmpLHS = TrueVal;
- AdjustedRHS = ZextRHS;
- } else if (match(FalseVal, m_ZExt(m_Specific(CmpLHS))) &&
- ZextRHS == TrueVal) {
- CmpLHS = FalseVal;
- AdjustedRHS = ZextRHS;
- } else {
- return false;
- }
- } else {
- return false;
- }
- } else {
- return false;
- }
-
- Pred = ICmpInst::getSwappedPredicate(Pred);
- CmpRHS = AdjustedRHS;
- std::swap(FalseVal, TrueVal);
- Cmp.setPredicate(Pred);
- Cmp.setOperand(0, CmpLHS);
- Cmp.setOperand(1, CmpRHS);
- Sel.setOperand(1, TrueVal);
- Sel.setOperand(2, FalseVal);
- Sel.swapProfMetadata();
-
- // Move the compare instruction right before the select instruction. Otherwise
- // the sext/zext value may be defined after the compare instruction uses it.
- Cmp.moveBefore(&Sel);
-
- return true;
-}
-
static Instruction *canonicalizeSPF(SelectInst &Sel, ICmpInst &Cmp,
InstCombinerImpl &IC) {
Value *LHS, *RHS;
@@ -1718,12 +1625,11 @@ Instruction *InstCombinerImpl::foldSelectInstWithICmp(SelectInst &SI,
tryToReuseConstantFromSelectInComparison(SI, *ICI, *this))
return NewSel;
- bool Changed = adjustMinMax(SI, *ICI);
-
if (Value *V = foldSelectICmpAnd(SI, ICI, Builder))
return replaceInstUsesWith(SI, V);
// NOTE: if we wanted to, this is where to detect integer MIN/MAX
+ bool Changed = false;
Value *TrueVal = SI.getTrueValue();
Value *FalseVal = SI.getFalseValue();
ICmpInst::Predicate Pred = ICI->getPredicate();
diff --git a/llvm/test/Transforms/InstCombine/select.ll b/llvm/test/Transforms/InstCombine/select.ll
index ccb62b027c65..39aeaa577fa5 100644
--- a/llvm/test/Transforms/InstCombine/select.ll
+++ b/llvm/test/Transforms/InstCombine/select.ll
@@ -3581,3 +3581,45 @@ define i32 @pr61361(i32 %arg) {
%ashr = ashr i32 %sel2, 1
ret i32 %ashr
}
+
+define i32 @pr62088() {
+; CHECK-LABEL: @pr62088(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: br label [[LOOP:%.*]]
+; CHECK: loop:
+; CHECK-NEXT: [[NOT2:%.*]] = phi i32 [ 0, [[ENTRY:%.*]] ], [ -2, [[LOOP]] ]
+; CHECK-NEXT: [[H_0:%.*]] = phi i32 [ 0, [[ENTRY]] ], [ 1, [[LOOP]] ]
+; CHECK-NEXT: [[XOR1:%.*]] = or i32 [[H_0]], [[NOT2]]
+; CHECK-NEXT: [[SUB5:%.*]] = sub i32 -1824888657, [[XOR1]]
+; CHECK-NEXT: [[XOR6:%.*]] = xor i32 [[SUB5]], -1260914025
+; CHECK-NEXT: [[CMP:%.*]] = icmp slt i32 [[XOR6]], 824855120
+; CHECK-NEXT: br i1 [[CMP]], label [[LOOP]], label [[EXIT:%.*]]
+; CHECK: exit:
+; CHECK-NEXT: ret i32 [[H_0]]
+;
+entry:
+ br label %loop
+
+loop:
+ %not2 = phi i32 [ 0, %entry ], [ -2, %loop ]
+ %i.0 = phi i32 [ 0, %entry ], [ %shr, %loop ]
+ %h.0 = phi i32 [ 0, %entry ], [ 1, %loop ]
+ %i.0.fr = freeze i32 %i.0
+ %sext = shl i32 %i.0.fr, 16
+ %conv = ashr exact i32 %sext, 16
+ %not = xor i32 %conv, -1
+ %and = and i32 %h.0, 1
+ %rem.urem = sub nsw i32 %and, %conv
+ %rem.cmp = icmp ult i32 %and, %conv
+ %rem = select i1 %rem.cmp, i32 %not, i32 %rem.urem
+ %xor = xor i32 %rem, %not2
+ %sub = sub nsw i32 0, %xor
+ %sub5 = sub i32 -1824888657, %xor
+ %xor6 = xor i32 %sub5, -1260914025
+ %cmp = icmp slt i32 %xor6, 824855120
+ %shr = ashr i32 %xor6, 40
+ br i1 %cmp, label %loop, label %exit
+
+exit:
+ ret i32 %rem
+}
More information about the llvm-commits
mailing list