<div dir="ltr">LGTM</div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jul 21, 2016 at 5:02 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">Fixed like this.  Ok to commit?<br>
<br>
diff --git a/llvm/lib/Transforms/Scalar/GVNHoist.cpp<br>
b/llvm/lib/Transforms/Scalar/GVNHoist.cpp<br>
index c420089..e7179a3 100644<br>
--- a/llvm/lib/Transforms/Scalar/GVNHoist.cpp<br>
+++ b/llvm/lib/Transforms/Scalar/GVNHoist.cpp<br>
@@ -585,9 +585,14 @@ public:<br>
       Gep = dyn_cast<GetElementPtrInst>(St->getPointerOperand());<br>
       Val = dyn_cast<Instruction>(St->getValueOperand());<br>
       // Check that the stored value is available.<br>
-      if (Val && !isa<GetElementPtrInst>(Val) &&<br>
-          !DT->dominates(Val->getParent(), HoistPt))<br>
-        return false;<br>
+      if (Val) {<br>
+        if (isa<GetElementPtrInst>(Val)) {<br>
<span class="">+          // Check whether we can compute the GEP at HoistPt.<br>
+          if (!allOperandsAvailable(Val, HoistPt))<br>
+            return false;<br>
</span>+        } else if (!DT->dominates(Val->getParent(), HoistPt))<br>
+          return false;<br>
+      }<br>
     }<br>
<br>
     // Check whether we can compute the Gep at HoistPt.<br>
@@ -613,9 +618,6 @@ public:<br>
<span class=""><br>
     // Also copy Val when it is a GEP.<br>
     if (Val && isa<GetElementPtrInst>(Val)) {<br>
</span>-      // Check whether we can compute the GEP at HoistPt.<br>
-      if (!allOperandsAvailable(Val, HoistPt))<br>
-        return false;<br>
<span class="im HOEnZb">       Instruction *ClonedVal = Val->clone();<br>
       ClonedVal->insertBefore(HoistPt->getTerminator());<br>
       // Conservatively discard any optimization hints, they may differ on the<br>
<br>
<br>
<br>
</span><div class="HOEnZb"><div class="h5">On Thu, Jul 21, 2016 at 6:53 PM, David Majnemer<br>
<<a href="mailto:david.majnemer@gmail.com">david.majnemer@gmail.com</a>> wrote:<br>
> This will return false after we've already mutated the IR.  We should return<br>
> false earlier.<br>
><br>
> On Thu, Jul 21, 2016 at 4:32 PM, Sebastian Pop via llvm-commits<br>
> <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br>
>><br>
>> Author: spop<br>
>> Date: Thu Jul 21 18:32:39 2016<br>
>> New Revision: 276364<br>
>><br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=276364&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=276364&view=rev</a><br>
>> Log:<br>
>> GVN-hoist: add missing check for all GEP operands available<br>
>><br>
>> Modified:<br>
>>     llvm/trunk/lib/Transforms/Scalar/GVNHoist.cpp<br>
>><br>
>> Modified: llvm/trunk/lib/Transforms/Scalar/GVNHoist.cpp<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/GVNHoist.cpp?rev=276364&r1=276363&r2=276364&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/GVNHoist.cpp?rev=276364&r1=276363&r2=276364&view=diff</a><br>
>><br>
>> ==============================================================================<br>
>> --- llvm/trunk/lib/Transforms/Scalar/GVNHoist.cpp (original)<br>
>> +++ llvm/trunk/lib/Transforms/Scalar/GVNHoist.cpp Thu Jul 21 18:32:39 2016<br>
>> @@ -613,6 +613,9 @@ public:<br>
>><br>
>>      // Also copy Val when it is a GEP.<br>
>>      if (Val && isa<GetElementPtrInst>(Val)) {<br>
>> +      // Check whether we can compute the GEP at HoistPt.<br>
>> +      if (!allOperandsAvailable(Val, HoistPt))<br>
>> +        return false;<br>
>>        Instruction *ClonedVal = Val->clone();<br>
>>        ClonedVal->insertBefore(HoistPt->getTerminator());<br>
>>        // Conservatively discard any optimization hints, they may differ<br>
>> on the<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>
><br>
><br>
</div></div></blockquote></div><br></div>