[AA] Skip the layer of indirection in returning conservative results.

[davidtgoldblatt]

Summary

Historically, AA implementations chained to a following implementation
to answer recursive queries. This is no longer the case, but the legacy
lives on in a confusing phrasing of the return-a-conservative-value
paths. Let's just return "don't know" directly, where appropriate; the
current two-step way is confusing.

[davidtgoldblatt]

I *didn't* change ObjCARCAliasAnalysis, but left a TODO. I don't have any good way to evaluate a "real" fix (i.e. doing the recursive queries in the modern way), and having the intent only documented in years-old revision history feels hard to spot. Not really a strong opinion though, happy to do something more intrusive there.

[nikic]

LGTM

[davidtgoldblatt]

Sorry for leaving this open for so long; various distractions kept popping up -- I'll wait a day to give a last chance for any objections and land tomorrow.

llvm/lib/Analysis/BasicAliasAnalysis.cpp

do {
     }
 
     // A global constant can't be mutated.
     if (const GlobalVariable *GV = dyn_cast<GlobalVariable>(V)) {
       // Note: this doesn't require GV to be "ODR" because it isn't legal for a
       // global to be marked constant in some modules and non-constant in
       // others.  GV may even be a declaration, not a definition.
       if (!GV->isConstant())
-        return AAResultBase::getModRefInfoMask(Loc, AAQI, IgnoreLocals);
+        return ModRefInfo::ModRef;
       continue;
     }
 
     // If both select values point to local memory, then so does the select.
     if (const SelectInst *SI = dyn_cast<SelectInst>(V)) {
       Worklist.push_back(SI->getTrueValue());
       Worklist.push_back(SI->getFalseValue());
       continue;
     }
 
     // If all values incoming to a phi node point to local memory, then so does
     // the phi.
     if (const PHINode *PN = dyn_cast<PHINode>(V)) {
       // Don't bother inspecting phi nodes with many operands.
       if (PN->getNumIncomingValues() > MaxLookup)
-        return AAResultBase::getModRefInfoMask(Loc, AAQI, IgnoreLocals);
+        return ModRefInfo::ModRef;
       append_range(Worklist, PN->incoming_values());
       continue;
     }
 
     // Otherwise be conservative.
-    return AAResultBase::getModRefInfoMask(Loc, AAQI, IgnoreLocals);
+    return ModRefInfo::ModRef;
   } while (!Worklist.empty() && --MaxLookup);
 
   // If we hit the maximum number of instructions to examine, be conservative.
   if (!Worklist.empty())
-    return AAResultBase::getModRefInfoMask(Loc, AAQI, IgnoreLocals);
+    return ModRefInfo::ModRef;
 
   return Result;
 }
 
 static bool isIntrinsicCall(const CallBase *Call, Intrinsic::ID IID) {
   const IntrinsicInst *II = dyn_cast<IntrinsicInst>(Call);
   return II && II->getIntrinsicID() == IID;
 }
if (Call->paramHasAttr(ArgIdx, Attribute::WriteOnly))
     return ModRefInfo::Mod;
 
   if (Call->paramHasAttr(ArgIdx, Attribute::ReadOnly))
     return ModRefInfo::Ref;
 
   if (Call->paramHasAttr(ArgIdx, Attribute::ReadNone))
     return ModRefInfo::NoModRef;
 
