From 8e438825721eb3924bef6c5cda78b7b93a544f01 Mon Sep 17 00:00:00 2001 From: Daniel Berlin Date: Mon, 26 Jan 2015 17:31:17 +0000 Subject: [PATCH] Fix incorrect partial aliasing Update testcases git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@227099 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Analysis/CFLAliasAnalysis.cpp | 27 ++++++++++++++----- .../full-store-partial-alias.ll | 8 +++--- .../CFLAliasAnalysis/gep-signed-arithmetic.ll | 6 +++-- .../CFLAliasAnalysis/must-and-partial.ll | 25 +++++++++++++---- 4 files changed, 49 insertions(+), 17 deletions(-) diff --git a/lib/Analysis/CFLAliasAnalysis.cpp b/lib/Analysis/CFLAliasAnalysis.cpp index 321b57b433c..676b6161d0a 100644 --- a/lib/Analysis/CFLAliasAnalysis.cpp +++ b/lib/Analysis/CFLAliasAnalysis.cpp @@ -298,8 +298,11 @@ public: } void visitSelectInst(SelectInst &Inst) { - auto *Condition = Inst.getCondition(); - Output.push_back(Edge(&Inst, Condition, EdgeType::Assign, AttrNone)); + // Condition is not processed here (The actual statement producing + // the condition result is processed elsewhere). For select, the + // condition is evaluated, but not loaded, stored, or assigned + // simply as a result of being the condition of a select. + auto *TrueVal = Inst.getTrueValue(); Output.push_back(Edge(&Inst, TrueVal, EdgeType::Assign, AttrNone)); auto *FalseVal = Inst.getFalseValue(); @@ -771,7 +774,10 @@ static Optional valueToAttrIndex(Value *Val) { return AttrGlobalIndex; if (auto *Arg = dyn_cast(Val)) - if (!Arg->hasNoAliasAttr()) + // Only pointer arguments should have the argument attribute, + // because things can't escape through scalars without us seeing a + // cast, and thus, interaction with them doesn't matter. + if (!Arg->hasNoAliasAttr() && Arg->getType()->isPointerTy()) return argNumberToAttrIndex(Arg->getArgNo()); return NoneType(); } @@ -994,10 +1000,6 @@ CFLAliasAnalysis::query(const AliasAnalysis::Location &LocA, auto SetA = *MaybeA; auto SetB = *MaybeB; - - if (SetA.Index == SetB.Index) - return AliasAnalysis::PartialAlias; - auto AttrsA = Sets.getLink(SetA.Index).Attrs; auto AttrsB = Sets.getLink(SetB.Index).Attrs; // Stratified set attributes are used as markets to signify whether a member @@ -1012,5 +1014,16 @@ CFLAliasAnalysis::query(const AliasAnalysis::Location &LocA, if (AttrsA.any() && AttrsB.any()) return AliasAnalysis::MayAlias; + // We currently unify things even if the accesses to them may not be in + // bounds, so we can't return partial alias here because we don't + // know whether the pointer is really within the object or not. + // IE Given an out of bounds GEP and an alloca'd pointer, we may + // unify the two. We can't return partial alias for this case. + // Since we do not currently track enough information to + // differentiate + + if (SetA.Index == SetB.Index) + return AliasAnalysis::MayAlias; + return AliasAnalysis::NoAlias; } diff --git a/test/Analysis/CFLAliasAnalysis/full-store-partial-alias.ll b/test/Analysis/CFLAliasAnalysis/full-store-partial-alias.ll index 664ea9eb118..21edfc276e6 100644 --- a/test/Analysis/CFLAliasAnalysis/full-store-partial-alias.ll +++ b/test/Analysis/CFLAliasAnalysis/full-store-partial-alias.ll @@ -2,8 +2,9 @@ ; RUN: opt -S -tbaa -gvn < %s | FileCheck %s ; Adapted from the BasicAA full-store-partial-alias.ll test. -; CFL AA should notice that the store stores to the entire %u object, +; CFL AA could notice that the store stores to the entire %u object, ; so the %tmp5 load is PartialAlias with the store and suppress TBAA. +; FIXME: However, right now, CFLAA cannot prove PartialAlias here ; Without CFL AA, TBAA should say that %tmp5 is NoAlias with the store. target datalayout = "e-p:64:64:64" @@ -14,8 +15,9 @@ target datalayout = "e-p:64:64:64" @endianness_test = global i64 1, align 8 define i32 @signbit(double %x) nounwind { -; CFLAA: ret i32 %tmp5.lobit -; CHECK: ret i32 0 +; FIXME: This would be ret i32 %tmp5.lobit if CFLAA could prove PartialAlias +; CFLAA: ret i32 0 +; CHECK: ret i32 0 entry: %u = alloca %union.anon, align 8 %tmp9 = getelementptr inbounds %union.anon* %u, i64 0, i32 0 diff --git a/test/Analysis/CFLAliasAnalysis/gep-signed-arithmetic.ll b/test/Analysis/CFLAliasAnalysis/gep-signed-arithmetic.ll index a0195d7f8d6..19d251cbb53 100644 --- a/test/Analysis/CFLAliasAnalysis/gep-signed-arithmetic.ll +++ b/test/Analysis/CFLAliasAnalysis/gep-signed-arithmetic.ll @@ -3,9 +3,11 @@ target datalayout = "e-p:32:32:32" -; CHECK: 1 partial alias response +; FIXME: This could be PartialAlias but CFLAA can't currently prove it +; CHECK: 1 may alias response -define i32 @test(i32* %tab, i32 %indvar) nounwind { +define i32 @test(i32 %indvar) nounwind { + %tab = alloca i32, align 4 %tmp31 = mul i32 %indvar, -2 %tmp32 = add i32 %tmp31, 30 %t.5 = getelementptr i32* %tab, i32 %tmp32 diff --git a/test/Analysis/CFLAliasAnalysis/must-and-partial.ll b/test/Analysis/CFLAliasAnalysis/must-and-partial.ll index df7de38ebd7..163a6c348a1 100644 --- a/test/Analysis/CFLAliasAnalysis/must-and-partial.ll +++ b/test/Analysis/CFLAliasAnalysis/must-and-partial.ll @@ -1,14 +1,15 @@ ; RUN: opt < %s -cfl-aa -aa-eval -print-all-alias-modref-info 2>&1 | FileCheck %s - ; When merging MustAlias and PartialAlias, merge to PartialAlias ; instead of MayAlias. 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" -; CHECK: PartialAlias: i16* %bigbase0, i8* %phi -define i8 @test0(i8* %base, i1 %x) { +; FIXME: This could be PartialAlias but CFLAA can't currently prove it +; CHECK: MayAlias: i16* %bigbase0, i8* %phi +define i8 @test0(i1 %x) { entry: + %base = alloca i8, align 4 %baseplusone = getelementptr i8* %base, i64 1 br i1 %x, label %red, label %green red: @@ -24,9 +25,11 @@ green: ret i8 %loaded } -; CHECK: PartialAlias: i16* %bigbase1, i8* %sel -define i8 @test1(i8* %base, i1 %x) { +; FIXME: This could be PartialAlias but CFLAA can't currently prove it +; CHECK: MayAlias: i16* %bigbase1, i8* %sel +define i8 @test1(i1 %x) { entry: + %base = alloca i8, align 4 %baseplusone = getelementptr i8* %base, i64 1 %sel = select i1 %x, i8* %baseplusone, i8* %base store i8 0, i8* %sel @@ -37,3 +40,15 @@ entry: %loaded = load i8* %sel ret i8 %loaded } + +; Incoming pointer arguments should not be PartialAlias because we do not know their initial state +; even if they are nocapture +; CHECK: MayAlias: double* %A, double* %Index +define void @testr2(double* nocapture readonly %A, double* nocapture readonly %Index) { + %arrayidx22 = getelementptr inbounds double* %Index, i64 2 + %1 = load double* %arrayidx22 + %arrayidx25 = getelementptr inbounds double* %A, i64 2 + %2 = load double* %arrayidx25 + %mul26 = fmul double %1, %2 + ret void +} -- 2.34.1