<html><head><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><div>On Jun 19, 2013, at 1:40 PM, Reid Kleckner <<a href="mailto:rnk@google.com">rnk@google.com</a>> wrote:</div><blockquote type="cite"><div dir="ltr"><div style="">On Wed, Jun 19, 2013 at 3:01 PM, John McCall <span dir="ltr"><<a href="mailto:rjmccall@apple.com" target="_blank">rjmccall@apple.com</a>></span> wrote:<br></div><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex; position: static; z-index: auto; "><div style="word-wrap:break-word"><div>If the cleanup doesn't actually get popped off the cleanup stack, it will still be around for later code in the full-expression.</div>
<div><br></div><div>Here, I think this test case should work:</div><div><br></div><div> struct A { A(); ~A(); };</div><div> int foo(A a, const A &a);</div><div> int bar();</div><div> ...</div><div> int baz(bool cond) {</div>
<div> return (cond ? foo(A(), A()) : bar());</div><div> }</div><div><br></div><div>Also, you should pop your cleanups in reverse order to make it more likely that they actually pop off.</div></div></blockquote><div>
<br></div><div style="">Now that I deactivate in reverse order, the cleanups are getting popped, and everything is much cleaner. I had to add more tests to cover the case where I actually deactivate a cleanup in a conditional.</div></div></div></div></blockquote><div><br></div>Not just in a conditional, but specifically in a conditional when you can't pop the cleanups because another (unpoppable) cleanup has been added. And if you want to be really good, make sure that a landing pad for that cleanup has been created by introducing a call after it's pushed but before the outer call tries to deactivate cleanups.</div><div><br><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
<div style="">I think the placeholder DominatingIP is OK as is because of this code in SetupCleanupBlockActivation:</div><div style=""><br></div><div style=""><div> // If we're in a conditional block, ignore the dominating IP and</div>
<div> // use the outermost conditional branch.</div><div> if (CGF.isInConditionalBranch()) {</div><div> CGF.setBeforeOutermostConditional(value, var);</div><div> } else {</div><div> new llvm::StoreInst(value, var, dominatingIP);</div>
<div> }</div><div><br></div><div style="">Does that make sense? Basically, the conditional case is already handled for me. The cleanup is created in a deactivated state, activated when I push it, and then deactivated before the call.</div></div></div></div></div></blockquote><div><br></div>Ah, yes, you're right. So the placeholders are unnecessary in this case but do no harm.</div><div><br></div><div><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div style="">
<div style="">Anyway, this has tests now. I wasn't sure how carefully I should match the labels from the landing pads in the lit tests, but they look correct from inspection.</div></div></div></div></div></blockquote><div><br></div>Yeah, unfortunately matching control flow in LLVM IR with FileCheck is very challenging.</div><div><br></div><div>John.</div></body></html>