<div dir="ltr">I was about to tell you what you just discovered :)<div><br></div><div>THe real issue here is that we are inconsistent in what the return values of these functions mean.</div><div>processNonLocalLoad is not returning whether it changed, but whether it deleted the load.</div><div>processLoad in turn is also returning whether it deleted the load.</div><div><br></div><div>However, processInstruction then uses this to determine if the function has changed ...</div><div><br></div><div>If you want to make these functions return whether anything changed, i'm not opposed, but you have to carefully go through the logic separate the two things it uses the return values for ;)</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jan 12, 2016 at 11:24 AM, Wenxiang Qiu via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">vincentqiuuu abandoned this revision.<br>
vincentqiuuu added a comment.<br>
<br>
After studying the test case that failed and GVN.cpp, I found that int the function processNonLocalLoad(LoadInst *LI), not taking the return value of performScalarPRE(I) into account is actually correct. It's not a bug because performScalarPRE(I) is just a step of analyzing when the first operand of the LoadInst is a GEP, which means the LoadInst follows a GEP. performScalarPRE(I) can return true (the GEP is thus eliminated) whereas the LoadInst is not eliminated and thus processNonLocalLoad(LoadInst *LI) should return false.<br>
<div class="HOEnZb"><div class="h5"><br>
<br>
<a href="http://reviews.llvm.org/D14899" rel="noreferrer" target="_blank">http://reviews.llvm.org/D14899</a><br>
<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div>