MemoryLifetimeVerifier: use CalleeCache instead of AliasAnalysis

To verify if a function may read from an indirect argument, don't use AliasAnalysis.
Instead use the CalleeCache to get the list of callees of an apply instruction.
Then use a simple call-back into the swift Function to check if a callee has any relevant memory effect set.

This avoids a dependency from SIL to the Optimizer.
It fixes a linker error when building some unit tests in debug.

Author: eeckstein

SwiftCompilerSources/Sources/SIL/Function.swift

@@ -368,6 +368,24 @@ final public class Function : CustomStringConvertible, HasShortDescription, Hash
       { (f: BridgedFunction, observeRetains: Bool) -> BridgedMemoryBehavior in
         let e = f.function.getSideEffects()
         return e.getMemBehavior(observeRetains: observeRetains)
+      },
+      // argumentMayRead  (used by the MemoryLifetimeVerifier)
+      { (f: BridgedFunction, bridgedArgOp: BridgedOperand, bridgedAddr: BridgedValue) -> Bool in
+        let argOp = Operand(bridged: bridgedArgOp)
+        let addr = bridgedAddr.value
+        let applySite = argOp.instruction as! FullApplySite
+        let addrPath = addr.accessPath
+        let argIdx = argOp.index - ApplyOperands.firstArgumentIndex
+        let calleeArgIdx = applySite.calleeArgIndex(callerArgIndex: argIdx)
+        let convention = applySite.getArgumentConvention(calleeArgIndex: calleeArgIdx)
+        assert(convention.isIndirectIn || convention.isInout)
+        let argPath = argOp.value.accessPath
+        assert(!argPath.isDistinct(from: addrPath))
+        let path = argPath.getProjection(to: addrPath) ?? SmallProjectionPath()
+        let effects = f.function.getSideEffects(forArgument: argOp.value.at(path),
+                                                atIndex: calleeArgIdx,
+                                                withConvention: convention)
+        return effects.memory.read
       }
     )
   }

include/swift/SIL/SILBridging.h

