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

[Bill Wendling]

On Mar 22, 2013, at 12:29 PM, Duncan Sands <baldrick at free.fr> wrote:

>>>> +  // 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...
> 
> But you have to add filters too, otherwise it may not be correct!  The entire
> list of things the exception is compared against has to be appended, i.e. all
> catch clauses but also all filters.  Of course you can often then simplify
> things, but the logical operation is appending everything from the outer
> landing pad onto the inner one.  If you look at the instcombine logic it knows
> how to simplify filters, and is careful to do it correctly; you can't always
> just throw them away.
> 
>> 
>>>> +      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.
> 
> Instcombine will clean it all up...  It knows all this and more.  I think that
> doing some simplification in the inliner would be advantageous if it allowed you
> to avoid inlining some basic blocks.  But I don't think it ever does: it is
> basic blocks in the function being inlined into that can become unreachable, and
> I don't think you have a good way of detecting and dropping those in the
> inliner, but I could be wrong.  Is instcombine failing to clean stuff up?
> 

Okay. I'll concat the clauses. Thanks!

-bw