<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On May 21, 2011, at 8:30 PM, Nick Lewycky wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">On 21 May 2011 11:02, Chris Lattner <span dir="ltr"><<a href="mailto:clattner@apple.com">clattner@apple.com</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div style="word-wrap:break-word"><div class="im"><br><div><div>On May 18, 2011, at 5:03 PM, Nick Lewycky wrote:</div><br><blockquote type="cite">Hi, I'm starting to get serious about lifetime markers.<div><br></div>
<div>
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!</div>
</blockquote><br></div></div><div>The algorithm looks right to me. Please make a couple of changes:</div><div><br></div><div>1. Please use IRBuilder. It would make sense to add a "getIntrinsic(Intrinsic::lifetime_start);" method to IRBuilder.</div>
<div>2. instead of "x <span style="font-family:Menlo;font-size:11px"> != Type::getInt8PtrTy(Context)". IRBuilder shoudl handle this check for you.</span></div><div><font face="Menlo" size="3"><span style="font-size:11px">3. Minor thing:</span></font></div>
<div><font face="Menlo" size="3"><span style="font-size:11px"><br></span></font></div><div><font face="Menlo" size="3"><span style="font-size:11px"><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">
+ Value *Args[2];</div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">+ Args[0] = MinusOne;</div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">+ Args[1] = AI;</div>
<div><br></div><div>Can be:</div><div><br></div><div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">+ Value *Args[2] = { MinusOne, AI };</div></div></span></font></div></div></blockquote>
<div><br></div><div>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.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"><div style="word-wrap:break-word"><div><font face="Menlo" size="3"><span style="font-size:11px"><div><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px">
<span class="Apple-style-span" style="font-family: arial; font-size: small; ">It would be good to synch up with Owen on this, I think he's working on similar things.</span></div></div></span></font></div></div></blockquote>
<div><br></div><div>Thanks! I've already chatted with Owen over IRC.</div><div></div></div></blockquote></div><br><div>Great!</div><div><br></div><div>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):</div><div><br></div><div>void foo() { </div><div> int x;</div><div> bar(&x);</div><div> abort();</div><div>}</div><div><br></div><div>As such, instead of "finding" them both, please rename the predicate to "hasLifetimeMarkers()".</div><div><br></div><div>-Chris</div></body></html>