From e492f0e03b01a5e4ec4b6333abb02d303c3e479e Mon Sep 17 00:00:00 2001 From: Roman Lebedev Date: Sat, 8 Aug 2020 17:57:32 +0300 Subject: [PATCH] [SimplifyCFG] Fix invoke->call fold w/ multiple invokes in presence of lifetime intrinsics SimplifyCFG has two main folds for resumes - one when resume is directly using the landingpad, and the other one where resume is using a PHI node. While for the first case, we were already correctly ignoring all the PHI nodes, and both the debug info intrinsics and lifetime intrinsics, in the PHI-based-one, we weren't ignoring PHI's in the resume block, and weren't ignoring lifetime intrinsics. That is clearly a bug. On RawSpeed library, this results in +9.34% (+81) more invoke->call folds, -0.19% (-39) landing pads, -0.24% (-81) invoke instructions but +51 call instructions and -132 basic blocks. Though, the run-time performance impact appears to be within the noise. --- llvm/lib/Transforms/Utils/SimplifyCFG.cpp | 67 +++++++++------------- .../SimplifyCFG/invoke_unwind_lifetime.ll | 23 +------- 2 files changed, 31 insertions(+), 59 deletions(-) diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp index 1f8e0d9..36161e3 100644 --- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp +++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp @@ -3958,17 +3958,36 @@ bool SimplifyCFGOpt::simplifyResume(ResumeInst *RI, IRBuilder<> &Builder) { return false; } +// Check if cleanup block is empty +static bool isCleanupBlockEmpty(iterator_range R) { + for (Instruction &I : R) { + auto *II = dyn_cast(&I); + if (!II) + return false; + + Intrinsic::ID IntrinsicID = II->getIntrinsicID(); + switch (IntrinsicID) { + case Intrinsic::dbg_declare: + case Intrinsic::dbg_value: + case Intrinsic::dbg_label: + case Intrinsic::lifetime_end: + break; + default: + return false; + } + } + return true; +} + // Simplify resume that is shared by several landing pads (phi of landing pad). bool SimplifyCFGOpt::simplifyCommonResume(ResumeInst *RI) { BasicBlock *BB = RI->getParent(); - // Check that there are no other instructions except for debug intrinsics - // between the phi of landing pads (RI->getValue()) and resume instruction. - BasicBlock::iterator I = cast(RI->getValue())->getIterator(), - E = RI->getIterator(); - while (++I != E) - if (!isa(I)) - return false; + // Check that there are no other instructions except for debug and lifetime + // intrinsics between the phi's and resume instruction. + if (!isCleanupBlockEmpty( + make_range(RI->getParent()->getFirstNonPHI(), BB->getTerminator()))) + return false; SmallSetVector TrivialUnwindBlocks; auto *PhiLPInst = cast(RI->getValue()); @@ -3989,17 +4008,8 @@ bool SimplifyCFGOpt::simplifyCommonResume(ResumeInst *RI) { if (IncomingValue != LandingPad) continue; - bool isTrivial = true; - - I = IncomingBB->getFirstNonPHI()->getIterator(); - E = IncomingBB->getTerminator()->getIterator(); - while (++I != E) - if (!isa(I)) { - isTrivial = false; - break; - } - - if (isTrivial) + if (isCleanupBlockEmpty( + make_range(LandingPad->getNextNode(), IncomingBB->getTerminator()))) TrivialUnwindBlocks.insert(IncomingBB); } @@ -4038,27 +4048,6 @@ bool SimplifyCFGOpt::simplifyCommonResume(ResumeInst *RI) { return !TrivialUnwindBlocks.empty(); } -// Check if cleanup block is empty -static bool isCleanupBlockEmpty(iterator_range R) { - for (Instruction &I : R) { - auto *II = dyn_cast(&I); - if (!II) - return false; - - Intrinsic::ID IntrinsicID = II->getIntrinsicID(); - switch (IntrinsicID) { - case Intrinsic::dbg_declare: - case Intrinsic::dbg_value: - case Intrinsic::dbg_label: - case Intrinsic::lifetime_end: - break; - default: - return false; - } - } - return true; -} - // Simplify resume that is only used by a single (non-phi) landing pad. bool SimplifyCFGOpt::simplifySingleResume(ResumeInst *RI) { BasicBlock *BB = RI->getParent(); diff --git a/llvm/test/Transforms/SimplifyCFG/invoke_unwind_lifetime.ll b/llvm/test/Transforms/SimplifyCFG/invoke_unwind_lifetime.ll index 5be6012..a498976 100644 --- a/llvm/test/Transforms/SimplifyCFG/invoke_unwind_lifetime.ll +++ b/llvm/test/Transforms/SimplifyCFG/invoke_unwind_lifetime.ll @@ -32,28 +32,11 @@ define void @caller(i1 %c) personality i8* bitcast (i32 (...)* @__gxx_personalit ; CHECK-NEXT: call void @escape(i32* [[I6]]) ; CHECK-NEXT: br i1 [[C:%.*]], label [[V0:%.*]], label [[V1:%.*]] ; CHECK: v0: -; CHECK-NEXT: invoke void @throwing_callee_foo() -; CHECK-NEXT: to label [[INVOKE_CONT:%.*]] unwind label [[LPAD_V0:%.*]] +; CHECK-NEXT: call void @throwing_callee_foo() +; CHECK-NEXT: unreachable ; CHECK: v1: -; CHECK-NEXT: invoke void @throwing_callee_bar() -; CHECK-NEXT: to label [[INVOKE_CONT]] unwind label [[LPAD_V1:%.*]] -; CHECK: invoke.cont: +; CHECK-NEXT: call void @throwing_callee_bar() ; CHECK-NEXT: unreachable -; CHECK: lpad.v0: -; CHECK-NEXT: [[I8:%.*]] = landingpad { i8*, i32 } -; CHECK-NEXT: cleanup -; CHECK-NEXT: call void @llvm.lifetime.end.p0i8(i64 4, i8* nonnull [[I1]]) -; CHECK-NEXT: br label [[END:%.*]] -; CHECK: lpad.v1: -; CHECK-NEXT: [[I9:%.*]] = landingpad { i8*, i32 } -; CHECK-NEXT: cleanup -; CHECK-NEXT: call void @llvm.lifetime.end.p0i8(i64 4, i8* nonnull [[I3]]) -; CHECK-NEXT: br label [[END]] -; CHECK: end: -; CHECK-NEXT: [[I10:%.*]] = phi { i8*, i32 } [ [[I8]], [[LPAD_V0]] ], [ [[I9]], [[LPAD_V1]] ] -; CHECK-NEXT: [[I11:%.*]] = phi i8* [ [[I5]], [[LPAD_V0]] ], [ [[I7]], [[LPAD_V1]] ] -; CHECK-NEXT: call void @llvm.lifetime.end.p0i8(i64 4, i8* nonnull [[I11]]) -; CHECK-NEXT: resume { i8*, i32 } [[I10]] ; entry: %i0 = alloca i32 -- 2.7.4