[llvm] a4088c7 - [Attributor][FIX] Carefully change invokes to calls (after manifest)
Johannes Doerfert via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 8 17:33:32 PST 2020
Author: Johannes Doerfert
Date: 2020-01-08T19:32:38-06:00
New Revision: a4088c75cc1034307400076d29b35905d0ae58b2
URL: https://github.com/llvm/llvm-project/commit/a4088c75cc1034307400076d29b35905d0ae58b2
DIFF: https://github.com/llvm/llvm-project/commit/a4088c75cc1034307400076d29b35905d0ae58b2.diff
LOG: [Attributor][FIX] Carefully change invokes to calls (after manifest)
Before we manually inserted unreachable early but that could lead to
broken PHI nodes. Now we use the existing late modification
functionality.
Added:
Modified:
llvm/include/llvm/Transforms/IPO/Attributor.h
llvm/lib/Transforms/IPO/Attributor.cpp
llvm/test/Transforms/Attributor/ArgumentPromotion/crash.ll
llvm/test/Transforms/Attributor/ArgumentPromotion/fp80.ll
llvm/test/Transforms/Attributor/ArgumentPromotion/live_called_from_dead.ll
llvm/test/Transforms/Attributor/ArgumentPromotion/nonzero-address-spaces.ll
llvm/test/Transforms/Attributor/IPConstantProp/recursion.ll
llvm/test/Transforms/Attributor/IPConstantProp/return-constant.ll
llvm/test/Transforms/Attributor/liveness.ll
llvm/test/Transforms/Attributor/noreturn_async.ll
Removed:
################################################################################
diff --git a/llvm/include/llvm/Transforms/IPO/Attributor.h b/llvm/include/llvm/Transforms/IPO/Attributor.h
index 4edfec2e982d..70642b201adb 100644
--- a/llvm/include/llvm/Transforms/IPO/Attributor.h
+++ b/llvm/include/llvm/Transforms/IPO/Attributor.h
@@ -865,6 +865,13 @@ struct Attributor {
ToBeChangedToUnreachableInsts.insert(I);
}
+ /// Record that \p II has at least one dead successor block. This information
+ /// is used, e.g., to replace \p II with a call, after information was
+ /// manifested.
+ void registerInvokeWithDeadSuccessor(InvokeInst &II) {
+ InvokeWithDeadSuccessor.push_back(&II);
+ }
+
/// Record that \p I is deleted after information was manifested. This also
/// triggers deletion of trivially dead istructions.
void deleteAfterManifest(Instruction &I) { ToBeDeletedInsts.insert(&I); }
@@ -1176,6 +1183,9 @@ struct Attributor {
/// Instructions we replace with `unreachable` insts after manifest is done.
SmallDenseSet<WeakVH, 16> ToBeChangedToUnreachableInsts;
+ /// Invoke instructions with at least a single dead successor block.
+ SmallVector<WeakVH, 16> InvokeWithDeadSuccessor;
+
/// Functions, blocks, and instructions we delete after manifest is done.
///
///{
diff --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp
index b3b9e1e185a6..39e2057b1b6e 100644
--- a/llvm/lib/Transforms/IPO/Attributor.cpp
+++ b/llvm/lib/Transforms/IPO/Attributor.cpp
@@ -31,6 +31,7 @@
#include "llvm/IR/CFG.h"
#include "llvm/IR/InstIterator.h"
#include "llvm/IR/IntrinsicInst.h"
+#include "llvm/IR/Verifier.h"
#include "llvm/InitializePasses.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/Debug.h"
@@ -2824,90 +2825,12 @@ struct AAIsDeadFunction : public AAIsDead {
bool MayReturn = !NoReturnAA.isAssumedNoReturn();
if (MayReturn && (!Invoke2CallAllowed || !isa<InvokeInst>(CB)))
continue;
- Instruction *I = const_cast<Instruction *>(DeadEndI);
- BasicBlock *BB = I->getParent();
- Instruction *SplitPos = I->getNextNode();
- // TODO: mark stuff before unreachable instructions as dead.
-
- if (auto *II = dyn_cast<InvokeInst>(I)) {
- // If we keep the invoke the split position is at the beginning of the
- // normal desitination block (it invokes a noreturn function after all).
- BasicBlock *NormalDestBB = II->getNormalDest();
- SplitPos = &NormalDestBB->front();
-
- /// Invoke is replaced with a call and unreachable is placed after it if
- /// the callee is nounwind and noreturn. Otherwise, we keep the invoke
- /// and only place an unreachable in the normal successor.
- if (Invoke2CallAllowed) {
- if (II->getCalledFunction()) {
- const IRPosition &IPos = IRPosition::callsite_function(*II);
- const auto &AANoUnw = A.getAAFor<AANoUnwind>(*this, IPos);
- if (AANoUnw.isAssumedNoUnwind()) {
- LLVM_DEBUG(dbgs()
- << "[AAIsDead] Replace invoke with call inst\n");
- CallInst *CI = createCallMatchingInvoke(II);
- CI->insertBefore(II);
- CI->takeName(II);
- replaceAllInstructionUsesWith(*II, *CI);
-
- // If this is a nounwind + mayreturn invoke we only remove the
- // unwind edge. This is done by moving the invoke into a new and
- // dead block and connecting the normal destination of the invoke
- // with a branch that follows the call replacement we created
- // above.
- if (MayReturn) {
- BasicBlock *NewDeadBB =
- SplitBlock(BB, II, nullptr, nullptr, nullptr, ".i2c");
- assert(isa<BranchInst>(BB->getTerminator()) &&
- BB->getTerminator()->getNumSuccessors() == 1 &&
- BB->getTerminator()->getSuccessor(0) == NewDeadBB);
- new UnreachableInst(I->getContext(), NewDeadBB);
- BB->getTerminator()->setOperand(0, NormalDestBB);
- A.deleteAfterManifest(*II);
- continue;
- }
-
- // We do not need an invoke (II) but instead want a call followed
- // by an unreachable. However, we do not remove II as other
- // abstract attributes might have it cached as part of their
- // results. Given that we modify the CFG anyway, we simply keep II
- // around but in a new dead block. To avoid II being live through
- // a
diff erent edge we have to ensure the block we place it in is
- // only reached from the current block of II and then not reached
- // at all when we insert the unreachable.
- SplitBlockPredecessors(NormalDestBB, {BB}, ".i2c");
- SplitPos = CI->getNextNode();
- }
- }
- }
-
- if (SplitPos == &NormalDestBB->front()) {
- // If this is an invoke of a noreturn function the edge to the normal
- // destination block is dead but not necessarily the block itself.
- // TODO: We need to move to an edge based system during deduction and
- // also manifest.
- assert(!NormalDestBB->isLandingPad() &&
- "Expected the normal destination not to be a landingpad!");
- if (NormalDestBB->getUniquePredecessor() == BB) {
- assumeLive(A, *NormalDestBB);
- } else {
- BasicBlock *SplitBB =
- SplitBlockPredecessors(NormalDestBB, {BB}, ".dead");
- // The split block is live even if it contains only an unreachable
- // instruction at the end.
- assumeLive(A, *SplitBB);
- SplitPos = SplitBB->getTerminator();
- HasChanged = ChangeStatus::CHANGED;
- }
- }
- }
- if (isa_and_nonnull<UnreachableInst>(SplitPos))
- continue;
-
- BB = SplitPos->getParent();
- SplitBlock(BB, SplitPos);
- A.changeToUnreachableAfterManifest(BB->getTerminator());
+ if (auto *II = dyn_cast<InvokeInst>(DeadEndI))
+ A.registerInvokeWithDeadSuccessor(const_cast<InvokeInst &>(*II));
+ else
+ A.changeToUnreachableAfterManifest(
+ const_cast<Instruction *>(DeadEndI->getNextNode()));
HasChanged = ChangeStatus::CHANGED;
}
@@ -5668,6 +5591,32 @@ ChangeStatus Attributor::run(Module &M) {
}
}
}
+ for (auto &V : InvokeWithDeadSuccessor)
+ if (InvokeInst *II = dyn_cast_or_null<InvokeInst>(V)) {
+ bool UnwindBBIsDead = II->hasFnAttr(Attribute::NoUnwind);
+ bool NormalBBIsDead = II->hasFnAttr(Attribute::NoReturn);
+ bool Invoke2CallAllowed =
+ !AAIsDeadFunction::mayCatchAsynchronousExceptions(
+ *II->getFunction());
+ assert((UnwindBBIsDead || NormalBBIsDead) &&
+ "Invoke does not have dead successors!");
+ BasicBlock *BB = II->getParent();
+ BasicBlock *NormalDestBB = II->getNormalDest();
+ if (UnwindBBIsDead) {
+ Instruction *NormalNextIP = &NormalDestBB->front();
+ if (Invoke2CallAllowed) {
+ changeToCall(II);
+ NormalNextIP = BB->getTerminator();
+ }
+ if (NormalBBIsDead)
+ ToBeChangedToUnreachableInsts.insert(NormalNextIP);
+ } else {
+ assert(NormalBBIsDead && "Broken invariant!");
+ if (!NormalDestBB->getUniquePredecessor())
+ NormalDestBB = SplitBlockPredecessors(NormalDestBB, {BB}, ".dead");
+ ToBeChangedToUnreachableInsts.insert(&NormalDestBB->front());
+ }
+ }
for (auto &V : ToBeChangedToUnreachableInsts)
if (Instruction *I = dyn_cast_or_null<Instruction>(V))
changeToUnreachable(I, /* UseLLVMTrap */ false);
@@ -6337,7 +6286,9 @@ static bool runAttributorOnModule(Module &M, AnalysisGetter &AG) {
A.identifyDefaultAbstractAttributes(F);
}
- return A.run(M) == ChangeStatus::CHANGED;
+ bool Changed = A.run(M) == ChangeStatus::CHANGED;
+ assert(!verifyModule(M, &errs()) && "Module verification failed!");
+ return Changed;
}
PreservedAnalyses AttributorPass::run(Module &M, ModuleAnalysisManager &AM) {
diff --git a/llvm/test/Transforms/Attributor/ArgumentPromotion/crash.ll b/llvm/test/Transforms/Attributor/ArgumentPromotion/crash.ll
index 87a3ba5811e1..db3db632e5f2 100644
--- a/llvm/test/Transforms/Attributor/ArgumentPromotion/crash.ll
+++ b/llvm/test/Transforms/Attributor/ArgumentPromotion/crash.ll
@@ -11,10 +11,6 @@ define void @zot() personality i32 (...)* @wibble {
; ATTRIBUTOR-NEXT: bb:
; ATTRIBUTOR-NEXT: call void @hoge()
; ATTRIBUTOR-NEXT: unreachable
-; ATTRIBUTOR: bb.split:
-; ATTRIBUTOR-NEXT: unreachable
-; ATTRIBUTOR: bb1.i2c:
-; ATTRIBUTOR-NEXT: unreachable
; ATTRIBUTOR: bb1:
; ATTRIBUTOR-NEXT: unreachable
; ATTRIBUTOR: bb2:
@@ -47,8 +43,6 @@ define internal void @hoge() {
; ATTRIBUTOR-LABEL: define {{[^@]+}}@hoge()
; ATTRIBUTOR-NEXT: bb:
; ATTRIBUTOR-NEXT: unreachable
-; ATTRIBUTOR: bb.split:
-; ATTRIBUTOR-NEXT: unreachable
;
bb:
%tmp = call fastcc i8* @spam(i1 (i8*)* @eggs)
@@ -77,8 +71,6 @@ define i32 @test_inf_promote_caller(i32 %arg) {
; CHECK-SAME: (i32 [[ARG:%.*]])
; CHECK-NEXT: bb:
; CHECK-NEXT: unreachable
-; CHECK: bb.split:
-; CHECK-NEXT: unreachable
;
bb:
%tmp = alloca %S
diff --git a/llvm/test/Transforms/Attributor/ArgumentPromotion/fp80.ll b/llvm/test/Transforms/Attributor/ArgumentPromotion/fp80.ll
index 36adfe08a4d2..153ce6893ba2 100644
--- a/llvm/test/Transforms/Attributor/ArgumentPromotion/fp80.ll
+++ b/llvm/test/Transforms/Attributor/ArgumentPromotion/fp80.ll
@@ -17,8 +17,6 @@ define void @run() {
; CHECK-NEXT: entry:
; CHECK-NEXT: [[TMP0:%.*]] = call i64 @CaptureAStruct(%struct.Foo* nofree nonnull readonly align 8 dereferenceable(16) @a)
; CHECK-NEXT: unreachable
-; CHECK: entry.split:
-; CHECK-NEXT: unreachable
;
entry:
tail call i8 @UseLongDoubleUnsafely(%union.u* byval align 16 bitcast (%struct.s* @b to %union.u*))
diff --git a/llvm/test/Transforms/Attributor/ArgumentPromotion/live_called_from_dead.ll b/llvm/test/Transforms/Attributor/ArgumentPromotion/live_called_from_dead.ll
index b81d35491f94..a5ca51e9bd99 100644
--- a/llvm/test/Transforms/Attributor/ArgumentPromotion/live_called_from_dead.ll
+++ b/llvm/test/Transforms/Attributor/ArgumentPromotion/live_called_from_dead.ll
@@ -1,4 +1,4 @@
-; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature --scrub-attributes
; RUN: opt -S -basicaa -attributor -attributor-disable=false -attributor-max-iterations-verify -attributor-max-iterations=3 < %s | FileCheck %s --check-prefixes=CHECK,OLDPM_MODULE
; RUN: opt -S -passes='attributor' -aa-pipeline='basic-aa' -attributor-disable=false -attributor-max-iterations-verify -attributor-max-iterations=3 < %s | FileCheck %s --check-prefixes=CHECK,NEWPM_MODULE
diff --git a/llvm/test/Transforms/Attributor/ArgumentPromotion/nonzero-address-spaces.ll b/llvm/test/Transforms/Attributor/ArgumentPromotion/nonzero-address-spaces.ll
index 271854a22456..d08969c0a262 100644
--- a/llvm/test/Transforms/Attributor/ArgumentPromotion/nonzero-address-spaces.ll
+++ b/llvm/test/Transforms/Attributor/ArgumentPromotion/nonzero-address-spaces.ll
@@ -13,8 +13,6 @@ define i32 @bar() {
; CHECK-NEXT: entry:
; CHECK-NEXT: [[CALL:%.*]] = call addrspace(1) i32 @foo()
; CHECK-NEXT: unreachable
-; CHECK: entry.split:
-; CHECK-NEXT: unreachable
;
entry:
diff --git a/llvm/test/Transforms/Attributor/IPConstantProp/recursion.ll b/llvm/test/Transforms/Attributor/IPConstantProp/recursion.ll
index b9fd0468d380..fc82342a989f 100644
--- a/llvm/test/Transforms/Attributor/IPConstantProp/recursion.ll
+++ b/llvm/test/Transforms/Attributor/IPConstantProp/recursion.ll
@@ -12,8 +12,6 @@ define internal i32 @foo(i32 %X) {
define void @bar() {
; CHECK-LABEL: define {{[^@]+}}@bar()
; CHECK-NEXT: unreachable
-; CHECK: .split:
-; CHECK-NEXT: unreachable
;
call i32 @foo( i32 17 ) ; <i32>:1 [#uses=0]
ret void
diff --git a/llvm/test/Transforms/Attributor/IPConstantProp/return-constant.ll b/llvm/test/Transforms/Attributor/IPConstantProp/return-constant.ll
index 04927726daa2..f30461c746af 100644
--- a/llvm/test/Transforms/Attributor/IPConstantProp/return-constant.ll
+++ b/llvm/test/Transforms/Attributor/IPConstantProp/return-constant.ll
@@ -8,8 +8,6 @@ define i1 @invokecaller(i1 %C) personality i32 (...)* @__gxx_personality_v0 {
; CHECK-SAME: (i1 [[C:%.*]]) #0 personality i32 (...)* @__gxx_personality_v0
; CHECK-NEXT: [[X:%.*]] = call i32 @foo(i1 [[C]])
; CHECK-NEXT: br label [[OK:%.*]]
-; CHECK: .i2c:
-; CHECK-NEXT: unreachable
; CHECK: OK:
; CHECK-NEXT: [[Y:%.*]] = icmp ne i32 52, 0
; CHECK-NEXT: ret i1 [[Y]]
diff --git a/llvm/test/Transforms/Attributor/liveness.ll b/llvm/test/Transforms/Attributor/liveness.ll
index cf9dc8b789dd..4dd37865fcc0 100644
--- a/llvm/test/Transforms/Attributor/liveness.ll
+++ b/llvm/test/Transforms/Attributor/liveness.ll
@@ -277,6 +277,96 @@ cleanup:
ret i32 0
}
+; UTC_ARGS: --turn on
+
+; TEST 5.4 unounwind invoke instruction replaced by a call and a branch instruction put after it.
+define i32 @invoke_nounwind_phi(i32 %a) personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
+; CHECK-LABEL: define {{[^@]+}}@invoke_nounwind_phi
+; CHECK-SAME: (i32 [[A:%.*]]) personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*)
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[A]], 0
+; CHECK-NEXT: br i1 [[CMP]], label [[COND_TRUE:%.*]], label [[COND_FALSE:%.*]]
+; CHECK: cond.true:
+; CHECK-NEXT: call void @normal_call()
+; CHECK-NEXT: [[CALL:%.*]] = call i32 @foo_nounwind()
+; CHECK-NEXT: br label [[CONTINUE:%.*]]
+; CHECK: cond.false:
+; CHECK-NEXT: call void @normal_call()
+; CHECK-NEXT: [[CALL1:%.*]] = call i32 @bar()
+; CHECK-NEXT: br label [[CONTINUE]]
+; CHECK: continue:
+; CHECK-NEXT: [[P:%.*]] = phi i32 [ 0, [[COND_TRUE]] ], [ 1, [[COND_FALSE]] ]
+; CHECK-NEXT: ret i32 [[P]]
+; CHECK: cleanup:
+; CHECK-NEXT: unreachable
+;
+entry:
+ %cmp = icmp eq i32 %a, 0
+ br i1 %cmp, label %cond.true, label %cond.false
+
+cond.true: ; preds = %entry
+ call void @normal_call()
+ %call = invoke i32 @foo_nounwind() to label %continue
+ unwind label %cleanup
+
+cond.false: ; preds = %entry
+ call void @normal_call()
+ %call1 = call i32 @bar()
+ br label %continue
+
+continue:
+ %p = phi i32 [ 0, %cond.true ], [ 1, %cond.false ]
+ ret i32 %p
+
+cleanup:
+ %res = landingpad { i8*, i32 } catch i8* null
+ ret i32 0
+}
+
+; TEST 5.5 unounwind invoke instruction replaced by a call and a branch instruction put after it.
+define i32 @invoke_nounwind_phi_dom(i32 %a) personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
+; CHECK-LABEL: define {{[^@]+}}@invoke_nounwind_phi_dom
+; CHECK-SAME: (i32 [[A:%.*]]) personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*)
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[A]], 0
+; CHECK-NEXT: br i1 [[CMP]], label [[COND_TRUE:%.*]], label [[COND_FALSE:%.*]]
+; CHECK: cond.true:
+; CHECK-NEXT: call void @normal_call()
+; CHECK-NEXT: [[CALL:%.*]] = call i32 @foo_nounwind()
+; CHECK-NEXT: br label [[CONTINUE:%.*]]
+; CHECK: cond.false:
+; CHECK-NEXT: call void @normal_call()
+; CHECK-NEXT: [[CALL1:%.*]] = call i32 @bar()
+; CHECK-NEXT: br label [[CONTINUE]]
+; CHECK: continue:
+; CHECK-NEXT: [[P:%.*]] = phi i32 [ [[CALL]], [[COND_TRUE]] ], [ [[CALL1]], [[COND_FALSE]] ]
+; CHECK-NEXT: ret i32 [[P]]
+; CHECK: cleanup:
+; CHECK-NEXT: unreachable
+;
+entry:
+ %cmp = icmp eq i32 %a, 0
+ br i1 %cmp, label %cond.true, label %cond.false
+
+cond.true: ; preds = %entry
+ call void @normal_call()
+ %call = invoke i32 @foo_nounwind() to label %continue
+ unwind label %cleanup
+
+cond.false: ; preds = %entry
+ call void @normal_call()
+ %call1 = call i32 @bar()
+ br label %continue
+
+continue:
+ %p = phi i32 [ %call, %cond.true ], [ %call1, %cond.false ]
+ ret i32 %p
+
+cleanup:
+ %res = landingpad { i8*, i32 } catch i8* null
+ ret i32 0
+}
+
; UTC_ARGS: --turn off
; TEST 6: Undefined behvior, taken from LangRef.
@@ -707,7 +797,6 @@ define internal void @dead_e2() { ret void }
; CHECK-NEXT: define internal void @non_dead_d15()
; CHECK-NOT: define internal void @dead_e
-
declare void @blowup() noreturn
define void @live_with_dead_entry() personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
; CHECK: define void @live_with_dead_entry(
@@ -735,19 +824,19 @@ define void @live_with_dead_entry_lp() personality i8* bitcast (i32 (...)* @__gx
; CHECK: define void @live_with_dead_entry_lp(
; CHECK-NEXT: entry:
; CHECK-NEXT: invoke void @blowup()
-; CHECK-NEXT: to label %live_with_dead_entry.dead unwind label %lp1
-; CHECK: lp1: ; preds = %entry
+; CHECK-NEXT: to label %[[LIVE_WITH_DEAD_ENTRY_DEAD1:.*]] unwind label %[[LP1:.*]]
+; CHECK: [[LP1]]: ; preds = %entry
; CHECK-NEXT: %lp = landingpad { i8*, i32 }
; CHECK-NEXT: catch i8* null
; CHECK-NEXT: invoke void @blowup()
-; CHECK-NEXT: to label %live_with_dead_entry.dead1 unwind label %lp2
-; CHECK: lp2: ; preds = %lp1
+; CHECK-NEXT: to label %[[LIVE_WITH_DEAD_ENTRY_DEAD2:.*]] unwind label %[[LP2:.*]]
+; CHECK: [[LP2]]: ; preds = %lp1
; CHECK-NEXT: %0 = landingpad { i8*, i32 }
; CHECK-NEXT: catch i8* null
; CHECK-NEXT: br label %live_with_dead_entry
-; CHECK: live_with_dead_entry.dead:
+; CHECK: [[LIVE_WITH_DEAD_ENTRY_DEAD1]]:
; CHECK-NEXT: unreachable
-; CHECK: live_with_dead_entry.dead1:
+; CHECK: [[LIVE_WITH_DEAD_ENTRY_DEAD2]]:
; CHECK-NEXT: unreachable
; CHECK: live_with_dead_entry: ; preds = %lp2
; CHECK-NEXT: ret void
diff --git a/llvm/test/Transforms/Attributor/noreturn_async.ll b/llvm/test/Transforms/Attributor/noreturn_async.ll
index 7c00a5a0b5cb..9fb99159acf5 100644
--- a/llvm/test/Transforms/Attributor/noreturn_async.ll
+++ b/llvm/test/Transforms/Attributor/noreturn_async.ll
@@ -100,9 +100,9 @@ define dso_local i32 @"?catchoverflow@@YAHXZ_may_throw"() personality i8* bitca
entry:
%retval = alloca i32, align 4
%__exception_code = alloca i32, align 4
-; CHECK: invoke void @"?overflow@@YAXXZ_may_throw"()
+; CHECK: invoke void @"?overflow@@YAXXZ_may_throw"()
; CHECK: to label %invoke.cont unwind label %catch.dispatch
- invoke void @"?overflow@@YAXXZ_may_throw"()
+ invoke void @"?overflow@@YAXXZ_may_throw"()
to label %invoke.cont unwind label %catch.dispatch
invoke.cont: ; preds = %entry
More information about the llvm-commits
mailing list