[llvm] r177680 - Always forward 'resume' instructions to the outter landing pad.

[Bill Wendling]

On Mar 22, 2013, at 1:44 AM, Duncan Sands <baldrick at free.fr> wrote:

> Hi Bill,
> 
> On 22/03/13 00:30, Bill Wendling wrote:
>> Author: void
>> Date: Thu Mar 21 18:30:12 2013
>> New Revision: 177680
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=177680&view=rev
>> Log:
>> Always forward 'resume' instructions to the outter landing pad.
>> 
>> How did this ever work?
>> 
>> Basically, if you have a function that's inlined into the caller, it may not
>> have any 'call' instructions, but any 'resume' instructions it may have should
>> still be forwarded to the outer (caller's) landing pad. This requires that all
>> of the 'landingpad' instructions in the callee have their clauses merged with
>> the caller's outer 'landingpad' instruction (hence the bit of ugly code in the
>> `forwardResume' method).
> 
> does this fix PR14116 too?  There is a comprehensive test case in comment 10.
> 
I have no clue. :) I'll have to check.

>> 
>> Testcase in a follow commit to the test-suite repository.
>> 
>> <rdar://problem/13360379> & PR15555
>> 
>> Modified:
>>     llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp
>> 
>> Modified: llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp?rev=177680&r1=177679&r2=177680&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp Thu Mar 21 18:30:12 2013
>> @@ -152,6 +154,36 @@ void InvokeInliningInfo::forwardResume(R
>> 
>>    InnerEHValuesPHI->addIncoming(RI->getOperand(0), Src);
>>    RI->eraseFromParent();
>> +
>> +  // Get all of the inlined landing pad instructions.
>> +  SmallPtrSet<LandingPadInst*, 16> InlinedLPads;
>> +  Function *Caller = FirstNewBlock->getParent();
>> +  for (Function::iterator I = FirstNewBlock, E = Caller->end(); I != E; ++I)
>> +    if (InvokeInst *II = dyn_cast<InvokeInst>(I->getTerminator())) {
>> +      LandingPadInst *LPI = II->getLandingPadInst();
>> +      if (!LPI->hasCatchAll())
>> +        InlinedLPads.insert(LPI);
> 
> I don't think you should be doing this kind of language specific optimization
> here.  Instcombine should be able to handle this, and it knows about different
> kinds of personality functions and what they are expecting.
> 
Okay.

>> +    }
>> +
>> +  // Merge the catch 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) {
>> +      bool hasClause = false;
>> +      if (OuterLPad->isFilter(OuterIdx)) continue;
> 
> Likewise.  Also, is this actually correct for C++, since the filter may not be
> filtering everything out, only some things.
> 
I don't know what you mean here. I'm skipping the 'filter' clause because it's not relevant to adding catch clauses here...

>> +      Value *OuterClause = OuterLPad->getClause(OuterIdx);
>> +      for (unsigned Idx = 0, N = InlinedLPad->getNumClauses(); Idx != N; ++Idx)
>> +        if (OuterClause == InlinedLPad->getClause(Idx)) {
>> +          hasClause = true;
>> +          break;
>> +        }
>> +      if (!hasClause)
>> +        InlinedLPad->addClause(OuterClause);
>> +    }
>> +  }
>>  }
> 
> I'm kind of confused as to what the above logic is doing.  Why don't you just
> append all of the outer clauses onto the inner landing pad, always, without
> any special thinking?
> 
Because I don't want multiple clauses that's catching the same value. It clutters up the IR and can cause the EH tables to become very large.

-bw