<div dir="ltr"><br><div class="gmail_extra"><div class="gmail_quote">On Mon, Oct 7, 2013 at 10:28 PM, Evan Cheng <span dir="ltr"><<a href="mailto:evan.cheng@apple.com" target="_blank">evan.cheng@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><div><div class="h5"><div>On Oct 2, 2013, at 11:20 AM, Alexey Samsonov <<a href="mailto:samsonov@google.com" target="_blank">samsonov@google.com</a>> wrote:</div>
<br><blockquote type="cite"><div dir="ltr"><br><div class="gmail_extra"><div class="gmail_quote">On Wed, Oct 2, 2013 at 9:52 PM, Bob Wilson <span dir="ltr"><<a href="mailto:bob.wilson@apple.com" target="_blank">bob.wilson@apple.com</a>></span> wrote:<br>
<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"><div style="word-wrap:break-word"><div><br><div>
<div>On Oct 2, 2013, at 10:08 AM, Bob Wilson <<a href="mailto:bob.wilson@apple.com" target="_blank">bob.wilson@apple.com</a>> wrote:</div><br><blockquote type="cite"><div style="word-wrap:break-word"><br><div><div>
On Oct 2, 2013, at 8:33 AM, Alexey Samsonov <<a href="mailto:samsonov@google.com" target="_blank">samsonov@google.com</a>> wrote:</div>
<br><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Oct 1, 2013 at 10:39 PM, Bob Wilson <span dir="ltr"><<a href="mailto:bob.wilson@apple.com" target="_blank">bob.wilson@apple.com</a>></span> wrote:<br>
<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"><div><br>
On Oct 1, 2013, at 10:20 AM, Nick Lewycky <<a href="mailto:nlewycky@gmail.com" target="_blank">nlewycky@gmail.com</a>> wrote:<br>
<br>
><br>
> The optimization was "important" for some SPEC test, but I think we're past caring about that.<br>
<br>
</div>Not necessarily. Has anyone measured the impact on SPEC and other benchmarks?<br></blockquote><div><br></div><div>I'm going to submit this anyway.</div></div></div></div></blockquote><div><br></div><div>That's not very friendly. You may not care about SPEC but that doesn't mean that no one else cares.</div>
<br><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"> See Reid's comment in <a href="http://llvm-reviews.chandlerc.com/D1754" target="_blank">http://llvm-reviews.chandlerc.com/D1754</a> - looks like this optimization has incorrect assumptions about C programs.</div>
</div></div></blockquote><div><br></div>The only comment I see from Reid there is this: "I missed any earlier discussion about this. Don't we want to keep this optimization, even if it's a real corner case that fires once in a blue moon? isLeakCheckerRoot() doesn't check any flags to see if any of the sanitizers are on."</div>
</div></blockquote><br></div></div><div>OK, I see the problem, and I can't think of a way to fix it.</div></div></blockquote><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">
<div style="word-wrap:break-word"><div><br></div><div>But, if this hurts SPEC results, we may need to investigate alternative ways to get the same results. Since you're the one pushing this, it would have been nice if you contributed to that effort by at least providing the performance data. You could still do that now. Please do.</div>
</div></blockquote></div><br><span style="font-family:arial,sans-serif;font-size:13px">Yes, the comment didn't appear in Phabricator, only the review thread:</span></div><div class="gmail_extra"><span style="font-family:arial,sans-serif;font-size:13px"><br>
</span></div><div class="gmail_extra"><span style="font-family:arial,sans-serif;font-size:13px">"The C++ standard sez [basic.start.main]p6: "The function main shall not be used (3.2) within a program." The C standard has no such language, so this optimization isn't really valid for C. IIRC this caused PNaCl some problems."</span></div>
<div class="gmail_extra"><span style="font-family:arial,sans-serif;font-size:13px"><br></span></div><div class="gmail_extra"><font face="arial, sans-serif">I will run the SPEC benchmarks and measure the effects of this change. Sorry for not doing this as a first step.</font></div>
</div></blockquote><div><br></div></div></div>Were you able to do this? We need to know the full impact on the LLVM test suite. Ideally on more than just x86.</div><div><br></div><div>Any change that has the potential to impact performance widely requires this investigation *before* the patch is committed.</div>
</div></blockquote><div><br></div><div>I've reverted the change in r192121 till I'm able to do it. Once again, sorry for the premature commit.</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"><span class="HOEnZb"><font color="#888888"><div><br></div><div>Evan</div></font></span><div><span class="HOEnZb"><font color="#888888"><br></font></span><blockquote type="cite"><span class="HOEnZb"><font color="#888888"><div dir="ltr">
<div class="gmail_extra"><div><br></div>-- <br><div>Alexey Samsonov, MSK</div>
</div></div></font></span><div class="im">
_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</div></blockquote></div><br></div>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div>Alexey Samsonov, MSK</div>
</div></div>