Commit 2840a1c0 authored by Tom Stellard's avatar Tom Stellard
Browse files

Merging r198290:

------------------------------------------------------------------------
r198290 | aschwaighofer | 2014-01-01 22:31:36 -0500 (Wed, 01 Jan 2014) | 23 lines

BasicAA: Fix value equality and phi cycles

When there are cycles in the value graph we have to be careful interpreting
"Value*" identity as "value" equivalence. We interpret the value of a phi node
as the value of its operands.
When we check for value equivalence now we make sure that the "Value*" dominates
all cycles (phis).

%0 = phi [%noaliasval, %addr2]
%l = load %ptr
%addr1 = gep @a, 0, %l
%addr2 = gep @a, 0, (%l + 1)
store %ptr ...

Before this patch we would return NoAlias for (%0, %addr1) which is wrong
because the value of the load is from different iterations of the loop.

Tested on x86_64 -mavx at O3 and O3 -flto with no performance or compile time
regressions.

PR18068
radar://15653794

------------------------------------------------------------------------

llvm-svn: 206051
parent 16004dc2
Loading
Loading
Loading
Loading
+112 −37
Original line number Diff line number Diff line
@@ -18,6 +18,7 @@
#include "llvm/ADT/SmallVector.h"
#include "llvm/Analysis/AliasAnalysis.h"
#include "llvm/Analysis/CaptureTracking.h"
#include "llvm/Analysis/Dominators.h"
#include "llvm/Analysis/InstructionSimplify.h"
#include "llvm/Analysis/MemoryBuiltins.h"
#include "llvm/Analysis/ValueTracking.h"
@@ -38,6 +39,12 @@
#include <algorithm>
using namespace llvm;

/// Cutoff after which to stop analysing a set of phi nodes potentially involved
/// in a cycle. Because we are analysing 'through' phi nodes we need to be
/// careful with value equivalence. We use dominance to make sure a value cannot
/// be involved in a cycle.
const unsigned MaxNumPhiBBsValueDominanceCheck = 20;

//===----------------------------------------------------------------------===//
// Useful predicates
//===----------------------------------------------------------------------===//
@@ -403,42 +410,6 @@ DecomposeGEPExpression(const Value *V, int64_t &BaseOffs,
  return V;
}

/// GetIndexDifference - Dest and Src are the variable indices from two
/// decomposed GetElementPtr instructions GEP1 and GEP2 which have common base
/// pointers.  Subtract the GEP2 indices from GEP1 to find the symbolic
/// difference between the two pointers.
static void GetIndexDifference(SmallVectorImpl<VariableGEPIndex> &Dest,
                               const SmallVectorImpl<VariableGEPIndex> &Src) {
  if (Src.empty()) return;

  for (unsigned i = 0, e = Src.size(); i != e; ++i) {
    const Value *V = Src[i].V;
    ExtensionKind Extension = Src[i].Extension;
    int64_t Scale = Src[i].Scale;

    // Find V in Dest.  This is N^2, but pointer indices almost never have more
    // than a few variable indexes.
    for (unsigned j = 0, e = Dest.size(); j != e; ++j) {
      if (Dest[j].V != V || Dest[j].Extension != Extension) continue;

      // If we found it, subtract off Scale V's from the entry in Dest.  If it
      // goes to zero, remove the entry.
      if (Dest[j].Scale != Scale)
        Dest[j].Scale -= Scale;
      else
        Dest.erase(Dest.begin()+j);
      Scale = 0;
      break;
    }

    // If we didn't consume this entry, add it to the end of the Dest list.
    if (Scale) {
      VariableGEPIndex Entry = { V, Extension, -Scale };
      Dest.push_back(Entry);
    }
  }
}

