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

[Duncan Sands]

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.

>
> 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.

> +    }
> +
> +  // 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.

> +      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?

>
>   /// HandleCallsInBlockInlinedThroughInvoke - When we inline a basic block into

Ciao, Duncan.