[LLVMdev] question about enabling cfl-aa and collecting a57 numbers
Daniel Berlin
dberlin at dberlin.org
Fri Jan 16 12:22:23 PST 2015
Okay, overnight i ran a ton of tests on this patch, and it seems right.
Nick, Hal, can you review it?
I've reattached it for simplicity
On Thu, Jan 15, 2015 at 3:05 PM, Daniel Berlin <dberlin at dberlin.org> wrote:
>
>
> On Thu, Jan 15, 2015 at 1:26 PM, Nick Lewycky <nlewycky at google.com> wrote:
>
>> On 15 January 2015 at 13:10, Daniel Berlin <dberlin at dberlin.org> wrote:
>>
>>> Yes.
>>> I've attached an updated patch that does the following:
>>>
>>> 1. Fixes the partialalias of globals/arguments
>>> 2. Enables partialalias for cases where nothing has been unified to a
>>> global/argument
>>> 3. Fixes that select was unifying the condition to the other pieces (the
>>> condition does not need to be processed :P). This was causing unnecessary
>>> aliasing.
>>>
>>
>> Consider this:
>>
>> void *p = ...;
>> uintptr_t i = p;
>> uintptr_t j = 0;
>> for (int a = 0; a < sizeof(uintptr_t); ++a) {
>> j = i >> (sizeof(uintptr_t) - a - 1) ? 1 : 0;
>> j <<= 1;
>> }
>> void *q = j;
>>
>> alias(p, q) isn't NoAlias. (Okay, it kinda is in C++, but not in the
>> equivalent LLVM IR. Please don't make me rewrite my example in LLVM IR.)
>>
>> Agreed :)
>
> But after chatting with you, i think we both agree that this change does
> not affect.
> I probably should not have said "the condition does not need to be
> processed". It would be more accurate to say "the reference to a condition
> in a select instruction, by itself, does not cause aliasing"
>
> What happens now is:
>
> given %4 = select %1, %2, %3
>
> we do
> aliasset(%4) += %1
> aliasset(%4) += %2
> aliasset(%4) += %3
>
> The first one is unnecessary.
> There can be no alias caused simply because it is referenced in condition
> of the select.
>
>> We still need to process what %1 refers to (and we do).
>
>
> To make this empirical, in your example, we get the right answer in CFL-AA.
>
> Interestingly, i'll point out that basic-aa says:
> NoAlias: i8* %p, i8** %q
> NoAlias: i8** %p.addr, i8** %q
>
> (I translated your case as best i can :P)
>
> So you may want to implement it for real if you think it's supposed to be
> handled right in basic-aa, because I don't believe it is :)
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150116/e2074450/attachment.html>
-------------- next part --------------
diff --git a/lib/Analysis/CFLAliasAnalysis.cpp b/lib/Analysis/CFLAliasAnalysis.cpp
index 88bb84a..3741bb9 100644
--- a/lib/Analysis/CFLAliasAnalysis.cpp
+++ b/lib/Analysis/CFLAliasAnalysis.cpp
@@ -227,10 +227,13 @@ public:
// Comparisons between global variables and other constants should be
// handled by BasicAA.
if (isa<Constant>(LocA.Ptr) && isa<Constant>(LocB.Ptr)) {
- return MayAlias;
+ return AliasAnalysis::alias(LocA, LocB);
}
+ AliasResult QueryResult = query(LocA, LocB);
+ if (QueryResult == MayAlias)
+ return AliasAnalysis::alias(LocA, LocB);
- return query(LocA, LocB);
+ return QueryResult;
}
void initializePass() override { InitializeAliasAnalysis(this); }
@@ -295,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();
@@ -768,7 +774,10 @@ static Optional<StratifiedAttr> valueToAttrIndex(Value *Val) {
return AttrGlobalIndex;
if (auto *Arg = dyn_cast<Argument>(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();
}
@@ -991,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
@@ -1009,5 +1014,12 @@ CFLAliasAnalysis::query(const AliasAnalysis::Location &LocA,
if (AttrsA.any() && AttrsB.any())
return AliasAnalysis::MayAlias;
+ // If we haven't interacted with globals, and they still got
+ // unified, they are partial alias right now.
+ // NOTE: This assumes we don't unify for any reason other than
+ // assignments
+ if (SetA.Index == SetB.Index)
+ return AliasAnalysis::PartialAlias;
+
return AliasAnalysis::NoAlias;
}
diff --git a/lib/CodeGen/Passes.cpp b/lib/CodeGen/Passes.cpp
index 28e9f84..82ca024 100644
--- a/lib/CodeGen/Passes.cpp
+++ b/lib/CodeGen/Passes.cpp
@@ -400,10 +400,10 @@ void TargetPassConfig::addIRPasses() {
// Add TypeBasedAliasAnalysis before BasicAliasAnalysis so that
// BasicAliasAnalysis wins if they disagree. This is intended to help
// support "obvious" type-punning idioms.
- if (UseCFLAA)
- addPass(createCFLAliasAnalysisPass());
addPass(createTypeBasedAliasAnalysisPass());
addPass(createScopedNoAliasAAPass());
+ if (UseCFLAA)
+ addPass(createCFLAliasAnalysisPass());
addPass(createBasicAliasAnalysisPass());
// Before running any passes, run the verifier to determine if the input
diff --git a/lib/Transforms/IPO/PassManagerBuilder.cpp b/lib/Transforms/IPO/PassManagerBuilder.cpp
index bb776ef..645686c 100644
--- a/lib/Transforms/IPO/PassManagerBuilder.cpp
+++ b/lib/Transforms/IPO/PassManagerBuilder.cpp
@@ -132,10 +132,10 @@ PassManagerBuilder::addInitialAliasAnalysisPasses(PassManagerBase &PM) const {
// Add TypeBasedAliasAnalysis before BasicAliasAnalysis so that
// BasicAliasAnalysis wins if they disagree. This is intended to help
// support "obvious" type-punning idioms.
- if (UseCFLAA)
- PM.add(createCFLAliasAnalysisPass());
PM.add(createTypeBasedAliasAnalysisPass());
PM.add(createScopedNoAliasAAPass());
+ if (UseCFLAA)
+ PM.add(createCFLAliasAnalysisPass());
PM.add(createBasicAliasAnalysisPass());
}
diff --git a/test/Analysis/CFLAliasAnalysis/gep-signed-arithmetic.ll b/test/Analysis/CFLAliasAnalysis/gep-signed-arithmetic.ll
index a0195d7..f68edaf 100644
--- a/test/Analysis/CFLAliasAnalysis/gep-signed-arithmetic.ll
+++ b/test/Analysis/CFLAliasAnalysis/gep-signed-arithmetic.ll
@@ -5,7 +5,8 @@ target datalayout = "e-p:32:32:32"
; CHECK: 1 partial 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 df7de38..2c66d90 100644
--- a/test/Analysis/CFLAliasAnalysis/must-and-partial.ll
+++ b/test/Analysis/CFLAliasAnalysis/must-and-partial.ll
@@ -7,8 +7,9 @@
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) {
+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:
@@ -25,8 +26,9 @@ green:
}
; CHECK: PartialAlias: i16* %bigbase1, i8* %sel
-define i8 @test1(i8* %base, i1 %x) {
+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 +39,15 @@ entry:
%loaded = load i8* %sel
ret i8 %loaded
}
+
+; Arguments should not be partial alias
+; 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
+}
+
More information about the llvm-dev
mailing list