[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

3 files changed, 120 insertions, 1 deletions

diff --git a/llvm/lib/Transforms/Scalar/ConstantHoisting.cpp b/llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
index e2ed488e398..5bfece010be 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 different
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 00000000000..6f8d00f664e
--- /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.
+
+@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 00000000000..a5ca9b4b859
--- /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"
+
+@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
+}