[llvm] 7e18cd8 - [InstCombine] Whitelist non-refining folds in SimplifyWithOpReplaced
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Mon Mar 22 14:13:08 PDT 2021
Author: Nikita Popov
Date: 2021-03-22T22:12:56+01:00
New Revision: 7e18cd887cd402e3d5465c57c218079e4df65231
URL: https://github.com/llvm/llvm-project/commit/7e18cd887cd402e3d5465c57c218079e4df65231
DIFF: https://github.com/llvm/llvm-project/commit/7e18cd887cd402e3d5465c57c218079e4df65231.diff
LOG: [InstCombine] Whitelist non-refining folds in SimplifyWithOpReplaced
This is an alternative to D98391/D98585, playing things more
conservatively. If AllowRefinement == false, then we don't use
InstSimplify methods at all, and instead explicitly implement a
small number of non-refining folds. Most cases are handled by
constant folding, and I only had to add three folds to cover
our unit tests / test-suite. While this may lose some optimization
power, I think it is safer to approach from this direction, given
how many issues this code has already caused.
Differential Revision: https://reviews.llvm.org/D99027
Added:
Modified:
llvm/include/llvm/Analysis/InstructionSimplify.h
llvm/lib/Analysis/InstructionSimplify.cpp
llvm/test/Transforms/InstCombine/minmax-fold.ll
llvm/test/Transforms/InstCombine/select.ll
llvm/test/Transforms/InstSimplify/pr49495.ll
Removed:
################################################################################
diff --git a/llvm/include/llvm/Analysis/InstructionSimplify.h b/llvm/include/llvm/Analysis/InstructionSimplify.h
index 17d6f30a35cb..dda90e826bba 100644
--- a/llvm/include/llvm/Analysis/InstructionSimplify.h
+++ b/llvm/include/llvm/Analysis/InstructionSimplify.h
@@ -294,8 +294,8 @@ Value *SimplifyInstruction(Instruction *I, const SimplifyQuery &Q,
/// See if V simplifies when its operand Op is replaced with RepOp. If not,
/// return null.
-/// AllowRefinement specifies whether the simplification can be a refinement,
-/// or whether it needs to be strictly identical.
+/// AllowRefinement specifies whether the simplification can be a refinement
+/// (e.g. 0 instead of poison), or whether it needs to be strictly identical.
Value *SimplifyWithOpReplaced(Value *V, Value *Op, Value *RepOp,
const SimplifyQuery &Q, bool AllowRefinement);
diff --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp
index 4d7e281312ba..1dc7499c85c8 100644
--- a/llvm/lib/Analysis/InstructionSimplify.cpp
+++ b/llvm/lib/Analysis/InstructionSimplify.cpp
@@ -3936,18 +3936,33 @@ static Value *SimplifyWithOpReplaced(Value *V, Value *Op, Value *RepOp,
transform(I->operands(), NewOps.begin(),
[&](Value *V) { return V == Op ? RepOp : V; });
- // Consider:
- // %cmp = icmp eq i32 %x, 2147483647
- // %add = add nsw i32 %x, 1
- // %sel = select i1 %cmp, i32 -2147483648, i32 %add
- //
- // We can't replace %sel with %add unless we strip away the flags (which will
- // be done in InstCombine).
- // TODO: This is unsound, because it only catches some forms of refinement.
- if (!AllowRefinement && canCreatePoison(cast<Operator>(I)))
- return nullptr;
+ if (!AllowRefinement) {
+ // General InstSimplify functions may refine the result, e.g. by returning
+ // a constant for a potentially poison value. To avoid this, implement only
+ // a few non-refining but profitable transforms here.
+
+ if (auto *BO = dyn_cast<BinaryOperator>(I)) {
+ unsigned Opcode = BO->getOpcode();
+ // id op x -> x, x op id -> x
+ if (NewOps[0] == ConstantExpr::getBinOpIdentity(Opcode, I->getType()))
+ return NewOps[1];
+ if (NewOps[1] == ConstantExpr::getBinOpIdentity(Opcode, I->getType(),
+ /* RHS */ true))
+ return NewOps[0];
+
+ // x & x -> x, x | x -> x
+ if ((Opcode == Instruction::And || Opcode == Instruction::Or) &&
+ NewOps[0] == NewOps[1])
+ return NewOps[0];
+ }
- if (MaxRecurse) {
+ if (auto *GEP = dyn_cast<GetElementPtrInst>(I)) {
+ // getelementptr x, 0 -> x
+ if (NewOps.size() == 2 && match(NewOps[1], m_Zero()) &&
+ !GEP->isInBounds())
+ return NewOps[0];
+ }
+ } else if (MaxRecurse) {
// The simplification queries below may return the original value. Consider:
// %div = udiv i32 %arg, %arg2
// %mul = mul nsw i32 %div, %arg2
@@ -3986,6 +4001,18 @@ static Value *SimplifyWithOpReplaced(Value *V, Value *Op, Value *RepOp,
return nullptr;
}
+ // Consider:
+ // %cmp = icmp eq i32 %x, 2147483647
+ // %add = add nsw i32 %x, 1
+ // %sel = select i1 %cmp, i32 -2147483648, i32 %add
+ //
+ // We can't replace %sel with %add unless we strip away the flags (which
+ // will be done in InstCombine).
+ // TODO: This may be unsound, because it only catches some forms of
+ // refinement.
+ if (!AllowRefinement && canCreatePoison(cast<Operator>(I)))
+ return nullptr;
+
if (CmpInst *C = dyn_cast<CmpInst>(I))
return ConstantFoldCompareInstOperands(C->getPredicate(), ConstOps[0],
ConstOps[1], Q.DL, Q.TLI);
diff --git a/llvm/test/Transforms/InstCombine/minmax-fold.ll b/llvm/test/Transforms/InstCombine/minmax-fold.ll
index 78126302f9b9..7aa3a3992449 100644
--- a/llvm/test/Transforms/InstCombine/minmax-fold.ll
+++ b/llvm/test/Transforms/InstCombine/minmax-fold.ll
@@ -1080,7 +1080,7 @@ define i37 @add_umax_constant_limit(i37 %x) {
define i37 @add_umax_simplify(i37 %x) {
; CHECK-LABEL: @add_umax_simplify(
-; CHECK-NEXT: [[A:%.*]] = add i37 [[X:%.*]], 42
+; CHECK-NEXT: [[A:%.*]] = add nuw i37 [[X:%.*]], 42
; CHECK-NEXT: ret i37 [[A]]
;
%a = add nuw i37 %x, 42
diff --git a/llvm/test/Transforms/InstCombine/select.ll b/llvm/test/Transforms/InstCombine/select.ll
index ad1d32999556..f98a369c144b 100644
--- a/llvm/test/Transforms/InstCombine/select.ll
+++ b/llvm/test/Transforms/InstCombine/select.ll
@@ -902,7 +902,9 @@ define i32 @test56(i16 %x) {
define i32 @test57(i32 %x, i32 %y) {
; CHECK-LABEL: @test57(
; CHECK-NEXT: [[AND:%.*]] = and i32 [[X:%.*]], [[Y:%.*]]
-; CHECK-NEXT: ret i32 [[AND]]
+; CHECK-NEXT: [[TOBOOL:%.*]] = icmp eq i32 [[X]], 0
+; CHECK-NEXT: [[DOTAND:%.*]] = select i1 [[TOBOOL]], i32 0, i32 [[AND]]
+; CHECK-NEXT: ret i32 [[DOTAND]]
;
%and = and i32 %x, %y
%tobool = icmp eq i32 %x, 0
diff --git a/llvm/test/Transforms/InstSimplify/pr49495.ll b/llvm/test/Transforms/InstSimplify/pr49495.ll
index f085de3b4a22..b3eca968c060 100644
--- a/llvm/test/Transforms/InstSimplify/pr49495.ll
+++ b/llvm/test/Transforms/InstSimplify/pr49495.ll
@@ -4,9 +4,11 @@
; The first comparison (a != b) should not be dropped
define i1 @test1(i8* %a, i8* %b) {
; CHECK-LABEL: @test1(
-; CHECK-NEXT: [[A2:%.*]] = getelementptr inbounds i8, i8* [[A:%.*]], i64 -1
-; CHECK-NEXT: [[COND2:%.*]] = icmp ugt i8* [[A2]], [[B:%.*]]
-; CHECK-NEXT: ret i1 [[COND2]]
+; CHECK-NEXT: [[COND1:%.*]] = icmp ne i8* [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT: [[A2:%.*]] = getelementptr inbounds i8, i8* [[A]], i64 -1
+; CHECK-NEXT: [[COND2:%.*]] = icmp ugt i8* [[A2]], [[B]]
+; CHECK-NEXT: [[RES:%.*]] = select i1 [[COND1]], i1 [[COND2]], i1 false
+; CHECK-NEXT: ret i1 [[RES]]
;
%cond1 = icmp ne i8* %a, %b
%a2 = getelementptr inbounds i8, i8* %a, i64 -1
@@ -18,9 +20,11 @@ define i1 @test1(i8* %a, i8* %b) {
; The first comparison (a != b) should not be dropped
define i1 @test2(i32 %a, i32 %b) {
; CHECK-LABEL: @test2(
-; CHECK-NEXT: [[A2:%.*]] = add nuw i32 [[A:%.*]], 1
-; CHECK-NEXT: [[COND2:%.*]] = icmp ult i32 [[A2]], [[B:%.*]]
-; CHECK-NEXT: ret i1 [[COND2]]
+; CHECK-NEXT: [[COND1:%.*]] = icmp ne i32 [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT: [[A2:%.*]] = add nuw i32 [[A]], 1
+; CHECK-NEXT: [[COND2:%.*]] = icmp ult i32 [[A2]], [[B]]
+; CHECK-NEXT: [[RES:%.*]] = select i1 [[COND1]], i1 [[COND2]], i1 false
+; CHECK-NEXT: ret i1 [[RES]]
;
%cond1 = icmp ne i32 %a, %b
%a2 = add nuw i32 %a, 1
More information about the llvm-commits
mailing list