[llvm] 89e3bb4 - [ConstantHoisting] Ignore unreachable bb:s when collecting candidates
Bjorn Pettersson via llvm-commits
llvm-commits at lists.llvm.org
Thu Dec 19 06:13:45 PST 2019
Author: Bjorn Pettersson
Date: 2019-12-19T15:07:55+01:00
New Revision: 89e3bb4502631543e648cd387405004850871af0
URL: https://github.com/llvm/llvm-project/commit/89e3bb4502631543e648cd387405004850871af0
DIFF: https://github.com/llvm/llvm-project/commit/89e3bb4502631543e648cd387405004850871af0.diff
LOG: [ConstantHoisting] Ignore unreachable bb:s when collecting candidates
Summary:
Ignore looking at blocks that are unreachable from entry when
collecting candidates for hosting.
Normally the consthoist pass is executed in the llc pipeline,
just after unreachableblockelim. So it is abnormal to have code
that is unreachable from the entry block. But when running the
pass as part of opt, for example as part of fuzzy testing, we
might trigger various kinds of asserts when collecting candidates
if we include unreachable blocks in that analysis.
It seems like a waste of time to hoist constants in unreachble
blocks, so the solution is to simply ignore such blocks when
collecting the hoisting candidates.
The two added test cases used to end up in two different asserts,
and the intention with the checks is just to verify that we no
longer fail.
Fixes: PR43903
Reviewers: spatel
Reviewed By: spatel
Subscribers: hiraditya, uabelho, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D71678
Added:
llvm/test/Transforms/ConstantHoisting/AArch64/consthoist-unreachable.ll
llvm/test/Transforms/ConstantHoisting/X86/pr43903-not-all-uses-rebased.ll
Modified:
llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Scalar/ConstantHoisting.cpp b/llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
index e2ed488e3988..5bfece010bec 100644
--- a/llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
+++ b/llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
@@ -501,9 +501,13 @@ void ConstantHoistingPass::collectConstantCandidates(
/// into an instruction itself.
void ConstantHoistingPass::collectConstantCandidates(Function &Fn) {
ConstCandMapType ConstCandMap;
- for (BasicBlock &BB : Fn)
+ for (BasicBlock &BB : Fn) {
+ // Ignore unreachable basic blocks.
+ if (!DT->isReachableFromEntry(&BB))
+ continue;
for (Instruction &Inst : BB)
collectConstantCandidates(ConstCandMap, &Inst);
+ }
}
// This helper function is necessary to deal with values that have
diff erent
diff --git a/llvm/test/Transforms/ConstantHoisting/AArch64/consthoist-unreachable.ll b/llvm/test/Transforms/ConstantHoisting/AArch64/consthoist-unreachable.ll
new file mode 100755
index 000000000000..6f8d00f664e8
--- /dev/null
+++ b/llvm/test/Transforms/ConstantHoisting/AArch64/consthoist-unreachable.ll
@@ -0,0 +1,64 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -mtriple aarch64-- -consthoist -S | FileCheck %s
+
+; This used to trigger an assertion failure:
+;
+; ../lib/Transforms/Scalar/ConstantHoisting.cpp:779: void llvm::ConstantHoistingPass::emitBaseConstants(llvm::Instruction *, llvm::Constant *, llvm::Type *, const llvm::consthoist::ConstantUser &): Assertion `CastInst->isCast() && "Expected an cast instruction!"' failed.
+
+ at c.a = external global i32, align 1
+
+define void @c() {
+; CHECK-LABEL: @c(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[TOBOOL:%.*]] = icmp ne i16 0, 0
+; CHECK-NEXT: br i1 undef, label [[LBL1_US:%.*]], label [[ENTRY_ENTRY_SPLIT_CRIT_EDGE:%.*]]
+; CHECK: entry.entry.split_crit_edge:
+; CHECK-NEXT: [[CONST:%.*]] = bitcast i32 1232131 to i32
+; CHECK-NEXT: br label [[LBL1:%.*]]
+; CHECK: lbl1.us:
+; CHECK-NEXT: [[CONST1:%.*]] = bitcast i32 1232131 to i32
+; CHECK-NEXT: store i32 [[CONST1]], i32* @c.a, align 1
+; CHECK-NEXT: br label [[FOR_COND4:%.*]]
+; CHECK: lbl1:
+; CHECK-NEXT: store i32 [[CONST]], i32* @c.a, align 1
+; CHECK-NEXT: br i1 undef, label [[IF_THEN:%.*]], label [[FOR_END12:%.*]]
+; CHECK: if.then:
+; CHECK-NEXT: br i1 undef, label [[LBL1]], label [[FOR_COND4]]
+; CHECK: for.cond4:
+; CHECK-NEXT: br label [[FOR_COND4]]
+; CHECK: for.body9:
+; CHECK-NEXT: store i32 1232131, i32* undef, align 1
+; CHECK-NEXT: store i32 1232132, i32* undef, align 1
+; CHECK-NEXT: br label [[FOR_BODY9:%.*]]
+; CHECK: for.end12:
+; CHECK-NEXT: ret void
+;
+entry:
+ %tobool = icmp ne i16 0, 0
+ br i1 undef, label %lbl1.us, label %entry.entry.split_crit_edge
+
+entry.entry.split_crit_edge: ; preds = %entry
+ br label %lbl1
+
+lbl1.us: ; preds = %entry
+ store i32 1232131, i32* @c.a, align 1
+ br label %for.cond4
+
+lbl1: ; preds = %if.then, %entry.entry.split_crit_edge
+ store i32 1232131, i32* @c.a, align 1
+ br i1 undef, label %if.then, label %for.end12
+
+if.then: ; preds = %lbl1
+ br i1 undef, label %lbl1, label %for.cond4
+
+for.cond4: ; preds = %for.cond4, %if.then, %lbl1.us
+ br label %for.cond4
+
+for.body9: ; preds = %for.body9
+ store i32 1232131, i32* undef, align 1
+ store i32 1232132, i32* undef, align 1
+ br label %for.body9
+
+for.end12: ; preds = %lbl1
+ ret void
+}
diff --git a/llvm/test/Transforms/ConstantHoisting/X86/pr43903-not-all-uses-rebased.ll b/llvm/test/Transforms/ConstantHoisting/X86/pr43903-not-all-uses-rebased.ll
new file mode 100644
index 000000000000..a5ca9b4b859e
--- /dev/null
+++ b/llvm/test/Transforms/ConstantHoisting/X86/pr43903-not-all-uses-rebased.ll
@@ -0,0 +1,51 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -mtriple=x86_64-unknown-linux-gnu -consthoist -consthoist-gep=1 -S | FileCheck %s
+
+; This is a reproducer for PR43903 where we hit an assertion:
+; opt: ../lib/Transforms/Scalar/ConstantHoisting.cpp:903: bool llvm::ConstantHoistingPass::emitBaseConstants(llvm::GlobalVariable *): Assertion `UsesNum == (ReBasesNum + NotRebasedNum) && "Not all uses are rebased"' failed.
+
+target triple = "x86_64-unknown-linux-gnu"
+
+ at a = external global [2 x i16], align 1
+
+define void @c() {
+; CHECK-LABEL: @c(
+; CHECK-NEXT: for.cond:
+; CHECK-NEXT: br i1 undef, label [[FOR_BODY2:%.*]], label [[FOR_END4:%.*]]
+; CHECK: for.body2:
+; CHECK-NEXT: br i1 undef, label [[LAND_RHS:%.*]], label [[LAND_END:%.*]]
+; CHECK: land.rhs:
+; CHECK-NEXT: unreachable
+; CHECK: land.end:
+; CHECK-NEXT: [[CONST1:%.*]] = bitcast i16* getelementptr inbounds ([2 x i16], [2 x i16]* @a, i32 0, i32 0) to i16*
+; CHECK-NEXT: [[CMP:%.*]] = icmp ule i16* undef, [[CONST1]]
+; CHECK-NEXT: unreachable
+; CHECK: for.cond3:
+; CHECK-NEXT: [[TMP0:%.*]] = load i16, i16* getelementptr inbounds ([2 x i16], [2 x i16]* @a, i32 0, i32 1), align 1
+; CHECK-NEXT: br label [[FOR_COND3:%.*]]
+; CHECK: for.end4:
+; CHECK-NEXT: [[CONST:%.*]] = bitcast i16* getelementptr inbounds ([2 x i16], [2 x i16]* @a, i32 0, i32 0) to i16*
+; CHECK-NEXT: [[TMP1:%.*]] = load i16, i16* [[CONST]], align 1
+; CHECK-NEXT: ret void
+;
+for.cond:
+ br i1 undef, label %for.body2, label %for.end4
+
+for.body2: ; preds = %for.cond
+ br i1 undef, label %land.rhs, label %land.end
+
+land.rhs: ; preds = %for.body2
+ unreachable
+
+land.end: ; preds = %for.body2
+ %cmp = icmp ule i16* undef, getelementptr inbounds ([2 x i16], [2 x i16]* @a, i32 0, i32 0)
+ unreachable
+
+for.cond3: ; preds = %for.cond3
+ %tmp0 = load i16, i16* getelementptr inbounds ([2 x i16], [2 x i16]* @a, i32 0, i32 1), align 1
+ br label %for.cond3
+
+for.end4: ; preds = %for.cond
+ %tmp1 = load i16, i16* getelementptr inbounds ([2 x i16], [2 x i16]* @a, i32 0, i32 0), align 1
+ ret void
+}
More information about the llvm-commits
mailing list