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

[Sebastian Pop via llvm-commits]

Committed as r276368.

On Thu, Jul 21, 2016 at 7:10 PM, David Majnemer
<david.majnemer at gmail.com> wrote:
> LGTM
>
> On Thu, Jul 21, 2016 at 5:02 PM, Sebastian Pop <sebpop at gmail.com> wrote:
>>
>> 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
>> >
>> >
>
>