[BasicAA] Guard intrinsics don't write to memory

[sanjoy]

Summary

The idea is very close to what we do for assume intrinsics: we mark the
guard intrinsics as writing to arbitrary memory to maintain control
dependence, but under the covers we teach AA that they do not alias any
particular memory location.

[chandlerc]

lib/Analysis/BasicAliasAnalysis.cpp

783–787 ↗

(On Diff #55150)

Why MRI_Ref instead of NoModRef? And in general, why anything different from assume?

802–813 ↗

(On Diff #55150)

Again, why different behavior from assume?

[sanjoy]

lib/Analysis/BasicAliasAnalysis.cpp

783–787 ↗

(On Diff #55150)

Guards have deoptimization state (via "deopt" operand bundles) and are thus allowed to read all memory. If a guard fails then, at the time of failure, the heap better look like it was supposed to look like in the original program. E.g. see @test6 in D19578.

[george.burgess.iv]

lib/Analysis/BasicAliasAnalysis.cpp

783–787 ↗

(On Diff #55150)

Can you add a comment in the code noting this, please? :)

[reames]

lib/Analysis/BasicAliasAnalysis.cpp

785 ↗

(On Diff #55150)

As others pointed out, this comment is really confusing.

[sanjoy]

• Add comments, as asked for in the review

[sanjoy]

• Clarify comments a bit more (s/aliases/mods/), and also ping

[reames]

LGTM w/minor comment.

test/Analysis/BasicAA/guards.ll

25 ↗

(On Diff #56617)

please rename to reflect this is an readonly call.

llvm/trunk/lib/Analysis/BasicAliasAnalysis.cpp

if (CS.paramHasAttr(ArgIdx + 1, Attribute::ReadOnly))
     return MRI_Ref;
 
   if (CS.paramHasAttr(ArgIdx + 1, Attribute::ReadNone))
     return MRI_NoModRef;
 
   return AAResultBase::getArgModRefInfo(CS, ArgIdx);
 }
 
-static bool isAssumeIntrinsic(ImmutableCallSite CS) {
+static bool isIntrinsicCall(ImmutableCallSite CS, Intrinsic::ID IID) {
   const IntrinsicInst *II = dyn_cast<IntrinsicInst>(CS.getInstruction());
-  return II && II->getIntrinsicID() == Intrinsic::assume;
+  return II && II->getIntrinsicID() == IID;
 }
 
 #ifndef NDEBUG
 static const Function *getParent(const Value *V) {
   if (const Instruction *inst = dyn_cast<Instruction>(V))
     return inst->getParent()->getParent();
 
   if (const Argument *arg = dyn_cast<Argument>(V))
if (isMallocLikeFn(Inst, &TLI) || isCallocLikeFn(Inst, &TLI)) {
     // fallback to the generic handling below.
     if (getBestAAResults().alias(MemoryLocation(Inst), Loc) == NoAlias)
       return MRI_NoModRef;
   }
 
   // While the assume intrinsic is marked as arbitrarily writing so that
   // proper control dependencies will be maintained, it never aliases any
   // particular memory location.
-  if (isAssumeIntrinsic(CS))
+  if (isIntrinsicCall(CS, Intrinsic::assume))
     return MRI_NoModRef;
 
+  // Like assumes, guard intrinsics are also marked as arbitrarily writing so
+  // that proper control dependencies are maintained but they never mods any
+  // particular memory location.
+  //
+  // *Unlike* assumes, guard intrinsics are modeled as reading memory since the
+  // heap state at the point the guard is issued needs to be consistent in case
+  // the guard invokes the "deopt" continuation.
+  if (isIntrinsicCall(CS, Intrinsic::experimental_guard))
+    return MRI_Ref;
+
   // The AAResultBase base class has some smarts, lets use them.
   return AAResultBase::getModRefInfo(CS, Loc);
 }
 
 ModRefInfo BasicAAResult::getModRefInfo(ImmutableCallSite CS1,
                                         ImmutableCallSite CS2) {
   // While the assume intrinsic is marked as arbitrarily writing so that
   // proper control dependencies will be maintained, it never aliases any
   // particular memory location.
-  if (isAssumeIntrinsic(CS1) || isAssumeIntrinsic(CS2))
+  if (isIntrinsicCall(CS1, Intrinsic::assume) ||
+      isIntrinsicCall(CS2, Intrinsic::assume))
     return MRI_NoModRef;
 
+  // Like assumes, guard intrinsics are also marked as arbitrarily writing so
+  // that proper control dependencies are maintained but they never mod any
+  // particular memory location.
+  //
+  // *Unlike* assumes, guard intrinsics are modeled as reading memory since the
+  // heap state at the point the guard is issued needs to be consistent in case
+  // the guard invokes the "deopt" continuation.
+
+  // NB! This function is *not* commutative, so we specical case two
+  // possibilities for guard intrinsics.
+
+  if (isIntrinsicCall(CS1, Intrinsic::experimental_guard))
+    return getModRefBehavior(CS2) & MRI_Mod ? MRI_Ref : MRI_NoModRef;
+
+  if (isIntrinsicCall(CS2, Intrinsic::experimental_guard))
+    return getModRefBehavior(CS1) & MRI_Mod ? MRI_Mod : MRI_NoModRef;
+
   // The AAResultBase base class has some smarts, lets use them.
   return AAResultBase::getModRefInfo(CS1, CS2);
 }
 
 /// Provide ad-hoc rules to disambiguate accesses through two GEP operators,
 /// both having the exact same pointer operand.
 static AliasResult aliasSameBasePointerGEPs(const GEPOperator *GEP1,
                                             uint64_t V1Size,

llvm/trunk/test/Analysis/BasicAA/guards.ll

+; RUN: opt < %s -basicaa -aa-eval -print-all-alias-modref-info -disable-output 2>&1 | FileCheck %s
+target datalayout = "e-p:32:32:32-i1:8:32-i8:8:32-i16:16:32-i32:32:32-i64:32:32-f32:32:32-f64:32:32-v64:32:64-v128:32:128-a0:0:32-n32"
+
+declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture, i8* nocapture, i64, i32, i1) #0
+declare void @llvm.experimental.guard(i1, ...)
+declare void @unknown_but_readonly() readonly
+
+define void @test1(i8* %P, i8* %Q) {
+  tail call void(i1,...) @llvm.experimental.guard(i1 true) [ "deopt"() ]
+  tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* %P, i8* %Q, i64 12, i32 1, i1 false)
+  ret void
+
+; CHECK-LABEL: Function: test1:
+
+; CHECK:  Just Ref:  Ptr: i8* %P  <->  tail call void (i1, ...) @llvm.experimental.guard(i1 true) [ "deopt"() ]
+; CHECK:  Just Ref:  Ptr: i8* %Q  <->  tail call void (i1, ...) @llvm.experimental.guard(i1 true) [ "deopt"() ]
+; CHECK:  Both ModRef:  Ptr: i8* %P <->  tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* %P, i8* %Q, i64 12, i32 1, i1 false)
+; CHECK:  Both ModRef:  Ptr: i8* %Q <->  tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* %P, i8* %Q, i64 12, i32 1, i1 false)
+; CHECK:  Just Ref:   tail call void (i1, ...) @llvm.experimental.guard(i1 true) [ "deopt"() ] <->   tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* %P, i8* %Q, i64 12, i32 1, i1 false)
+; CHECK:  Just Mod:   tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* %P, i8* %Q, i64 12, i32 1, i1 false) <->   tail call void (i1, ...) @llvm.experimental.guard(i1 true) [ "deopt"() ]
+}
+
+define void @test2() {
+  tail call void(i1,...) @llvm.experimental.guard(i1 true) [ "deopt"() ]
+  tail call void @unknown_but_readonly()
+  ret void
+; CHECK-LABEL: Function: test2:
+; CHECK:  NoModRef:   tail call void (i1, ...) @llvm.experimental.guard(i1 true) [ "deopt"() ] <->   tail call void @unknown_but_readonly()
+; CHECK:  NoModRef:   tail call void @unknown_but_readonly() <->   tail call void (i1, ...) @llvm.experimental.guard(i1 true) [ "deopt"() ]
+}