-  return AAResultBase::getArgModRefInfo(Call, ArgIdx);
+  return ModRefInfo::ModRef;
 }
 
 #ifndef NDEBUG
 static const Function *getParent(const Value *V) {
   if (const Instruction *inst = dyn_cast<Instruction>(V)) {
     if (!inst->getParent())
       return nullptr;
     return inst->getParent()->getParent();
ModRefInfo BasicAAResult::getModRefInfo(const CallBase *Call,
   // print(val);
   //
   // The transformation will cause the second store to be ignored (based on
   // rules of invariant.start)  and print 40, while the first program always
   // prints 50.
   if (isIntrinsicCall(Call, Intrinsic::invariant_start))
     return ModRefInfo::Ref;
 
-  // The AAResultBase base class has some smarts, lets use them.
-  return AAResultBase::getModRefInfo(Call, Loc, AAQI);
+  // Be conservative.
+  return ModRefInfo::ModRef;
 }
 
 ModRefInfo BasicAAResult::getModRefInfo(const CallBase *Call1,
                                         const CallBase *Call2,
                                         AAQueryInfo &AAQI) {
   // Guard intrinsics are marked as arbitrarily writing so that proper control
   // dependencies are maintained but they never mods any particular memory
   // location.
return isModSet(getMemoryEffects(Call2, AAQI).getModRef())
                ? ModRefInfo::Ref
                : ModRefInfo::NoModRef;
 
   if (isIntrinsicCall(Call2, Intrinsic::experimental_guard))
     return isModSet(getMemoryEffects(Call1, AAQI).getModRef())
                ? ModRefInfo::Mod
                : ModRefInfo::NoModRef;
 
-  // The AAResultBase base class has some smarts, lets use them.
-  return AAResultBase::getModRefInfo(Call1, Call2, AAQI);
+  // Be conservative.
+  return ModRefInfo::ModRef;
 }
 
 /// Return true if we know V to the base address of the corresponding memory
 /// object.  This implies that any address less than V must be out of bounds
 /// for the underlying object.  Note that just being isIdentifiedObject() is
 /// not enough - For example, a negative offset from a noalias argument or call
 /// can be inbounds w.r.t the actual underlying object.
 static bool isBaseOfObject(const Value *V) {

llvm/lib/Analysis/GlobalsModRef.cpp

void GlobalsAAResult::DeletionCallbackHandle::deleted() {
   GAR->Handles.erase(I);
   // This object is now destroyed!
 }
 
 MemoryEffects GlobalsAAResult::getMemoryEffects(const Function *F) {
   if (FunctionInfo *FI = getFunctionInfo(F))
     return MemoryEffects(FI->getModRefInfo());
 
-  return AAResultBase::getMemoryEffects(F);
+  return MemoryEffects::unknown();
 }
 
 /// Returns the function info for the function, or null if we don't have
 /// anything useful to say about it.
 GlobalsAAResult::FunctionInfo *
 GlobalsAAResult::getFunctionInfo(const Function *F) {
   auto I = FunctionInfos.find(F);
   if (I != FunctionInfos.end())
if (auto *PN = dyn_cast<PHINode>(Input)) {
         if (Visited.insert(Op).second)
           Inputs.push_back(Op);
       }
       continue;
     }
 
     // FIXME: It would be good to handle other obvious no-alias cases here, but
     // it isn't clear how to do so reasonably without building a small version
-    // of BasicAA into this code. We could recurse into AAResultBase::alias
-    // here but that seems likely to go poorly as we're inside the
-    // implementation of such a query. Until then, just conservatively return
-    // false.
+    // of BasicAA into this code.
     return false;
   } while (!Inputs.empty());
 
   // If all the inputs to V were definitively no-alias, then V is no-alias.
   return true;
 }
 
 bool GlobalsAAResult::invalidate(Module &, const PreservedAnalyses &PA,
AliasResult GlobalsAAResult::alias(const MemoryLocation &LocA,
 
   // If one is based on an indirect global and the other isn't, it isn't
   // strictly safe but we can fake this result if necessary for performance.
   // This does not appear to be a common problem in practice.
   if (EnableUnsafeGlobalsModRefAliasResults)
     if ((GV1 || GV2) && GV1 != GV2)
       return AliasResult::NoAlias;
 
-  return AAResultBase::alias(LocA, LocB, AAQI, nullptr);
+  return AliasResult::MayAlias;
 }
 
 ModRefInfo GlobalsAAResult::getModRefInfoForArgument(const CallBase *Call,
                                                      const GlobalValue *GV,
                                                      AAQueryInfo &AAQI) {
   if (Call->doesNotAccessMemory())
     return ModRefInfo::NoModRef;
   ModRefInfo ConservativeResult =

llvm/lib/Analysis/ObjCARCAliasAnalysis.cpp

 ///
 /// WARNING: This file knows about how certain Objective-C library functions are
 /// used. Naive LLVM IR transformations which would otherwise be
 /// behavior-preserving may break these assumptions.
 ///
 /// TODO: Theoretically we could check for dependencies between objc_* calls
 /// and FMRB_OnlyAccessesArgumentPointees calls or other well-behaved calls.
 ///
+/// TODO: The calls here to AAResultBase member functions are all effectively
+/// no-ops that just return a conservative result. The original intent was to
+/// chain to another analysis for a recursive query, but this was lost in a
+/// refactor. These should instead be rephrased in terms of queries to AAQI.AAR.
+///
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Analysis/ObjCARCAliasAnalysis.h"
 #include "llvm/Analysis/ObjCARCAnalysisUtils.h"
 #include "llvm/Analysis/Passes.h"
 #include "llvm/IR/Function.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/Pass.h"

llvm/lib/Analysis/ScalarEvolutionAliasAnalysis.cpp

if (alias(MemoryLocation(AO ? AO : LocA.Ptr,
                              AO ? AAMDNodes() : LocA.AATags),
               MemoryLocation(BO ? BO : LocB.Ptr,
                              BO ? LocationSize::beforeOrAfterPointer()
                                 : LocB.Size,
                              BO ? AAMDNodes() : LocB.AATags),
               AAQI, nullptr) == AliasResult::NoAlias)
       return AliasResult::NoAlias;
 
-  // Forward the query to the next analysis.
-  return AAResultBase::alias(LocA, LocB, AAQI, nullptr);
+  return AliasResult::MayAlias;
 }
 
 /// Given an expression, try to find a base value.
 ///
 /// Returns null if none was found.
 Value *SCEVAAResult::GetBaseValue(const SCEV *S) {
   if (const SCEVAddRecExpr *AR = dyn_cast<SCEVAddRecExpr>(S)) {
     // In an addrec, assume that the base will be in the start, rather

llvm/lib/Analysis/ScopedNoAliasAA.cpp

 static cl::opt<bool> EnableScopedNoAlias("enable-scoped-noalias",
                                          cl::init(true), cl::Hidden);
 
 AliasResult ScopedNoAliasAAResult::alias(const MemoryLocation &LocA,
                                          const MemoryLocation &LocB,
                                          AAQueryInfo &AAQI,
                                          const Instruction *) {
   if (!EnableScopedNoAlias)
-    return AAResultBase::alias(LocA, LocB, AAQI, nullptr);
+    return AliasResult::MayAlias;
 
   // Get the attached MDNodes.
   const MDNode *AScopes = LocA.AATags.Scope, *BScopes = LocB.AATags.Scope;
 
   const MDNode *ANoAlias = LocA.AATags.NoAlias, *BNoAlias = LocB.AATags.NoAlias;
 
   if (!mayAliasInScopes(AScopes, BNoAlias))
     return AliasResult::NoAlias;
 
   if (!mayAliasInScopes(BScopes, ANoAlias))
     return AliasResult::NoAlias;
 
-  // If they may alias, chain to the next AliasAnalysis.
-  return AAResultBase::alias(LocA, LocB, AAQI, nullptr);
+  return AliasResult::MayAlias;
 }
 
 ModRefInfo ScopedNoAliasAAResult::getModRefInfo(const CallBase *Call,
                                                 const MemoryLocation &Loc,
                                                 AAQueryInfo &AAQI) {
   if (!EnableScopedNoAlias)
-    return AAResultBase::getModRefInfo(Call, Loc, AAQI);
+    return ModRefInfo::ModRef;
 
   if (!mayAliasInScopes(Loc.AATags.Scope,
                         Call->getMetadata(LLVMContext::MD_noalias)))
     return ModRefInfo::NoModRef;
 
   if (!mayAliasInScopes(Call->getMetadata(LLVMContext::MD_alias_scope),
                         Loc.AATags.NoAlias))
     return ModRefInfo::NoModRef;
 
-  return AAResultBase::getModRefInfo(Call, Loc, AAQI);
+  return ModRefInfo::ModRef;
 }
 
 ModRefInfo ScopedNoAliasAAResult::getModRefInfo(const CallBase *Call1,
                                                 const CallBase *Call2,
                                                 AAQueryInfo &AAQI) {
   if (!EnableScopedNoAlias)
-    return AAResultBase::getModRefInfo(Call1, Call2, AAQI);
+    return ModRefInfo::ModRef;
 
   if (!mayAliasInScopes(Call1->getMetadata(LLVMContext::MD_alias_scope),
                         Call2->getMetadata(LLVMContext::MD_noalias)))
     return ModRefInfo::NoModRef;
 
   if (!mayAliasInScopes(Call2->getMetadata(LLVMContext::MD_alias_scope),
                         Call1->getMetadata(LLVMContext::MD_noalias)))
     return ModRefInfo::NoModRef;
 
-  return AAResultBase::getModRefInfo(Call1, Call2, AAQI);
+  return ModRefInfo::ModRef;
 }
 
 static void collectMDInDomain(const MDNode *List, const MDNode *Domain,
                               SmallPtrSetImpl<const MDNode *> &Nodes) {
   for (const MDOperand &MDOp : List->operands())
     if (const MDNode *MD = dyn_cast<MDNode>(MDOp))
       if (AliasScopeNode(MD).getDomain() == Domain)
         Nodes.insert(MD);

llvm/lib/Analysis/TypeBasedAliasAnalysis.cpp

static bool isStructPathTBAA(const MDNode *MD) {
   // a TBAA tag.
   return isa<MDNode>(MD->getOperand(0)) && MD->getNumOperands() >= 3;
 }
 
 AliasResult TypeBasedAAResult::alias(const MemoryLocation &LocA,
                                      const MemoryLocation &LocB,
                                      AAQueryInfo &AAQI, const Instruction *) {
   if (!EnableTBAA)
-    return AAResultBase::alias(LocA, LocB, AAQI, nullptr);
+    return AliasResult::MayAlias;
 
-  // If accesses may alias, chain to the next AliasAnalysis.
   if (Aliases(LocA.AATags.TBAA, LocB.AATags.TBAA))
-    return AAResultBase::alias(LocA, LocB, AAQI, nullptr);
+    return AliasResult::MayAlias;
 
   // Otherwise return a definitive result.
   return AliasResult::NoAlias;
 }
 
 ModRefInfo TypeBasedAAResult::getModRefInfoMask(const MemoryLocation &Loc,
                                                 AAQueryInfo &AAQI,
                                                 bool IgnoreLocals) {
   if (!EnableTBAA)
-    return AAResultBase::getModRefInfoMask(Loc, AAQI, IgnoreLocals);
+    return ModRefInfo::ModRef;
 
   const MDNode *M = Loc.AATags.TBAA;
   if (!M)
-    return AAResultBase::getModRefInfoMask(Loc, AAQI, IgnoreLocals);
+    return ModRefInfo::ModRef;
 
   // If this is an "immutable" type, we can assume the pointer is pointing
   // to constant memory.
   if ((!isStructPathTBAA(M) && TBAANode(M).isTypeImmutable()) ||
       (isStructPathTBAA(M) && TBAAStructTagNode(M).isTypeImmutable()))
     return ModRefInfo::NoModRef;
 
-  return AAResultBase::getModRefInfoMask(Loc, AAQI, IgnoreLocals);
+  return ModRefInfo::ModRef;
 }
 
 MemoryEffects TypeBasedAAResult::getMemoryEffects(const CallBase *Call,
                                                   AAQueryInfo &AAQI) {
   if (!EnableTBAA)
-    return AAResultBase::getMemoryEffects(Call, AAQI);
+    return MemoryEffects::unknown();
 
   // If this is an "immutable" type, the access is not observable.
   if (const MDNode *M = Call->getMetadata(LLVMContext::MD_tbaa))
     if ((!isStructPathTBAA(M) && TBAANode(M).isTypeImmutable()) ||
         (isStructPathTBAA(M) && TBAAStructTagNode(M).isTypeImmutable()))
       return MemoryEffects::none();
 
-  return AAResultBase::getMemoryEffects(Call, AAQI);
+  return MemoryEffects::unknown();
 }
 
 MemoryEffects TypeBasedAAResult::getMemoryEffects(const Function *F) {
-  // Functions don't have metadata. Just chain to the next implementation.
-  return AAResultBase::getMemoryEffects(F);
+  // Functions don't have metadata.
+  return MemoryEffects::unknown();
 }
 
 ModRefInfo TypeBasedAAResult::getModRefInfo(const CallBase *Call,
                                             const MemoryLocation &Loc,
                                             AAQueryInfo &AAQI) {
   if (!EnableTBAA)
-    return AAResultBase::getModRefInfo(Call, Loc, AAQI);
+    return ModRefInfo::ModRef;
 
   if (const MDNode *L = Loc.AATags.TBAA)
     if (const MDNode *M = Call->getMetadata(LLVMContext::MD_tbaa))
       if (!Aliases(L, M))
         return ModRefInfo::NoModRef;
 
-  return AAResultBase::getModRefInfo(Call, Loc, AAQI);
+  return ModRefInfo::ModRef;
 }
 
 ModRefInfo TypeBasedAAResult::getModRefInfo(const CallBase *Call1,
                                             const CallBase *Call2,
                                             AAQueryInfo &AAQI) {
   if (!EnableTBAA)
-    return AAResultBase::getModRefInfo(Call1, Call2, AAQI);
+    return ModRefInfo::ModRef;
 
   if (const MDNode *M1 = Call1->getMetadata(LLVMContext::MD_tbaa))
     if (const MDNode *M2 = Call2->getMetadata(LLVMContext::MD_tbaa))
       if (!Aliases(M1, M2))
         return ModRefInfo::NoModRef;
 
-  return AAResultBase::getModRefInfo(Call1, Call2, AAQI);
+  return ModRefInfo::ModRef;
 }
 
 bool MDNode::isTBAAVtableAccess() const {
   if (!isStructPathTBAA(this)) {
     if (getNumOperands() < 1)
       return false;
     if (MDString *Tag1 = dyn_cast<MDString>(getOperand(0))) {
       if (Tag1->getString() == "vtable pointer")

llvm/lib/Target/AMDGPU/AMDGPUAliasAnalysis.cpp

if (const LoadInst *LI = dyn_cast<LoadInst>(ObjA)) {
         // TODO: In the regular function, if that local variable in the
         // location B is not captured, that argument pointer won't alias to it
         // as well.
         break;
       }
     }
   }
 
-  // Forward the query to the next alias analysis.
-  return AAResultBase::alias(LocA, LocB, AAQI, nullptr);
+  return AliasResult::MayAlias;
 }
 
 ModRefInfo AMDGPUAAResult::getModRefInfoMask(const MemoryLocation &Loc,
                                              AAQueryInfo &AAQI,
                                              bool IgnoreLocals) {
   unsigned AS = Loc.Ptr->getType()->getPointerAddressSpace();
   if (AS == AMDGPUAS::CONSTANT_ADDRESS ||
       AS == AMDGPUAS::CONSTANT_ADDRESS_32BIT)
     return ModRefInfo::NoModRef;
 
   const Value *Base = getUnderlyingObject(Loc.Ptr);
   AS = Base->getType()->getPointerAddressSpace();
   if (AS == AMDGPUAS::CONSTANT_ADDRESS ||
       AS == AMDGPUAS::CONSTANT_ADDRESS_32BIT)
     return ModRefInfo::NoModRef;
 
-  return AAResultBase::getModRefInfoMask(Loc, AAQI, IgnoreLocals);
+  return ModRefInfo::ModRef;
 }

llvm/lib/Target/NVPTX/NVPTXAliasAnalysis.cpp

ModRefInfo NVPTXAAResult::getModRefInfoMask(const MemoryLocation &Loc,
                                             bool IgnoreLocals) {
   if (isConstOrParam(Loc.Ptr->getType()->getPointerAddressSpace()))
     return ModRefInfo::NoModRef;
 
   const Value *Base = getUnderlyingObject(Loc.Ptr);
   if (isConstOrParam(Base->getType()->getPointerAddressSpace()))
     return ModRefInfo::NoModRef;
 
-  return AAResultBase::getModRefInfoMask(Loc, AAQI, IgnoreLocals);
+  return ModRefInfo::ModRef;
 }