[llvm] 2b52e4e - [InstCombine] Remove known bits constant folding
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 20 12:54:17 PDT 2020
Author: Nikita Popov
Date: 2020-03-20T20:54:06+01:00
New Revision: 2b52e4e629e6793f832caef5e47f9d84607740f3
URL: https://github.com/llvm/llvm-project/commit/2b52e4e629e6793f832caef5e47f9d84607740f3
DIFF: https://github.com/llvm/llvm-project/commit/2b52e4e629e6793f832caef5e47f9d84607740f3.diff
LOG: [InstCombine] Remove known bits constant folding
If ExpensiveCombines is enabled (which is the case with -O3 on the
legacy PM and always on the new PM), InstCombine tries to compute
the known bits of all instructions in the hope that all bits end up
being known, which is fairly expensive.
How effective is it? If we add some statistics on how often the
constant folding succeeds and how many KnownBits calculations are
performed and run test-suite we get:
"instcombine.NumConstPropKnownBits": 642,
"instcombine.NumConstPropKnownBitsComputed": 18744965,
In other words, we get one fold for every 30000 KnownBits calculations.
However, the truth is actually much worse: Currently, known bits are
computed before performing other folds, so there is a high chance
that cases that get folded by known bits would also have been
handled by other folds.
What happens if we compute known bits after all other folds
(hacky implementation: https://gist.github.com/nikic/751f25b3b9d9e0860db5dde934f70f46)?
"instcombine.NumConstPropKnownBits": 0,
"instcombine.NumConstPropKnownBitsComputed": 18105547,
So it turns out despite doing 18 million known bits calculations,
the known bits fold does not do anything useful on test-suite.
I was originally planning to move this into AggressiveInstCombine
so it only runs once in the pipeline, but seeing this, I think
we're better off removing it entirely.
As this is the only use of the "expensive combines" mechanism,
it may be removed afterwards, but I'll leave that to a separate patch.
Differential Revision: https://reviews.llvm.org/D75801
Added:
Modified:
llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
llvm/test/Transforms/InstCombine/assume.ll
llvm/test/Transforms/InstCombine/known-signbit-shift.ll
llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll
llvm/test/Transforms/InstCombine/phi-shifts.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 9bcd6d79756a..f2bd04536dbf 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -130,6 +130,7 @@ static cl::opt<bool>
EnableCodeSinking("instcombine-code-sinking", cl::desc("Enable code sinking"),
cl::init(true));
+// FIXME: This option is no longer used for anything and may be removed.
static cl::opt<bool>
EnableExpensiveCombines("expensive-combines",
cl::desc("Enable expensive instruction combines"));
@@ -3491,27 +3492,6 @@ bool InstCombiner::run() {
}
}
- // In general, it is possible for computeKnownBits to determine all bits in
- // a value even when the operands are not all constants.
- Type *Ty = I->getType();
- if (ExpensiveCombines && !I->use_empty() && Ty->isIntOrIntVectorTy() &&
- !isMustTailCall(I)) {
- KnownBits Known = computeKnownBits(I, /*Depth*/0, I);
- if (Known.isConstant()) {
- Constant *C = ConstantInt::get(Ty, Known.getConstant());
- LLVM_DEBUG(dbgs() << "IC: ConstFold (all bits known) to: " << *C
- << " from: " << *I << '\n');
-
- // Add operands to the worklist.
- replaceInstUsesWith(*I, C);
- ++NumConstProp;
- if (isInstructionTriviallyDead(I, &TLI))
- eraseInstFromFunction(*I);
- MadeIRChange = true;
- continue;
- }
- }
-
// See if we can trivially sink this instruction to a successor basic block.
if (EnableCodeSinking && I->hasOneUse()) {
BasicBlock *BB = I->getParent();
diff --git a/llvm/test/Transforms/InstCombine/assume.ll b/llvm/test/Transforms/InstCombine/assume.ll
index ff3bf5665ed9..a1b1211ac60d 100644
--- a/llvm/test/Transforms/InstCombine/assume.ll
+++ b/llvm/test/Transforms/InstCombine/assume.ll
@@ -353,16 +353,12 @@ define i1 @nonnull5(i32** %a) {
; PR35846 - https://bugs.llvm.org/show_bug.cgi?id=35846
define i32 @assumption_conflicts_with_known_bits(i32 %a, i32 %b) {
-; EXPENSIVE-ON-LABEL: @assumption_conflicts_with_known_bits(
-; EXPENSIVE-ON-NEXT: tail call void @llvm.assume(i1 false)
-; EXPENSIVE-ON-NEXT: ret i32 0
-;
-; EXPENSIVE-OFF-LABEL: @assumption_conflicts_with_known_bits(
-; EXPENSIVE-OFF-NEXT: [[AND1:%.*]] = and i32 [[B:%.*]], 3
-; EXPENSIVE-OFF-NEXT: tail call void @llvm.assume(i1 false)
-; EXPENSIVE-OFF-NEXT: [[CMP2:%.*]] = icmp eq i32 [[AND1]], 0
-; EXPENSIVE-OFF-NEXT: tail call void @llvm.assume(i1 [[CMP2]])
-; EXPENSIVE-OFF-NEXT: ret i32 0
+; CHECK-LABEL: @assumption_conflicts_with_known_bits(
+; CHECK-NEXT: [[AND1:%.*]] = and i32 [[B:%.*]], 3
+; CHECK-NEXT: tail call void @llvm.assume(i1 false)
+; CHECK-NEXT: [[CMP2:%.*]] = icmp eq i32 [[AND1]], 0
+; CHECK-NEXT: tail call void @llvm.assume(i1 [[CMP2]])
+; CHECK-NEXT: ret i32 0
;
%and1 = and i32 %b, 3
%B1 = lshr i32 %and1, %and1
diff --git a/llvm/test/Transforms/InstCombine/known-signbit-shift.ll b/llvm/test/Transforms/InstCombine/known-signbit-shift.ll
index 0a4e7660ef1f..4d8e387ad7b8 100644
--- a/llvm/test/Transforms/InstCombine/known-signbit-shift.ll
+++ b/llvm/test/Transforms/InstCombine/known-signbit-shift.ll
@@ -30,11 +30,8 @@ define i1 @test_shift_negative(i32 %a, i32 %b) {
; If sign bit is a known zero, it cannot be a known one.
; This test should not crash opt. The shift produces poison.
define i32 @test_no_sign_bit_conflict1(i1 %b) {
-; EXPENSIVE-OFF-LABEL: @test_no_sign_bit_conflict1(
-; EXPENSIVE-OFF-NEXT: ret i32 undef
-;
-; EXPENSIVE-ON-LABEL: @test_no_sign_bit_conflict1(
-; EXPENSIVE-ON-NEXT: ret i32 0
+; CHECK-LABEL: @test_no_sign_bit_conflict1(
+; CHECK-NEXT: ret i32 undef
;
%sel = select i1 %b, i32 8193, i32 8192
%mul = shl nsw i32 %sel, 18
@@ -44,11 +41,8 @@ define i32 @test_no_sign_bit_conflict1(i1 %b) {
; If sign bit is a known one, it cannot be a known zero.
; This test should not crash opt. The shift produces poison.
define i32 @test_no_sign_bit_conflict2(i1 %b) {
-; EXPENSIVE-OFF-LABEL: @test_no_sign_bit_conflict2(
-; EXPENSIVE-OFF-NEXT: ret i32 undef
-;
-; EXPENSIVE-ON-LABEL: @test_no_sign_bit_conflict2(
-; EXPENSIVE-ON-NEXT: ret i32 0
+; CHECK-LABEL: @test_no_sign_bit_conflict2(
+; CHECK-NEXT: ret i32 undef
;
%sel = select i1 %b, i32 -8193, i32 -8194
%mul = shl nsw i32 %sel, 18
diff --git a/llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll b/llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll
index 5c1867f81006..6eb9b377e384 100644
--- a/llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll
+++ b/llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll
@@ -4,17 +4,11 @@
; Check that we don't crash on unreasonable constant indexes
define i32 @test_out_of_bounds(i32 %a, i1 %x, i1 %y) {
-; EXPENSIVE-OFF-LABEL: @test_out_of_bounds(
-; EXPENSIVE-OFF-NEXT: entry:
-; EXPENSIVE-OFF-NEXT: [[AND1:%.*]] = and i32 [[A:%.*]], 3
-; EXPENSIVE-OFF-NEXT: tail call void @llvm.assume(i1 undef)
-; EXPENSIVE-OFF-NEXT: ret i32 [[AND1]]
-;
-; EXPENSIVE-ON-LABEL: @test_out_of_bounds(
-; EXPENSIVE-ON-NEXT: entry:
-; EXPENSIVE-ON-NEXT: [[AND1:%.*]] = and i32 [[A:%.*]], 3
-; EXPENSIVE-ON-NEXT: tail call void @llvm.assume(i1 false)
-; EXPENSIVE-ON-NEXT: ret i32 [[AND1]]
+; CHECK-LABEL: @test_out_of_bounds(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[AND1:%.*]] = and i32 [[A:%.*]], 3
+; CHECK-NEXT: tail call void @llvm.assume(i1 undef)
+; CHECK-NEXT: ret i32 [[AND1]]
;
entry:
%and1 = and i32 %a, 3
@@ -25,15 +19,10 @@ entry:
}
define i128 @test_non64bit(i128 %a) {
-; EXPENSIVE-OFF-LABEL: @test_non64bit(
-; EXPENSIVE-OFF-NEXT: [[AND1:%.*]] = and i128 [[A:%.*]], 3
-; EXPENSIVE-OFF-NEXT: tail call void @llvm.assume(i1 undef)
-; EXPENSIVE-OFF-NEXT: ret i128 [[AND1]]
-;
-; EXPENSIVE-ON-LABEL: @test_non64bit(
-; EXPENSIVE-ON-NEXT: [[AND1:%.*]] = and i128 [[A:%.*]], 3
-; EXPENSIVE-ON-NEXT: tail call void @llvm.assume(i1 false)
-; EXPENSIVE-ON-NEXT: ret i128 [[AND1]]
+; CHECK-LABEL: @test_non64bit(
+; CHECK-NEXT: [[AND1:%.*]] = and i128 [[A:%.*]], 3
+; CHECK-NEXT: tail call void @llvm.assume(i1 undef)
+; CHECK-NEXT: ret i128 [[AND1]]
;
%and1 = and i128 %a, 3
%B = lshr i128 %and1, -1
diff --git a/llvm/test/Transforms/InstCombine/phi-shifts.ll b/llvm/test/Transforms/InstCombine/phi-shifts.ll
index af94d8b4001b..f1ad6dcafb62 100644
--- a/llvm/test/Transforms/InstCombine/phi-shifts.ll
+++ b/llvm/test/Transforms/InstCombine/phi-shifts.ll
@@ -4,21 +4,13 @@
; OSS Fuzz: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=15217
define i64 @fuzz15217(i1 %cond, i8* %Ptr, i64 %Val) {
-; EXPENSIVE-OFF-LABEL: @fuzz15217(
-; EXPENSIVE-OFF-NEXT: entry:
-; EXPENSIVE-OFF-NEXT: br i1 [[COND:%.*]], label [[END:%.*]], label [[TWO:%.*]]
-; EXPENSIVE-OFF: two:
-; EXPENSIVE-OFF-NEXT: br label [[END]]
-; EXPENSIVE-OFF: end:
-; EXPENSIVE-OFF-NEXT: ret i64 undef
-;
-; EXPENSIVE-ON-LABEL: @fuzz15217(
-; EXPENSIVE-ON-NEXT: entry:
-; EXPENSIVE-ON-NEXT: br i1 [[COND:%.*]], label [[END:%.*]], label [[TWO:%.*]]
-; EXPENSIVE-ON: two:
-; EXPENSIVE-ON-NEXT: br label [[END]]
-; EXPENSIVE-ON: end:
-; EXPENSIVE-ON-NEXT: ret i64 0
+; CHECK-LABEL: @fuzz15217(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: br i1 [[COND:%.*]], label [[END:%.*]], label [[TWO:%.*]]
+; CHECK: two:
+; CHECK-NEXT: br label [[END]]
+; CHECK: end:
+; CHECK-NEXT: ret i64 undef
;
entry:
br i1 %cond, label %end, label %two
More information about the llvm-commits
mailing list