<div dir="ltr">LGTM</div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Sep 19, 2016 at 3:37 PM, Sebastian Pop <span dir="ltr"><<a href="mailto:sebpop@gmail.com" target="_blank">sebpop@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">sebpop updated this revision to Diff 71873.<br>
sebpop added a comment.<br>
<br>
Updated patch to only filter out TerminatorInst.<br>
As I was mentioning, the other instructions are rare, and we would spend more time in isa<>() than allowing these instructions to be discarded after computing a VN.<br>
<span class=""><br>
<br>
<a href="https://reviews.llvm.org/D23929" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D23929</a><br>
<br>
Files:<br>
  llvm/lib/Transforms/Scalar/<wbr>GVNHoist.cpp<br>
<br>
Index: llvm/lib/Transforms/Scalar/<wbr>GVNHoist.cpp<br>
==============================<wbr>==============================<wbr>=======<br>
--- llvm/lib/Transforms/Scalar/<wbr>GVNHoist.cpp<br>
+++ llvm/lib/Transforms/Scalar/<wbr>GVNHoist.cpp<br>
</span>@@ -913,7 +913,8 @@<br>
             break;<br>
<br>
           CI.insert(Call, VN);<br>
-        } else if (HoistingGeps || !isa<GetElementPtrInst>(&I1))<br>
+        } else if (!isa<TerminatorInst>(&I1) &&<br>
+                   (HoistingGeps || !isa<GetElementPtrInst>(&I1)))<br>
           // Do not hoist scalars past calls that may write to memory because<br>
<div class="HOEnZb"><div class="h5">           // that could result in spills later. geps are handled separately.<br>
           // TODO: We can relax this for targets like AArch64 as they have more<br>
<br>
<br>
</div></div></blockquote></div><br></div>