[llvm-commits] patch: make the inliner emit lifetime markers

[Chris Lattner]

On May 21, 2011, at 8:30 PM, Nick Lewycky wrote:

> On 21 May 2011 11:02, Chris Lattner <clattner at apple.com> wrote:
> 
> On May 18, 2011, at 5:03 PM, Nick Lewycky wrote:
> 
>> Hi, I'm starting to get serious about lifetime markers.
>> 
>> The attached patch teaches the inliner to emit lifetime.start and lifetime.end markers for the alloca's that it hoists to the beginning of the function. Please review, or propose some testing that you'd like to see done on this patch!
> 
> The algorithm looks right to me.  Please make a couple of changes:
> 
> 1. Please use IRBuilder.  It would make sense to add a "getIntrinsic(Intrinsic::lifetime_start);" method to IRBuilder.
> 2. instead of "x  != Type::getInt8PtrTy(Context)".  IRBuilder shoudl handle this check for you.
> 3. Minor thing:
> 
> +      Value *Args[2];
> +      Args[0] = MinusOne;
> +      Args[1] = AI;
> 
> Can be:
> 
> +      Value *Args[2] = { MinusOne, AI };
> 
> Thanks for the review! Updated patch attached. I'm just finishing up a bootstrap build with this patch applied (Debug+Asserts for stage1 and stage2). I haven't looked for any performance impact yet.
>  
> It would be good to synch up with Owen on this, I think he's working on similar things.
> 
> Thanks! I've already chatted with Owen over IRC.

Great!

In FindLifetimeMarkers, why push entries into a vector then iterate over the vector?  You can just pull your "for each use" inner loop out to its own predicate function and eliminate the smallvector.  Also, there is no need for both the start and end intrinsic: if you find either, you can assume that there is no need to add one for an alloca.  Here's an example where you'd have a start but no end (assuming a frontend was generating them):

void foo() { 
  int x;
  bar(&x);
  abort();
}

As such, instead of "finding" them both, please rename the predicate to "hasLifetimeMarkers()".

-Chris
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110521/216b333d/attachment.html>