[llvm-commits] [llvm] r136313 - /llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp

[Bill Wendling]

On Jul 27, 2011, at 6:23 PM, John McCall wrote:

> On Jul 27, 2011, at 5:38 PM, Bill Wendling wrote:
>> Author: void
>> Date: Wed Jul 27 19:38:23 2011
>> New Revision: 136313
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=136313&view=rev
>> Log:
>> Initial stab at getting inlining working with the EH rewrite.
>> 
>> This takes the new 'resume' instruction and turns it into a direct jump to the
>> caller's landing pad code. The caller's landingpad instruction is merged with
>> the landingpad instructions of the callee. This is a bit rough and makes some
>> assumptions in how the code works. But it passes a simple test.
> 
> +void InvokeInliningInfo::forwardResume(ResumeInst *RI) {
> 
> I would suggest patterning this code more closely after forwardEHResume:
> 1. Make getInnerUnwindDest() recognize a LandingPadInst in the outer
>  landing pad;  if present, set up a InnerEHValues PHI instead of the
>  InnerSelectionPHI / InnerExceptionPHI.
> 2. Teach forwardEHResume how to forward to either an InnerEHValues PHI
>  (with a pair of insertvalues) or a Selection/Exception PHI pair, or just have it
>  assert that it's an S/E PHI pair.
> 3. Teach forwardResume how to forward to either an InnerEHValues PHI
>  or a Selection/Exception PHI pair (with a pair of extractvalues), or just have
>  it assert that it's an EHValues PHI.
> 
Okay. I'll take a look at those methods.

> +/// mergeLandingPadClauses - Visit all of the landing pad instructions merge the
> +/// clauses from the new destination (the caller's landing pad).
> +void InvokeInliningInfo::mergeLandingPadClauses(ResumeInst *RI) {
> +  for (SmallVectorImpl<LandingPadInst*>::iterator
> +         I = CalleeLPads.begin(), E = CalleeLPads.end(); I != E; ++I)
> +    for (unsigned i = 0, e = CallerLPad->getNumClauses(); i != e; ++i)
> +      (*I)->addClause(CallerLPad->getClauseType(i),
> +                      CallerLPad->getClauseValue(i));
> +}
> 
> I don't see the value in queuing up the landingpad instructions here;
> just merge in the caller's clauses as you find them.  That would have
> the added advantage of not adding the clauses twice if you find two
> or more resume instructions in the callee.
> 
> Also, you're not merging the cleanup bit.
> 
Is that the correct semantics? I know I need to keep the cleanup bit on the callee's landingpad instruction, but what about the caller's cleanup bit?

> You might find it useful to assert that personalities and return types match,
> just to catch mistakes in your preflight check.
> 
> It might be a good idea to have LandingPadInst provide a
> reserveClauses(unsigned) method, and you might also want to
> explicitly constrain addClause and getClauseValue to take and
> return a Constant*.
> 

Okay.

-bw