From: Bjorn Pettersson Date: Wed, 18 Dec 2019 12:30:47 +0000 (+0100) Subject: [ConstantHoisting] Ignore unreachable bb:s when collecting candidates X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=89e3bb4502631543e648cd387405004850871af0;p=platform%2Fupstream%2Fllvm.git [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 --- diff --git a/llvm/lib/Transforms/Scalar/ConstantHoisting.cpp b/llvm/lib/Transforms/Scalar/ConstantHoisting.cpp index e2ed488..5bfece0 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 0000000..6f8d00f --- /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 0000000..a5ca9b4 --- /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 +}