[llvm] r196710 - Fix inlining to not produce duplicate landingpad clauses
Mark Seaborn
mseaborn at chromium.org
Sat Dec 7 16:50:58 PST 2013
Author: mseaborn
Date: Sat Dec 7 18:50:58 2013
New Revision: 196710
URL: http://llvm.org/viewvc/llvm-project?rev=196710&view=rev
Log:
Fix inlining to not produce duplicate landingpad clauses
Before this change, inlining one "invoke" into an outer "invoke" call
site can lead to the outer landingpad's catch/filter clauses being
copied multiple times into the resulting landingpad. This happens:
* when the inlined function contains multiple "resume" instructions,
because forwardResume() copies the clauses but is called multiple
times;
* when the inlined function contains a "resume" and a "call", because
HandleCallsInBlockInlinedThroughInvoke() copies the clauses but is
redundant with forwardResume().
Fix this by deduplicating the code.
This problem doesn't lead to any incorrect execution; it's only
untidy.
This change will make fixing PR17872 a little easier.
Added:
llvm/trunk/test/Transforms/Inline/invoke-combine-clauses.ll
Modified:
llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp
llvm/trunk/test/Transforms/Inline/inline_invoke.ll
Modified: llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp?rev=196710&r1=196709&r2=196710&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp Sat Dec 7 18:50:58 2013
@@ -144,7 +144,6 @@ BasicBlock *InvokeInliningInfo::getInner
void InvokeInliningInfo::forwardResume(ResumeInst *RI,
SmallPtrSet<LandingPadInst*, 16> &InlinedLPads) {
BasicBlock *Dest = getInnerResumeDest();
- LandingPadInst *OuterLPad = getLandingPadInst();
BasicBlock *Src = RI->getParent();
BranchInst::Create(Dest, Src);
@@ -155,16 +154,6 @@ void InvokeInliningInfo::forwardResume(R
InnerEHValuesPHI->addIncoming(RI->getOperand(0), Src);
RI->eraseFromParent();
-
- // Append the clauses from the outer landing pad instruction into the inlined
- // landing pad instructions.
- for (SmallPtrSet<LandingPadInst*, 16>::iterator I = InlinedLPads.begin(),
- E = InlinedLPads.end(); I != E; ++I) {
- LandingPadInst *InlinedLPad = *I;
- for (unsigned OuterIdx = 0, OuterNum = OuterLPad->getNumClauses();
- OuterIdx != OuterNum; ++OuterIdx)
- InlinedLPad->addClause(OuterLPad->getClause(OuterIdx));
- }
}
/// HandleCallsInBlockInlinedThroughInvoke - When we inline a basic block into
@@ -174,18 +163,9 @@ void InvokeInliningInfo::forwardResume(R
/// nodes in that block with the values specified in InvokeDestPHIValues.
static void HandleCallsInBlockInlinedThroughInvoke(BasicBlock *BB,
InvokeInliningInfo &Invoke) {
- LandingPadInst *LPI = Invoke.getLandingPadInst();
-
for (BasicBlock::iterator BBI = BB->begin(), E = BB->end(); BBI != E; ) {
Instruction *I = BBI++;
- if (LandingPadInst *L = dyn_cast<LandingPadInst>(I)) {
- unsigned NumClauses = LPI->getNumClauses();
- L->reserveClauses(NumClauses);
- for (unsigned i = 0; i != NumClauses; ++i)
- L->addClause(LPI->getClause(i));
- }
-
// We only need to check for function calls: inlined invoke
// instructions require no special handling.
CallInst *CI = dyn_cast<CallInst>(I);
@@ -248,6 +228,18 @@ static void HandleInlinedInvoke(InvokeIn
if (InvokeInst *II = dyn_cast<InvokeInst>(I->getTerminator()))
InlinedLPads.insert(II->getLandingPadInst());
+ // Append the clauses from the outer landing pad instruction into the inlined
+ // landing pad instructions.
+ LandingPadInst *OuterLPad = Invoke.getLandingPadInst();
+ for (SmallPtrSet<LandingPadInst*, 16>::iterator I = InlinedLPads.begin(),
+ E = InlinedLPads.end(); I != E; ++I) {
+ LandingPadInst *InlinedLPad = *I;
+ unsigned OuterNum = OuterLPad->getNumClauses();
+ InlinedLPad->reserveClauses(OuterNum);
+ for (unsigned OuterIdx = 0; OuterIdx != OuterNum; ++OuterIdx)
+ InlinedLPad->addClause(OuterLPad->getClause(OuterIdx));
+ }
+
for (Function::iterator BB = FirstNewBlock, E = Caller->end(); BB != E; ++BB){
if (InlinedCodeInfo.ContainsCalls)
HandleCallsInBlockInlinedThroughInvoke(BB, Invoke);
Modified: llvm/trunk/test/Transforms/Inline/inline_invoke.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Inline/inline_invoke.ll?rev=196710&r1=196709&r2=196710&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/Inline/inline_invoke.ll (original)
+++ llvm/trunk/test/Transforms/Inline/inline_invoke.ll Sat Dec 7 18:50:58 2013
@@ -96,7 +96,6 @@ eh.resume:
; CHECK: landingpad { i8*, i32 } personality i32 (...)* @__gxx_personality_v0
; CHECK-NEXT: cleanup
; CHECK-NEXT: catch i8* bitcast (i8** @_ZTIi to i8*)
-; CHECK-NEXT: catch i8* bitcast (i8** @_ZTIi to i8*)
; CHECK-NEXT: invoke void @_ZN1AD1Ev(%struct.A* [[A]])
; CHECK-NEXT: to label %[[LBL:[^\s]+]] unwind
; CHECK: [[LBL]]:
@@ -167,7 +166,6 @@ eh.resume:
; CHECK-NEXT: [[LPADVAL1:%.*]] = landingpad { i8*, i32 } personality i32 (...)* @__gxx_personality_v0
; CHECK-NEXT: cleanup
; CHECK-NEXT: catch i8* bitcast (i8** @_ZTIi to i8*)
-; CHECK-NEXT: catch i8* bitcast (i8** @_ZTIi to i8*)
; CHECK-NEXT: invoke void @_ZN1AD1Ev(%struct.A* [[A1]])
; CHECK-NEXT: to label %[[RESUME1:[^\s]+]] unwind
; CHECK: [[RESUME1]]:
@@ -187,7 +185,6 @@ eh.resume:
; CHECK-NEXT: [[LPADVAL2:%.*]] = landingpad { i8*, i32 } personality i32 (...)* @__gxx_personality_v0
; CHECK-NEXT: cleanup
; CHECK-NEXT: catch i8* bitcast (i8** @_ZTIi to i8*)
-; CHECK-NEXT: catch i8* bitcast (i8** @_ZTIi to i8*)
; CHECK-NEXT: invoke void @_ZN1AD1Ev(%struct.A* [[A2]])
; CHECK-NEXT: to label %[[RESUME2:[^\s]+]] unwind
; CHECK: [[RESUME2]]:
@@ -275,7 +272,6 @@ lpad.cont:
; CHECK: landingpad { i8*, i32 } personality i32 (...)* @__gxx_personality_v0
; CHECK-NEXT: cleanup
; CHECK-NEXT: catch i8* bitcast (i8** @_ZTIi to i8*)
-; CHECK-NEXT: catch i8* bitcast (i8** @_ZTIi to i8*)
; CHECK-NEXT: invoke void @_ZN1AD1Ev(
; CHECK-NEXT: to label %[[L:[^\s]+]] unwind
; CHECK: [[L]]:
@@ -322,7 +318,6 @@ terminate:
; CHECK: landingpad { i8*, i32 } personality i32 (...)* @__gxx_personality_v0
; CHECK-NEXT: cleanup
; CHECK-NEXT: catch i8* bitcast (i8** @_ZTIi to i8*)
-; CHECK-NEXT: catch i8* bitcast (i8** @_ZTIi to i8*)
; CHECK-NEXT: invoke void @_ZN1AD1Ev(
; CHECK-NEXT: to label %[[L:[^\s]+]] unwind
; CHECK: [[L]]:
Added: llvm/trunk/test/Transforms/Inline/invoke-combine-clauses.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Inline/invoke-combine-clauses.ll?rev=196710&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/Inline/invoke-combine-clauses.ll (added)
+++ llvm/trunk/test/Transforms/Inline/invoke-combine-clauses.ll Sat Dec 7 18:50:58 2013
@@ -0,0 +1,117 @@
+; RUN: opt %s -inline -S | FileCheck %s
+
+declare void @external_func()
+declare void @abort()
+
+ at exception_inner = external global i8
+ at exception_outer = external global i8
+ at condition = external global i1
+
+
+; Check for a bug in which multiple "resume" instructions in the
+; inlined function caused "catch i8* @exception_outer" to appear
+; multiple times in the resulting landingpad.
+
+define internal void @inner_multiple_resume() {
+ invoke void @external_func()
+ to label %cont unwind label %lpad
+cont:
+ ret void
+lpad:
+ %lp = landingpad i32 personality i8* null
+ catch i8* @exception_inner
+ %cond = load i1* @condition
+ br i1 %cond, label %resume1, label %resume2
+resume1:
+ resume i32 1
+resume2:
+ resume i32 2
+}
+
+define void @outer_multiple_resume() {
+ invoke void @inner_multiple_resume()
+ to label %cont unwind label %lpad
+cont:
+ ret void
+lpad:
+ %lp = landingpad i32 personality i8* null
+ catch i8* @exception_outer
+ resume i32 %lp
+}
+; CHECK: define void @outer_multiple_resume()
+; CHECK: %lp.i = landingpad
+; CHECK-NEXT: catch i8* @exception_inner
+; CHECK-NEXT: catch i8* @exception_outer
+; Check that there isn't another "catch" clause:
+; CHECK-NEXT: load
+
+
+; Check for a bug in which having a "resume" and a "call" in the
+; inlined function caused "catch i8* @exception_outer" to appear
+; multiple times in the resulting landingpad.
+
+define internal void @inner_resume_and_call() {
+ call void @external_func()
+ invoke void @external_func()
+ to label %cont unwind label %lpad
+cont:
+ ret void
+lpad:
+ %lp = landingpad i32 personality i8* null
+ catch i8* @exception_inner
+ resume i32 %lp
+}
+
+define void @outer_resume_and_call() {
+ invoke void @inner_resume_and_call()
+ to label %cont unwind label %lpad
+cont:
+ ret void
+lpad:
+ %lp = landingpad i32 personality i8* null
+ catch i8* @exception_outer
+ resume i32 %lp
+}
+; CHECK: define void @outer_resume_and_call()
+; CHECK: %lp.i = landingpad
+; CHECK-NEXT: catch i8* @exception_inner
+; CHECK-NEXT: catch i8* @exception_outer
+; Check that there isn't another "catch" clause:
+; CHECK-NEXT: br
+
+
+; Check what happens if the inlined function contains an "invoke" but
+; no "resume". In this case, the inlined landingpad does not need to
+; include the "catch i8* @exception_outer" clause from the outer
+; function (since the outer function's landingpad will not be
+; reachable), but it's OK to include this clause.
+
+define internal void @inner_no_resume_or_call() {
+ invoke void @external_func()
+ to label %cont unwind label %lpad
+cont:
+ ret void
+lpad:
+ %lp = landingpad i32 personality i8* null
+ catch i8* @exception_inner
+ ; A landingpad might have no "resume" if a C++ destructor aborts.
+ call void @abort() noreturn nounwind
+ unreachable
+}
+
+define void @outer_no_resume_or_call() {
+ invoke void @inner_no_resume_or_call()
+ to label %cont unwind label %lpad
+cont:
+ ret void
+lpad:
+ %lp = landingpad i32 personality i8* null
+ catch i8* @exception_outer
+ resume i32 %lp
+}
+; CHECK: define void @outer_no_resume_or_call()
+; CHECK: %lp.i = landingpad
+; CHECK-NEXT: catch i8* @exception_inner
+; CHECK-NEXT: catch i8* @exception_outer
+; Check that there isn't another "catch" clause:
+; CHECK-NEXT: call void @abort()
More information about the llvm-commits
mailing list