[PATCH] D34478: [BasicAliasAnalysis] Allow idAddofNonZero() for values coming from the same loop iteration.

[Daniel Berlin via llvm-commits]

(you had one in the first patch, but the commit doesnt' have it)


On Mon, Jul 10, 2017 at 1:16 PM, Daniel Berlin <dberlin at dberlin.org> wrote:

> Please add a testcase, too :)
>
> On Mon, Jul 10, 2017 at 1:16 PM, Phabricator via Phabricator <
> reviews at reviews.llvm.org> wrote:
>
>> This revision was automatically updated to reflect the committed changes.
>> Closed by commit rL307581: Avoid doing conservative phi checks in
>> aliasSameBasePointerGEPs() if no phis… (authored by faaleen).
>>
>> Changed prior to commit:
>>   https://reviews.llvm.org/D34478?vs=103471&id=105908#toc
>>
>> Repository:
>>   rL LLVM
>>
>> https://reviews.llvm.org/D34478
>>
>> Files:
>>   llvm/trunk/include/llvm/Analysis/BasicAliasAnalysis.h
>>   llvm/trunk/lib/Analysis/BasicAliasAnalysis.cpp
>>   llvm/trunk/lib/Analysis/ValueTracking.cpp
>>
>>
>> Index: llvm/trunk/lib/Analysis/ValueTracking.cpp
>> ===================================================================
>> --- llvm/trunk/lib/Analysis/ValueTracking.cpp
>> +++ llvm/trunk/lib/Analysis/ValueTracking.cpp
>> @@ -1873,7 +1873,7 @@
>>        if (Known.countMaxLeadingZeros() < BitWidth - ShiftVal)
>>          return true;
>>        // Are all the bits to be shifted out known zero?
>> -      if (Known.countMinTrailingZeros() >= ShiftVal)
>> +      if (Known.isUnknown() || Known.countMinTrailingZeros() >= ShiftVal)
>>          return isKnownNonZero(X, Depth, Q);
>>      }
>>    }
>> Index: llvm/trunk/lib/Analysis/BasicAliasAnalysis.cpp
>> ===================================================================
>> --- llvm/trunk/lib/Analysis/BasicAliasAnalysis.cpp
>> +++ llvm/trunk/lib/Analysis/BasicAliasAnalysis.cpp
>> @@ -922,11 +922,11 @@
>>
>>  /// Provide ad-hoc rules to disambiguate accesses through two GEP
>> operators,
>>  /// both having the exact same pointer operand.
>> -static AliasResult aliasSameBasePointerGEPs(const GEPOperator *GEP1,
>> -                                            uint64_t V1Size,
>> -                                            const GEPOperator *GEP2,
>> -                                            uint64_t V2Size,
>> -                                            const DataLayout &DL) {
>> +AliasResult BasicAAResult::aliasSameBasePointerGEPs(const GEPOperator
>> *GEP1,
>> +                                                    uint64_t V1Size,
>> +                                                    const GEPOperator
>> *GEP2,
>> +                                                    uint64_t V2Size,
>> +                                                    const DataLayout
>> &DL) {
>>
>>    assert(GEP1->getPointerOperand()->stripPointerCastsAndBarriers() ==
>>               GEP2->getPointerOperand()->stripPointerCastsAndBarriers()
>> &&
>> @@ -1006,19 +1006,23 @@
>>      // Because they cannot partially overlap and because fields in an
>> array
>>      // cannot overlap, if we can prove the final indices are different
>> between
>>      // GEP1 and GEP2, we can conclude GEP1 and GEP2 don't alias.
>> -
>> +
>>      // If the last indices are constants, we've already checked they
>> don't
>>      // equal each other so we can exit early.
>>      if (C1 && C2)
>>        return NoAlias;
>>      {
>>        Value *GEP1LastIdx = GEP1->getOperand(GEP1->getNumOperands() - 1);
>>        Value *GEP2LastIdx = GEP2->getOperand(GEP2->getNumOperands() - 1);
>> -      if (isa<PHINode>(GEP1LastIdx) || isa<PHINode>(GEP2LastIdx)) {
>> +      if ((isa<PHINode>(GEP1LastIdx) || isa<PHINode>(GEP2LastIdx)) &&
>> +          !VisitedPhiBBs.empty()) {
>>          // If one of the indices is a PHI node, be safe and only use
>>          // computeKnownBits so we don't make any assumptions about the
>>          // relationships between the two indices. This is important if
>> we're
>>          // asking about values from different loop iterations. See
>> PR32314.
>> +        // But, with empty visitedPhiBBs we can guarantee that the
>> values are
>> +        // from the same iteration. Therefore, we can avoid doing this
>> +        // conservative check.
>>          // TODO: We may be able to change the check so we only do this
>> when
>>          // we definitely looked through a PHINode.
>>          if (GEP1LastIdx != GEP2LastIdx &&
>> Index: llvm/trunk/include/llvm/Analysis/BasicAliasAnalysis.h
>> ===================================================================
>> --- llvm/trunk/include/llvm/Analysis/BasicAliasAnalysis.h
>> +++ llvm/trunk/include/llvm/Analysis/BasicAliasAnalysis.h
>> @@ -183,6 +183,12 @@
>>                         uint64_t V2Size, const AAMDNodes &V2AAInfo,
>>                         const Value *UnderlyingV1, const Value
>> *UnderlyingV2);
>>
>> +  AliasResult aliasSameBasePointerGEPs(const GEPOperator *GEP1,
>> +                                       uint64_t V1Size,
>> +                                       const GEPOperator *GEP2,
>> +                                       uint64_t V2Size,
>> +                                       const DataLayout &DL);
>> +
>>    AliasResult aliasPHI(const PHINode *PN, uint64_t PNSize,
>>                         const AAMDNodes &PNAAInfo, const Value *V2,
>>                         uint64_t V2Size, const AAMDNodes &V2AAInfo,
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170710/19c0afc6/attachment.html>