[PATCH] D93183: [BasicAA] Make sure context instruction is symmetric

[Nikita Popov via Phabricator via llvm-commits]

nikic created this revision.
nikic added reviewers: fhahn, jdoerfert, asbirlea.
Herald added subscribers: arphaman, hiraditya.
nikic requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

D71264 <https://reviews.llvm.org/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 picking the instruction with larger context, or no context instruction if there is no dominance relationship. This is not great, but I couldn't figure out a deterministic criterion for that case.

(I have some doubts whether it's possible really possible to make AA fully symmetric, to the point that we can assert it. I'm okay with the resolution here being "don't bother with these edge-cases".)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93183

Files:
  llvm/lib/Analysis/BasicAliasAnalysis.cpp
  llvm/test/Analysis/BasicAA/assume-index-positive.ll


Index: llvm/test/Analysis/BasicAA/assume-index-positive.ll
===================================================================
--- llvm/test/Analysis/BasicAA/assume-index-positive.ll
+++ llvm/test/Analysis/BasicAA/assume-index-positive.ll
@@ -113,4 +113,20 @@
   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
+;
+  %c.off = add nuw nsw i32 %c, 1
+  %gep1 = getelementptr [0 x i8], [0 x i8]* %ptr, i32 %a, i32 %b
+  call void @barrier()
+  %b.cmp = icmp slt i32 %b, 0
+  call void @llvm.assume(i1 %b.cmp)
+  %c.cmp = icmp sgt i32 %c, -1
+  call void @llvm.assume(i1 %c.cmp)
+  %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()
Index: llvm/lib/Analysis/BasicAliasAnalysis.cpp
===================================================================
--- llvm/lib/Analysis/BasicAliasAnalysis.cpp
+++ llvm/lib/Analysis/BasicAliasAnalysis.cpp
@@ -1222,6 +1222,21 @@
   }
 
   if (!DecompGEP1.VarIndices.empty()) {
+    // Pick context instruction for ValueTracking queries. Using either
+    // instruction is correct, but the one with the larger context (the "more
+    // dominated" one) is preferable. If both instructions are GEPs not in a
+    // dominance relationship, don't use a context instruction, as we have no
+    // select criterion that is independent of the order in the alias() call.
+    const auto *CxtI = dyn_cast<Instruction>(GEP1);
+    if (CxtI) {
+      if (const auto *I2 = dyn_cast<GetElementPtrInst>(V2)) {
+        if (DT->dominates(CxtI, I2))
+          CxtI = I2;
+        else if (!DT->dominates(I2, CxtI))
+          CxtI = nullptr;
+      }
+    }
+
     APInt GCD;
     bool AllNonNegative = DecompGEP1.Offset.isNonNegative();
     bool AllNonPositive = DecompGEP1.Offset.isNonPositive();
@@ -1238,8 +1253,7 @@
         // give up if we can't determine conditions that hold for every cycle:
         const Value *V = DecompGEP1.VarIndices[i].V;
 
-        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();
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D93183.311464.patch
Type: text/x-patch
Size: 2355 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201213/fcc02033/attachment.bin>