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

[Nick Lewycky]

On 21 May 2011 20:59, Chris Lattner <clattner at apple.com> wrote:

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

Good catch!

 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()".
>

I'm convinced. Updated patch attached!

Nick
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110521/ef3c9230/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: llvm-inliner-lifetime-3.patch
Type: text/x-patch
Size: 5815 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110521/ef3c9230/attachment.bin>