<div dir="ltr">Disabled in r301505.</div><br><div class="gmail_quote"><div dir="ltr">On Wed, Apr 26, 2017 at 5:01 PM Chandler Carruth <<a href="mailto:chandlerc@gmail.com">chandlerc@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Finally got a good test case for this:<div><br></div><div><a href="http://llvm.org/PR32821" target="_blank">http://llvm.org/PR32821</a></div><div><br></div><div>I'm going to flip the flag to "off" as suggested by Geoff, I completely agree with that rather than moving it around.</div></div><br><div class="gmail_quote"><div dir="ltr">On Mon, Apr 24, 2017 at 11:02 AM Aditya K <<a href="mailto:hiraditya@msn.com" target="_blank">hiraditya@msn.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div id="m_1226190981124018431m_-5239805043247944764divtagdefaultwrapper" style="font-size:12pt;color:#000000;font-family:Calibri,Arial,Helvetica,sans-serif" dir="ltr">
<p></p>
<div><br>
</div>
<div>>If any one decides to revert this, I'd like to request that you attempt </div>
<div>>to do so by just turning off GVNHoist by default instead of reverting </div>
<div>>r300200 if possible.  I believe the current location in the pass </div>
<div>>pipeline is the desired one, and reverting r300200 will just move it </div>
<div>>back to being done before inlining, which is likely just hiding bugs.</div>
<div>></div>
<div>>FWIW, before re-enabling this most recently I did correctness testing on </div>
<div>>X86 and AArch64 of the test-suite, SPEC and several other benchmarks, as </div>
<div>>well as bootstrapping clang, all without issue.</div>
<div><br>
</div>
</div></div><div dir="ltr"><div id="m_1226190981124018431m_-5239805043247944764divtagdefaultwrapper" style="font-size:12pt;color:#000000;font-family:Calibri,Arial,Helvetica,sans-serif" dir="ltr"><div>I have also tested the recent GVNHoist and it bootstraps, passes SPEC2006. Maybe it is related to the bug <a href="https://bugs.llvm.org/show_bug.cgi?id=30806" class="m_1226190981124018431m_-5239805043247944764OWAAutoLink" id="m_1226190981124018431m_-5239805043247944764LPlnk570829" target="_blank">https://bugs.llvm.org/show_bug.cgi?id=30806</a></div>
<div>If there is a test case to reproduce the bug I'll be glad to fix it.</div>
<div><br>
</div>
<div><br>
</div>
<div>Thanks,</div>
<div>-Aditya</div></div></div><div dir="ltr"><div id="m_1226190981124018431m_-5239805043247944764divtagdefaultwrapper" style="font-size:12pt;color:#000000;font-family:Calibri,Arial,Helvetica,sans-serif" dir="ltr">
<div><br>
</div>
<div>></div>
<div>>On 4/23/2017 12:45 PM, Philip Reames wrote:</div>
<div>>> If this does turn out to be the problematic commit, I'll ask that we </div>
<div>>> do not re-enable GVNHoist without some llvm-dev discussion of what </div>
<div>>> we've done to test it. I've literally lost track of the number of </div>
<div>>> times this has been enabled and disabled by now.  There's clearly a </div>
<div>>> problem here; it's time we discuss it and what needs done to address </div>
<div>>> it going forward.</div>
<div>>></div>
<div>>> Philip</div>
<div>>></div>
<div>>> On 04/21/2017 08:24 PM, Chandler Carruth via llvm-commits wrote:</div>
<div>>>> FYI and just as a heads up, we're seeing buggy hoisting leading to </div>
<div>>>> null pointers being accessed, and this commit is in the range and </div>
<div>>>> seems very likely to be responsible. We're working on getting this </div>
<div>>>> isolated enough to be sure, but wanted to mention it in case anyone </div>
<div>>>> else is similarly looking at issues after this commit.</div>
<br>
<p></p>
</div></div></blockquote></div></blockquote></div>