<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<style type="text/css" style="display:none;"><!-- P {margin-top:0;margin-bottom:0;} --></style>
</head>
<body dir="ltr">
<div id="divtagdefaultwrapper" 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>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="OWAAutoLink" id="LPlnk570829" previewremoved="true">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><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>
</body>
</html>