//===----------------------------------------------------------------------===//
// BasicAliasAnalysis Pass
//===----------------------------------------------------------------------===//
@@ -482,6 +453,7 @@ namespace {

    virtual AliasResult alias(const Location &LocA,
                              const Location &LocB) {
      DT = 0;
      assert(AliasCache.empty() && "AliasCache must be cleared after use!");
      assert(notDifferentParent(LocA.Ptr, LocB.Ptr) &&
             "BasicAliasAnalysis doesn't support interprocedural queries.");
@@ -492,6 +464,7 @@ namespace {
      // SmallDenseMap if it ever grows larger.
      // FIXME: This should really be shrink_to_inline_capacity_and_clear().
      AliasCache.shrink_and_clear();
      VisitedPhiBBs.clear();
      return Alias;
    }

@@ -532,9 +505,41 @@ namespace {
    typedef SmallDenseMap<LocPair, AliasResult, 8> AliasCacheTy;
    AliasCacheTy AliasCache;

    /// \brief Track phi nodes we have visited. When interpret "Value" pointer
    /// equality as value equality we need to make sure that the "Value" is not
    /// part of a cycle. Otherwise, two uses could come from different
    /// "iterations" of a cycle and see different values for the same "Value"
    /// pointer.
    /// The following example shows the problem:
    ///   %p = phi(%alloca1, %addr2)
    ///   %l = load %ptr
    ///   %addr1 = gep, %alloca2, 0, %l
    ///   %addr2 = gep  %alloca2, 0, (%l + 1)
    ///      alias(%p, %addr1) -> MayAlias !
    ///   store %l, ...
    SmallPtrSet<const BasicBlock*, 8> VisitedPhiBBs;

    // Visited - Track instructions visited by pointsToConstantMemory.
    SmallPtrSet<const Value*, 16> Visited;

    // We use the dominator tree to check values can't be part of a cycle.
    DominatorTree *DT;

    /// \brief Check whether two Values can be considered equivalent.
    ///
    /// In addition to pointer equivalence of \p V1 and \p V2 this checks
    /// whether they can not be part of a cycle in the value graph by looking at
    /// all visited phi nodes an making sure that the value dominates all of
    /// them.
    bool isValueEqual(const Value *V1, const Value *V2);

    /// \brief Dest and Src are the variable indices from two decomposed
    /// GetElementPtr instructions GEP1 and GEP2 which have common base
    /// pointers.  Subtract the GEP2 indices from GEP1 to find the symbolic
    /// difference between the two pointers.
    void GetIndexDifference(SmallVectorImpl<VariableGEPIndex> &Dest,
                            const SmallVectorImpl<VariableGEPIndex> &Src);

    // aliasGEP - Provide a bunch of ad-hoc rules to disambiguate a GEP
    // instruction against another.
    AliasResult aliasGEP(const GEPOperator *V1, uint64_t V1Size,
@@ -1094,6 +1099,10 @@ BasicAliasAnalysis::aliasPHI(const PHINode *PN, uint64_t PNSize,
                             const MDNode *PNTBAAInfo,
                             const Value *V2, uint64_t V2Size,
                             const MDNode *V2TBAAInfo) {
  // Track phi nodes we have visited. We use this information when we determine
  // value equivalence.
  VisitedPhiBBs.insert(PN->getParent());

  // If the values are PHIs in the same block, we can do a more precise
  // as well as efficient check: just check for aliases between the values
  // on corresponding edges.
@@ -1187,7 +1196,7 @@ BasicAliasAnalysis::aliasCheck(const Value *V1, uint64_t V1Size,
  V2 = V2->stripPointerCasts();

  // Are we checking for alias of the same value?
  if (V1 == V2) return MustAlias;
  if (isValueEqual(V1, V2)) return MustAlias;

  if (!V1->getType()->isPointerTy() || !V2->getType()->isPointerTy())
    return NoAlias;  // Scalars cannot alias each other
@@ -1307,3 +1316,69 @@ BasicAliasAnalysis::aliasCheck(const Value *V1, uint64_t V1Size,
                         Location(V2, V2Size, V2TBAAInfo));
  return AliasCache[Locs] = Result;
}

bool BasicAliasAnalysis::isValueEqual(const Value *V, const Value *V2) {
  if (V != V2)
    return false;

  const Instruction *Inst = dyn_cast<Instruction>(V);
  if (!Inst)
    return true;

  // Use the dominance if available.
  DT = getAnalysisIfAvailable<DominatorTree>();
  if (DT) {
    if (VisitedPhiBBs.size() > MaxNumPhiBBsValueDominanceCheck)
      return false;

    // Make sure that the visited phis are dominated by the Value.
    for (SmallPtrSet<const BasicBlock *, 8>::iterator
             PI = VisitedPhiBBs.begin(),
             PE = VisitedPhiBBs.end();
         PI != PE; ++PI)
      if (!DT->dominates(Inst, *PI))
        return false;
    return true;
  }

  return false;
}

/// GetIndexDifference - Dest and Src are the variable indices from two
/// decomposed GetElementPtr instructions GEP1 and GEP2 which have common base
/// pointers.  Subtract the GEP2 indices from GEP1 to find the symbolic
/// difference between the two pointers.
void BasicAliasAnalysis::GetIndexDifference(
    SmallVectorImpl<VariableGEPIndex> &Dest,
    const SmallVectorImpl<VariableGEPIndex> &Src) {
  if (Src.empty())
    return;

  for (unsigned i = 0, e = Src.size(); i != e; ++i) {
    const Value *V = Src[i].V;
    ExtensionKind Extension = Src[i].Extension;
    int64_t Scale = Src[i].Scale;

    // Find V in Dest.  This is N^2, but pointer indices almost never have more
    // than a few variable indexes.
    for (unsigned j = 0, e = Dest.size(); j != e; ++j) {
      if (!isValueEqual(Dest[j].V, V) || Dest[j].Extension != Extension)
        continue;

      // If we found it, subtract off Scale V's from the entry in Dest.  If it
      // goes to zero, remove the entry.
      if (Dest[j].Scale != Scale)
        Dest[j].Scale -= Scale;
      else
        Dest.erase(Dest.begin() + j);
      Scale = 0;
      break;
    }

    // If we didn't consume this entry, add it to the end of the Dest list.
    if (Scale) {
      VariableGEPIndex Entry = { V, Extension, -Scale };
      Dest.push_back(Entry);
    }
  }
}
+1 −1
Original line number Diff line number Diff line
; RUN: opt < %s -basicaa -aa-eval -disable-output 2>&1 | FileCheck %s
; RUN: opt < %s -domtree -basicaa -aa-eval -disable-output 2>&1 | FileCheck %s
; TEST that A[1][0] may alias A[0][i].
target datalayout = "E-p:64:64:64-a0:0:8-f32:32:32-f64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-v64:64:64-v128:128:128"

+54 −0
Original line number Diff line number Diff line
; RUN: opt < %s -basicaa -aa-eval -print-all-alias-modref-info -disable-output 2>&1 | FileCheck %s
; RUN: opt < %s -domtree -basicaa -aa-eval -print-all-alias-modref-info -disable-output 2>&1 | FileCheck %s --check-prefix=DOM
target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

; rdar://7282591

@X = common global i32 0
@Y = common global i32 0
@Z = common global i32 0

; CHECK-LABEL: foo
; CHECK:  NoAlias: i32* %P, i32* @Z

; DOM-LABEL: foo
; DOM:  NoAlias: i32* %P, i32* @Z

define void @foo(i32 %cond) nounwind {
entry:
  %"alloca point" = bitcast i32 0 to i32
@@ -29,3 +37,49 @@ bb2:
return:
  ret void
}

; Pointers can vary in between iterations of loops.
; PR18068

; CHECK-LABEL: pr18068
; CHECK: MayAlias: i32* %0, i32* %arrayidx5

; DOM-LABEL: pr18068
; DOM: MayAlias: i32* %0, i32* %arrayidx5

define i32 @pr18068(i32* %jj7, i32* %j) {
entry:
  %oa5 = alloca [100 x i32], align 16
  br label %codeRepl

codeRepl:
  %0 = phi i32* [ %arrayidx13, %for.body ], [ %j, %entry ]
  %targetBlock = call i1 @cond(i32* %jj7)
  br i1 %targetBlock, label %for.body, label %bye

for.body:
  %1 = load i32* %jj7, align 4
  %idxprom4 = zext i32 %1 to i64
  %arrayidx5 = getelementptr inbounds [100 x i32]* %oa5, i64 0, i64 %idxprom4
  %2 = load i32* %arrayidx5, align 4
  %sub6 = sub i32 %2, 6
  store i32 %sub6, i32* %arrayidx5, align 4
  ; %0 and %arrayidx5 can alias! It is not safe to DSE the above store.
  %3 = load i32* %0, align 4
  store i32 %3, i32* %arrayidx5, align 4
  %sub11 = add i32 %1, -1
  %idxprom12 = zext i32 %sub11 to i64
  %arrayidx13 = getelementptr inbounds [100 x i32]* %oa5, i64 0, i64 %idxprom12
  call void @inc(i32* %jj7)
  br label %codeRepl

bye:
  %.reload = load i32* %jj7, align 4
  ret i32 %.reload
}

declare i1 @cond(i32*)

declare void @inc(i32*)

+1 −1
Original line number Diff line number Diff line
target datalayout = "E-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-f128:128:128-v128:128:128-n32:64"
target triple = "powerpc64-bgq-linux"
; RUN: opt < %s -basicaa -aa-eval -print-all-alias-modref-info -disable-output 2>&1 | FileCheck %s
; RUN: opt < %s -domtree -basicaa -aa-eval -print-all-alias-modref-info -disable-output 2>&1 | FileCheck %s

@X = external global [16000 x double], align 32
@Y = external global [16000 x double], align 32
+1 −1
Original line number Diff line number Diff line
; RUN: opt < %s -basicaa -globalsmodref-aa -gvn -S | FileCheck %s
; RUN: opt < %s -domtree -basicaa -globalsmodref-aa -gvn -S | FileCheck %s

@X = internal global i32 4		; <i32*> [#uses=1]

Loading