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

[Daniel Berlin via llvm-commits]

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/d83c9a20/attachment.html>