[llvm-branch-commits] [llvm] b96a6ea - [BasicAA] Make sure context instruction is symmetric

Nikita Popov via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Fri Dec 25 02:40:29 PST 2020


Author: Nikita Popov
Date: 2020-12-25T11:35:46+01:00
New Revision: b96a6ea0a94e45ede46f534c6b5319f4ffb9d986

URL: https://github.com/llvm/llvm-project/commit/b96a6ea0a94e45ede46f534c6b5319f4ffb9d986
DIFF: https://github.com/llvm/llvm-project/commit/b96a6ea0a94e45ede46f534c6b5319f4ffb9d986.diff

LOG: [BasicAA] Make sure context instruction is symmetric

D71264 started using a context instruction in a computeKnownBits()
call. However, if aliasing between two GEPs is checked, then the
choice of context instruction will be different for alias(GEP1, GEP2)
and alias(GEP2, GEP1), which is not supposed to happen.

Resolve this by remembering which GEP a certain VarIndex belongs to,
and use that as the context instruction. This makes the choice of
context instruction predictable and symmetric.

It should be noted that this choice of context instruction is
non-optimal (just like the previous choice): The AA query result is
only valid at points that are reachable from *both* instructions.
Using either one of them is conservatively correct, but a larger
context may also be valid to use.

Differential Revision: https://reviews.llvm.org/D93183

Added: 
    

Modified: 
    llvm/include/llvm/Analysis/BasicAliasAnalysis.h
    llvm/lib/Analysis/BasicAliasAnalysis.cpp
    llvm/test/Analysis/BasicAA/assume-index-positive.ll

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Analysis/BasicAliasAnalysis.h b/llvm/include/llvm/Analysis/BasicAliasAnalysis.h
index 6eb37c1c4b07..cffe9ed3c738 100644
--- a/llvm/include/llvm/Analysis/BasicAliasAnalysis.h
+++ b/llvm/include/llvm/Analysis/BasicAliasAnalysis.h
@@ -117,6 +117,9 @@ class BasicAAResult : public AAResultBase<BasicAAResult> {
 
     APInt Scale;
 
+    // Context instruction to use when querying information about this index.
+    const Instruction *CxtI;
+
     bool operator==(const VariableGEPIndex &Other) const {
       return V == Other.V && ZExtBits == Other.ZExtBits &&
              SExtBits == Other.SExtBits && Scale == Other.Scale;

diff  --git a/llvm/lib/Analysis/BasicAliasAnalysis.cpp b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
index caa0635459b3..4a30e075af0a 100644
--- a/llvm/lib/Analysis/BasicAliasAnalysis.cpp
+++ b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
@@ -422,6 +422,7 @@ BasicAAResult::DecomposeGEPExpression(const Value *V, const DataLayout &DL,
   // Limit recursion depth to limit compile time in crazy cases.
   unsigned MaxLookup = MaxLookupSearchDepth;
   SearchTimes++;
+  const Instruction *CxtI = dyn_cast<Instruction>(V);
 
   unsigned MaxPointerSize = getMaxPointerSize(DL);
   DecomposedGEP Decomposed;
@@ -584,7 +585,7 @@ BasicAAResult::DecomposeGEPExpression(const Value *V, const DataLayout &DL,
       Scale = adjustToPointerSize(Scale, PointerSize);
 
       if (!!Scale) {
-        VariableGEPIndex Entry = {Index, ZExtBits, SExtBits, Scale};
+        VariableGEPIndex Entry = {Index, ZExtBits, SExtBits, Scale, CxtI};
         Decomposed.VarIndices.push_back(Entry);
       }
     }
@@ -1225,9 +1226,9 @@ AliasResult BasicAAResult::aliasGEP(
         // the Value this cycle may not hold in the next cycle. We'll just
         // give up if we can't determine conditions that hold for every cycle:
         const Value *V = DecompGEP1.VarIndices[i].V;
+        const Instruction *CxtI = DecompGEP1.VarIndices[i].CxtI;
 
-        KnownBits Known =
-            computeKnownBits(V, DL, 0, &AC, dyn_cast<Instruction>(GEP1), DT);
+        KnownBits Known = computeKnownBits(V, DL, 0, &AC, CxtI, DT);
         bool SignKnownZero = Known.isNonNegative();
         bool SignKnownOne = Known.isNegative();
 
@@ -1755,7 +1756,7 @@ void BasicAAResult::GetIndexDifference(
 
     // If we didn't consume this entry, add it to the end of the Dest list.
     if (!!Scale) {
-      VariableGEPIndex Entry = {V, ZExtBits, SExtBits, -Scale};
+      VariableGEPIndex Entry = {V, ZExtBits, SExtBits, -Scale, Src[i].CxtI};
       Dest.push_back(Entry);
     }
   }

diff  --git a/llvm/test/Analysis/BasicAA/assume-index-positive.ll b/llvm/test/Analysis/BasicAA/assume-index-positive.ll
index e8a039a1018e..54eb34a2cdb9 100644
--- a/llvm/test/Analysis/BasicAA/assume-index-positive.ll
+++ b/llvm/test/Analysis/BasicAA/assume-index-positive.ll
@@ -113,4 +113,20 @@ define void @test4(double* %ptr, i32 %skip) {
   ret void
 }
 
+define void @symmetry([0 x i8]* %ptr, i32 %a, i32 %b, i32 %c) {
+; CHECK-LABEL: Function: symmetry
+; CHECK: NoAlias: i8* %gep1, i8* %gep2
+;
+  %b.cmp = icmp slt i32 %b, 0
+  call void @llvm.assume(i1 %b.cmp)
+  %gep1 = getelementptr [0 x i8], [0 x i8]* %ptr, i32 %a, i32 %b
+  call void @barrier()
+  %c.cmp = icmp sgt i32 %c, -1
+  call void @llvm.assume(i1 %c.cmp)
+  %c.off = add nuw nsw i32 %c, 1
+  %gep2 = getelementptr [0 x i8], [0 x i8]* %ptr, i32 %a, i32 %c.off
+  ret void
+}
+
 declare void @llvm.assume(i1 %cond)
+declare void @barrier()


        


More information about the llvm-branch-commits mailing list