@@ -373,13 +373,15 @@ struct BridgedFunction {
   typedef SwiftInt (* _Nonnull CopyEffectsFn)(BridgedFunction, BridgedFunction);
   typedef EffectInfo (* _Nonnull GetEffectInfoFn)(BridgedFunction, SwiftInt);
   typedef BridgedMemoryBehavior (* _Nonnull GetMemBehaviorFn)(BridgedFunction, bool);
+  typedef bool (* _Nonnull ArgumentMayReadFn)(BridgedFunction, BridgedOperand, BridgedValue);
 
   static void registerBridging(SwiftMetatype metatype,
               RegisterFn initFn, RegisterFn destroyFn,
               WriteFn writeFn, ParseFn parseFn,
               CopyEffectsFn copyEffectsFn,
               GetEffectInfoFn effectInfoFn,
-              GetMemBehaviorFn memBehaviorFn);
+              GetMemBehaviorFn memBehaviorFn,
+              ArgumentMayReadFn argumentMayReadFn);
 };
 
 struct OptionalBridgedFunction {

include/swift/SIL/SILFunction.h

@@ -39,7 +39,7 @@ class SILProfiler;
 class BasicBlockBitfield;
 class NodeBitfield;
 class OperandBitfield;
-class SILPassManager;
+class CalleeCache;
 
 namespace Lowering {
 class TypeLowering;
@@ -1141,6 +1141,9 @@ class SILFunction
   void visitArgEffects(std::function<void(int, int, bool)> c) const;
   MemoryBehavior getMemoryBehavior(bool observeRetains);
 
+  // Used by the MemoryLifetimeVerifier
+  bool argumentMayRead(Operand *argOp, SILValue addr);
+
   Purpose getSpecialPurpose() const { return specialPurpose; }
 
   /// Get this function's global_init attribute.
@@ -1497,19 +1500,19 @@ class SILFunction
 
   /// verify - Run the SIL verifier to make sure that the SILFunction follows
   /// invariants.
-  void verify(SILPassManager *passManager = nullptr,
+  void verify(CalleeCache *calleeCache = nullptr,
               bool SingleFunction = true,
               bool isCompleteOSSA = true,
               bool checkLinearLifetime = true) const;
 
   /// Run the SIL verifier without assuming OSSA lifetimes end at dead end
   /// blocks.
   void verifyIncompleteOSSA() const {
-    verify(/*passManager*/nullptr, /*SingleFunction=*/true, /*completeOSSALifetimes=*/false);
+    verify(/*calleeCache*/nullptr, /*SingleFunction=*/true, /*completeOSSALifetimes=*/false);
   }
 
   /// Verifies the lifetime of memory locations in the function.
-  void verifyMemoryLifetime(SILPassManager *passManager);
+  void verifyMemoryLifetime(CalleeCache *calleeCache);
 
   /// Run the SIL ownership verifier to check that all values with ownership
   /// have a linear lifetime. Regular OSSA invariants are checked separately in

include/swift/SIL/SILModule.h

@@ -926,7 +926,7 @@ class SILModule {
 
   /// Run the SIL verifier to make sure that all Functions follow
   /// invariants.
-  void verify(SILPassManager *passManager,
+  void verify(CalleeCache *calleeCache,
               bool isCompleteOSSA = true,
               bool checkLinearLifetime = true) const;
 

include/swift/SILOptimizer/Analysis/BasicCalleeAnalysis.h

@@ -97,6 +97,8 @@ class BasicCalleeAnalysis : public SILAnalysis {
   }
 
   MemoryBehavior getMemoryBehavior(ApplySite as, bool observeRetains);
+
+  CalleeCache *getCalleeCache() { return Cache.get(); }
 };
 
 bool isDeinitBarrier(SILInstruction *const instruction,

lib/SIL/IR/SILFunction.cpp

@@ -146,6 +146,7 @@ static BridgedFunction::ParseFn parseFunction = nullptr;
 static BridgedFunction::CopyEffectsFn copyEffectsFunction = nullptr;
 static BridgedFunction::GetEffectInfoFn getEffectInfoFunction = nullptr;
 static BridgedFunction::GetMemBehaviorFn getMemBehvaiorFunction = nullptr;
+static BridgedFunction::ArgumentMayReadFn argumentMayReadFunction = nullptr;
 
 SILFunction::SILFunction(
     SILModule &Module, SILLinkage Linkage, StringRef Name,
@@ -1010,7 +1011,8 @@ void BridgedFunction::registerBridging(SwiftMetatype metatype,
             WriteFn writeFn, ParseFn parseFn,
             CopyEffectsFn copyEffectsFn,
             GetEffectInfoFn effectInfoFn,
-            GetMemBehaviorFn memBehaviorFn) {
+            GetMemBehaviorFn memBehaviorFn,
+            ArgumentMayReadFn argumentMayReadFn) {
   functionMetatype = metatype;
   initFunction = initFn;
   destroyFunction = destroyFn;
@@ -1019,6 +1021,7 @@ void BridgedFunction::registerBridging(SwiftMetatype metatype,
   copyEffectsFunction = copyEffectsFn;
   getEffectInfoFunction = effectInfoFn;
   getMemBehvaiorFunction = memBehaviorFn;
+  argumentMayReadFunction = argumentMayReadFn;
 }
 
 std::pair<const char *, int>  SILFunction::
@@ -1111,3 +1114,11 @@ MemoryBehavior SILFunction::getMemoryBehavior(bool observeRetains) {
   auto b = getMemBehvaiorFunction({this}, observeRetains);
   return (MemoryBehavior)b;
 }
+
+// Used by the MemoryLifetimeVerifier
+bool SILFunction::argumentMayRead(Operand *argOp, SILValue addr) {
+  if (!argumentMayReadFunction)
+    return true;
+
+  return argumentMayReadFunction({this}, {argOp}, {addr});
+}

lib/SIL/Verifier/MemoryLifetimeVerifier.cpp

@@ -13,12 +13,11 @@
 #define DEBUG_TYPE "sil-memory-lifetime-verifier"
 #include "swift/SIL/MemoryLocations.h"
 #include "swift/SIL/BitDataflow.h"
+#include "swift/SIL/CalleeCache.h"
 #include "swift/SIL/SILBasicBlock.h"
 #include "swift/SIL/SILFunction.h"
 #include "swift/SIL/ApplySite.h"
 #include "swift/SIL/BasicBlockDatastructures.h"
-#include "swift/SILOptimizer/Analysis/AliasAnalysis.h"
-#include "swift/SILOptimizer/PassManager/PassManager.h"
 #include "llvm/Support/CommandLine.h"
 
 using namespace swift;
@@ -42,7 +41,7 @@ class MemoryLifetimeVerifier {
   using BlockState = BitDataflow::BlockState;
 
   SILFunction *function;
-  AliasAnalysis *aliasAnalysis;
+  CalleeCache *calleeCache;
   MemoryLocations locations;
 
   /// alloc_stack memory locations which are used for store_borrow.
@@ -80,7 +79,9 @@ class MemoryLifetimeVerifier {
   /// \p addr, are set in \p bits.
   void requireBitsSet(const Bits &bits, SILValue addr, SILInstruction *where);
 
-  void requireBitsSetForArgument(const Bits &bits, SILValue addr, SILInstruction *applyInst);
+  void requireBitsSetForArgument(const Bits &bits, Operand *argOp);
+
+  bool applyMayRead(Operand *argOp, SILValue addr);
 
   bool isStoreBorrowLocation(SILValue addr) {
     auto *loc = locations.getLocation(addr);
@@ -137,10 +138,9 @@ class MemoryLifetimeVerifier {
   }
 
 public:
-  MemoryLifetimeVerifier(SILFunction *function, SILPassManager *passManager) :
+  MemoryLifetimeVerifier(SILFunction *function, CalleeCache *calleeCache) :
     function(function),
-    aliasAnalysis(passManager ? passManager->getAnalysis<AliasAnalysis>(function)
-                              : nullptr),
+    calleeCache(calleeCache),
     locations(/*handleNonTrivialProjections*/ true,
               /*handleTrivialLocations*/ true) {}
 
@@ -277,30 +277,41 @@ void MemoryLifetimeVerifier::requireBitsSet(const Bits &bits, SILValue addr,
   }
 }
 
-void MemoryLifetimeVerifier::requireBitsSetForArgument(const Bits &bits, SILValue addr,
-                                           SILInstruction *applyInst) {
-  // Optimizations can rely on alias analysis to know that an in-argument (or
-  // parts of it) is not actually read.
-  // We have to do the same in the verifier: if alias analysis says that an in-
-  // argument is not read, there is no need that the memory location is initialized.
-
-  // Not all calls to the verifier provide the alias analysis.
-  if (!aliasAnalysis)
-    return;
-
-  if (auto *loc = locations.getLocation(addr)) {
+void MemoryLifetimeVerifier::requireBitsSetForArgument(const Bits &bits, Operand *argOp) {
+  if (auto *loc = locations.getLocation(argOp->get())) {
     Bits missingBits = ~bits & loc->subLocations;
     for (int errorLocIdx = missingBits.find_first(); errorLocIdx >= 0;
          errorLocIdx = missingBits.find_next(errorLocIdx)) {
       auto *errorLoc = locations.getLocation(errorLocIdx);
-      if (aliasAnalysis->mayReadFromMemory(applyInst, errorLoc->representativeValue)) {
+
+      if (applyMayRead(argOp, errorLoc->representativeValue)) {
         reportError("memory is not initialized, but should be",
-                    errorLocIdx, applyInst);
+                    errorLocIdx, argOp->getUser());
       }
     }
   }
 }
 
+bool MemoryLifetimeVerifier::applyMayRead(Operand *argOp, SILValue addr) {
+  FullApplySite as(argOp->getUser());
+  CalleeList callees;
+  if (calleeCache) {
+    callees = calleeCache->getCalleeList(as);
+    if (callees.isIncomplete())
+      return true;
+  } else if (auto *callee = as.getReferencedFunctionOrNull()) {
+    callees = CalleeList(callee);
+  } else {
+    return false;
+  }
+
+  for (SILFunction *callee : callees) {
+    if (callee->argumentMayRead(argOp, addr))
+      return true;
+  }
+  return false;
+}
+
 void MemoryLifetimeVerifier::requireNoStoreBorrowLocation(
     SILValue addr, SILInstruction *where) {
   if (isa<StoreBorrowInst>(addr)) {
@@ -864,7 +875,7 @@ void MemoryLifetimeVerifier::checkFuncArgument(Bits &bits, Operand &argumentOp,
 
   switch (argumentConvention) {
     case SILArgumentConvention::Indirect_In:
-      requireBitsSetForArgument(bits, argumentOp.get(), applyInst);
+      requireBitsSetForArgument(bits, &argumentOp);
       locations.clearBits(bits, argumentOp.get());
       break;
     case SILArgumentConvention::Indirect_Out:
@@ -874,7 +885,7 @@ void MemoryLifetimeVerifier::checkFuncArgument(Bits &bits, Operand &argumentOp,
       break;
     case SILArgumentConvention::Indirect_In_Guaranteed:
     case SILArgumentConvention::Indirect_Inout:
-      requireBitsSetForArgument(bits, argumentOp.get(), applyInst);
+      requireBitsSetForArgument(bits, &argumentOp);
       break;
     case SILArgumentConvention::Indirect_InoutAliasable:
       // We don't require any locations to be initialized for a partial_apply
@@ -883,7 +894,7 @@ void MemoryLifetimeVerifier::checkFuncArgument(Bits &bits, Operand &argumentOp,
       // closures capture the whole "self". When this is done in an initializer
       // it can happen that not all fields of "self" are initialized, yet.
       if (!isa<PartialApplyInst>(applyInst))
-        requireBitsSetForArgument(bits, argumentOp.get(), applyInst);
+        requireBitsSetForArgument(bits, &argumentOp);
       break;
     case SILArgumentConvention::Direct_Owned:
     case SILArgumentConvention::Direct_Unowned:
@@ -919,7 +930,7 @@ void MemoryLifetimeVerifier::verify() {
 
 } // anonymous namespace
 
-void SILFunction::verifyMemoryLifetime(SILPassManager *passManager) {
-  MemoryLifetimeVerifier verifier(this, passManager);
+void SILFunction::verifyMemoryLifetime(CalleeCache *calleeCache) {
+  MemoryLifetimeVerifier verifier(this, calleeCache);
   verifier.verify();
 }

lib/SIL/Verifier/SILVerifier.cpp

@@ -29,6 +29,7 @@
 #include "swift/SIL/ApplySite.h"
 #include "swift/SIL/BasicBlockBits.h"
 #include "swift/SIL/BasicBlockUtils.h"
+#include "swift/SIL/CalleeCache.h"
 #include "swift/SIL/DebugUtils.h"
 #include "swift/SIL/Dominance.h"
 #include "swift/SIL/DynamicCasts.h"
@@ -745,7 +746,7 @@ static void checkAddressWalkerCanVisitAllTransitiveUses(SILValue address) {
 class SILVerifier : public SILVerifierBase<SILVerifier> {
   ModuleDecl *M;
   const SILFunction &F;
-  SILPassManager *passManager;
+  CalleeCache *calleeCache;
   SILFunctionConventions fnConv;
   Lowering::TypeConverter &TC;
 
@@ -1015,10 +1016,10 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
     return numInsts;
   }
 
-  SILVerifier(const SILFunction &F, SILPassManager *passManager,
+  SILVerifier(const SILFunction &F, CalleeCache *calleeCache,
               bool SingleFunction, bool checkLinearLifetime)
       : M(F.getModule().getSwiftModule()), F(F),
-        passManager(passManager),
+        calleeCache(calleeCache),
         fnConv(F.getConventionsInContext()), TC(F.getModule().Types),
         SingleFunction(SingleFunction),
         checkLinearLifetime(checkLinearLifetime),
@@ -6796,7 +6797,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
 
     if (F->hasOwnership() && F->shouldVerifyOwnership() &&
         !mod.getASTContext().hadError()) {
-      F->verifyMemoryLifetime(passManager);
+      F->verifyMemoryLifetime(calleeCache);
     }
   }
 
@@ -6836,7 +6837,7 @@ static bool verificationEnabled(const SILModule &M) {
 
 /// verify - Run the SIL verifier to make sure that the SILFunction follows
 /// invariants.
-void SILFunction::verify(SILPassManager *passManager,
+void SILFunction::verify(CalleeCache *calleeCache,
                          bool SingleFunction, bool isCompleteOSSA,
                          bool checkLinearLifetime) const {
   if (!verificationEnabled(getModule()))
@@ -6845,15 +6846,15 @@ void SILFunction::verify(SILPassManager *passManager,
   // Please put all checks in visitSILFunction in SILVerifier, not here. This
   // ensures that the pretty stack trace in the verifier is included with the
   // back trace when the verifier crashes.
-  SILVerifier verifier(*this, passManager, SingleFunction, checkLinearLifetime);
+  SILVerifier verifier(*this, calleeCache, SingleFunction, checkLinearLifetime);
   verifier.verify(isCompleteOSSA);
 }
 
 void SILFunction::verifyCriticalEdges() const {
   if (!verificationEnabled(getModule()))
     return;
 
-  SILVerifier(*this, /*passManager=*/nullptr,
+  SILVerifier(*this, /*calleeCache=*/nullptr,
                      /*SingleFunction=*/true,
                      /*checkLinearLifetime=*/ false).verifyBranches(this);
 }
@@ -6973,7 +6974,7 @@ void SILVTable::verify(const SILModule &M) const {
 
     if (M.getStage() != SILStage::Lowered &&
         !M.getASTContext().LangOpts.hasFeature(Feature::Embedded)) {
-      SILVerifier(*entry.getImplementation(), /*passManager=*/nullptr,
+      SILVerifier(*entry.getImplementation(), /*calleeCache=*/nullptr,
                                               /*SingleFunction=*/true,
                                               /*checkLinearLifetime=*/ false)
           .requireABICompatibleFunctionTypes(
@@ -7118,12 +7119,12 @@ void SILGlobalVariable::verify() const {
 }
 
 void SILModule::verify(bool isCompleteOSSA, bool checkLinearLifetime) const {
-  SILPassManager pm(const_cast<SILModule *>(this), /*isMandatory=*/false, /*IRMod=*/nullptr);
-  verify(&pm, isCompleteOSSA, checkLinearLifetime);
+  CalleeCache calleeCache(*const_cast<SILModule *>(this));
+  verify(&calleeCache, isCompleteOSSA, checkLinearLifetime);
 }
 
 /// Verify the module.
-void SILModule::verify(SILPassManager *passManager,
+void SILModule::verify(CalleeCache *calleeCache,
                        bool isCompleteOSSA, bool checkLinearLifetime) const {
   if (!verificationEnabled(*this))
     return;
@@ -7139,7 +7140,7 @@ void SILModule::verify(SILPassManager *passManager,
       llvm::errs() << "Symbol redefined: " << f.getName() << "!\n";
       assert(false && "triggering standard assertion failure routine");
     }
-    f.verify(passManager, /*singleFunction*/ false, isCompleteOSSA, checkLinearLifetime);
+    f.verify(calleeCache, /*singleFunction*/ false, isCompleteOSSA, checkLinearLifetime);
   }
 
   // Check all globals.

lib/SILOptimizer/LoopTransforms/ArrayBoundsCheckOpts.cpp

@@ -24,6 +24,7 @@
 #include "swift/SILOptimizer/Analysis/AliasAnalysis.h"
 #include "swift/SILOptimizer/Analysis/Analysis.h"
 #include "swift/SILOptimizer/Analysis/ArraySemantic.h"
+#include "swift/SILOptimizer/Analysis/BasicCalleeAnalysis.h"
 #include "swift/SILOptimizer/Analysis/DestructorAnalysis.h"
 #include "swift/SILOptimizer/Analysis/DominanceAnalysis.h"
 #include "swift/SILOptimizer/Analysis/IVAnalysis.h"
@@ -1319,7 +1320,7 @@ bool ABCOpt::processLoop(SILLoop *Loop) {
   Changed |= hoistChecksInLoop(DT->getNode(Header), ABC, IndVars, Preheader,
                                Header, SingleExitingBlk, /*recursionDepth*/ 0);
   if (Changed) {
-    Preheader->getParent()->verify(getPassManager());
+    Preheader->getParent()->verify(getAnalysis<BasicCalleeAnalysis>()->getCalleeCache());
   }
   return Changed;
 }