[llvm] r276364 - GVN-hoist: add missing check for all GEP operands available

[Sebastian Pop via llvm-commits]

Fixed like this.  Ok to commit?

diff --git a/llvm/lib/Transforms/Scalar/GVNHoist.cpp
b/llvm/lib/Transforms/Scalar/GVNHoist.cpp
index c420089..e7179a3 100644
--- a/llvm/lib/Transforms/Scalar/GVNHoist.cpp
+++ b/llvm/lib/Transforms/Scalar/GVNHoist.cpp
@@ -585,9 +585,14 @@ public:
       Gep = dyn_cast<GetElementPtrInst>(St->getPointerOperand());
       Val = dyn_cast<Instruction>(St->getValueOperand());
       // Check that the stored value is available.
-      if (Val && !isa<GetElementPtrInst>(Val) &&
-          !DT->dominates(Val->getParent(), HoistPt))
-        return false;
+      if (Val) {
+        if (isa<GetElementPtrInst>(Val)) {
+          // Check whether we can compute the GEP at HoistPt.
+          if (!allOperandsAvailable(Val, HoistPt))
+            return false;
+        } else if (!DT->dominates(Val->getParent(), HoistPt))
+          return false;
+      }
     }

     // Check whether we can compute the Gep at HoistPt.
@@ -613,9 +618,6 @@ public:

     // Also copy Val when it is a GEP.
     if (Val && isa<GetElementPtrInst>(Val)) {
-      // Check whether we can compute the GEP at HoistPt.
-      if (!allOperandsAvailable(Val, HoistPt))
-        return false;
       Instruction *ClonedVal = Val->clone();
       ClonedVal->insertBefore(HoistPt->getTerminator());
       // Conservatively discard any optimization hints, they may differ on the



On Thu, Jul 21, 2016 at 6:53 PM, David Majnemer
<david.majnemer at gmail.com> wrote:
> This will return false after we've already mutated the IR.  We should return
> false earlier.
>
> On Thu, Jul 21, 2016 at 4:32 PM, Sebastian Pop via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
>>
>> Author: spop
>> Date: Thu Jul 21 18:32:39 2016
>> New Revision: 276364
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=276364&view=rev
>> Log:
>> GVN-hoist: add missing check for all GEP operands available
>>
>> Modified:
>>     llvm/trunk/lib/Transforms/Scalar/GVNHoist.cpp
>>
>> Modified: llvm/trunk/lib/Transforms/Scalar/GVNHoist.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/GVNHoist.cpp?rev=276364&r1=276363&r2=276364&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/Scalar/GVNHoist.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Scalar/GVNHoist.cpp Thu Jul 21 18:32:39 2016
>> @@ -613,6 +613,9 @@ public:
>>
>>      // Also copy Val when it is a GEP.
>>      if (Val && isa<GetElementPtrInst>(Val)) {
>> +      // Check whether we can compute the GEP at HoistPt.
>> +      if (!allOperandsAvailable(Val, HoistPt))
>> +        return false;
>>        Instruction *ClonedVal = Val->clone();
>>        ClonedVal->insertBefore(HoistPt->getTerminator());
>>        // Conservatively discard any optimization hints, they may differ
>> on the
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>