[region-isolation] Make sure not to run RegionAnalysis on functions that we do not support.

Before this commit, this was done at the beginning of TransferNonSendable. I
thought that those checks would be sufficient to ensure that
RegionAnalysisFunctionInfo was not created for functions that we do not
support. Turns out when we perform certain forms of verification, we force all
function analyses to be created for all functions meaning that we would create a
RegionAnalysisFunctionInfo for such an unsupported function causing us to hit
asserts.

In this commit, I move the check to whether or not we support a function into
RegionAnalysisFunctionInfo itself and use that to determine if we should run
TransferNonSendable. This additionally allows me to change
RegionAnalysisFunctionInfo so that one can construct one for an unsupported
function... as long as one doesn't actually touch any of its methods. If one
does, I put in an assert so we will know that operator error has occured.

Author: gottesmm

include/swift/SILOptimizer/Analysis/RegionAnalysis.h

@@ -358,15 +358,25 @@ class RegionAnalysisFunctionInfo {
 
   PostOrderFunctionInfo *pofi;
 
+  /// Set to true if we have already processed our regions.
   bool solved;
 
+  /// Set to true if this is a function that we know how to process regions for.
+  ///
+  /// DISCUSSION: We do not support if the correct features are not enabled, if
+  /// the function doesn't have a parent module, or if the function doesn't have
+  /// ownership.
+  bool supportedFunction;
+
 public:
   using LazyType = LazyFunctionInfo<RegionAnalysis, RegionAnalysisFunctionInfo>;
 
   RegionAnalysisFunctionInfo(SILFunction *fn, PostOrderFunctionInfo *pofi);
   ~RegionAnalysisFunctionInfo();
 
   BlockPartitionState &getPartitionState(SILBasicBlock *block) const {
+    assert(supportedFunction &&
+           "Cannot getPartitionState for a non-supported function");
     // Lazily run the dataflow.
     if (!solved)
       const_cast<RegionAnalysisFunctionInfo *>(this)->runDataflow();
@@ -375,20 +385,46 @@ class RegionAnalysisFunctionInfo {
 
   SILFunction *getFunction() const { return fn; }
 
+  bool isSupportedFunction() const { return supportedFunction; }
+
   using iterator = BasicBlockData::iterator;
   using const_iterator = BasicBlockData::const_iterator;
   using reverse_iterator = BasicBlockData::reverse_iterator;
   using const_reverse_iterator = BasicBlockData::const_reverse_iterator;
 
-  iterator begin() { return blockStates->begin(); }
-  iterator end() { return blockStates->end(); }
-  const_iterator begin() const { return blockStates->begin(); }
-  const_iterator end() const { return blockStates->end(); }
+  iterator begin() {
+    assert(supportedFunction && "Unsupported Function?!");
+    return blockStates->begin();
+  }
+  iterator end() {
+    assert(supportedFunction && "Unsupported Function?!");
+    return blockStates->end();
+  }
+  const_iterator begin() const {
+    assert(supportedFunction && "Unsupported Function?!");
+    return blockStates->begin();
+  }
+  const_iterator end() const {
+    assert(supportedFunction && "Unsupported Function?!");
+    return blockStates->end();
+  }
 
-  reverse_iterator rbegin() { return blockStates->rbegin(); }
-  reverse_iterator rend() { return blockStates->rend(); }
-  const_reverse_iterator rbegin() const { return blockStates->rbegin(); }
-  const_reverse_iterator rend() const { return blockStates->rend(); }
+  reverse_iterator rbegin() {
+    assert(supportedFunction && "Unsupported Function?!");
+    return blockStates->rbegin();
+  }
+  reverse_iterator rend() {
+    assert(supportedFunction && "Unsupported Function?!");
+    return blockStates->rend();
+  }
+  const_reverse_iterator rbegin() const {
+    assert(supportedFunction && "Unsupported Function?!");
+    return blockStates->rbegin();
+  }
+  const_reverse_iterator rend() const {
+    assert(supportedFunction && "Unsupported Function?!");
+    return blockStates->rend();
+  }
 
   using range = llvm::iterator_range<iterator>;
   using const_range = llvm::iterator_range<const_iterator>;
@@ -401,10 +437,14 @@ class RegionAnalysisFunctionInfo {
   const_reverse_range getReverseRange() const { return {rbegin(), rend()}; }
 
   TransferringOperandSetFactory &getOperandSetFactory() {
+    assert(supportedFunction && "Unsupported Function?!");
     return ptrSetFactory;
   }
 
-  RegionAnalysisValueMap &getValueMap() { return valueMap; }
+  RegionAnalysisValueMap &getValueMap() {
+    assert(supportedFunction && "Unsupported Function?!");
+    return valueMap;
+  }
 
   bool isClosureCaptured(SILValue value, Operand *op);
 

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

@@ -2472,11 +2472,44 @@ void BlockPartitionState::print(llvm::raw_ostream &os) const {
 //                         MARK: Dataflow Entrypoint
 //===----------------------------------------------------------------------===//
 
+static bool canComputeRegionsForFunction(SILFunction *fn) {
+  if (!fn->getASTContext().LangOpts.hasFeature(Feature::RegionBasedIsolation))
+    return false;
+
+  // If this function does not correspond to a syntactic declContext and it
+  // doesn't have a parent module, don't check it since we cannot check if a
+  // type is sendable.
+  if (!fn->getDeclContext() && !fn->getParentModule()) {
+    return false;
+  }
+
+  if (!fn->hasOwnership()) {
+    LLVM_DEBUG(llvm::dbgs() << "Only runs on Ownership SSA, skipping!\n");
+    return false;
+  }
+
+  // The sendable protocol should /always/ be available if TransferNonSendable
+  // is enabled. If not, there is a major bug in the compiler and we should
+  // fail loudly.
+  if (!fn->getASTContext().getProtocol(KnownProtocolKind::Sendable))
+    return false;
+
+  return true;
+}
+
 RegionAnalysisFunctionInfo::RegionAnalysisFunctionInfo(
     SILFunction *fn, PostOrderFunctionInfo *pofi)
-    : allocator(), fn(fn),
-      translator(new(allocator) PartitionOpTranslator(fn, pofi, valueMap)),
-      ptrSetFactory(allocator), blockStates(), pofi(pofi), solved(false) {
+    : allocator(), fn(fn), translator(), ptrSetFactory(allocator),
+      blockStates(), pofi(pofi), solved(false), supportedFunction(true) {
+  // Before we do anything, make sure that we support processing this function.
+  //
+  // NOTE: See documentation on supportedFunction for criteria.
+  if (!canComputeRegionsForFunction(fn)) {
+    supportedFunction = false;
+    return;
+  }
+
+  translator = new (allocator) PartitionOpTranslator(fn, pofi, valueMap);
   blockStates.emplace(fn, [this](SILBasicBlock *block) -> BlockPartitionState {
     return BlockPartitionState(block, *translator, ptrSetFactory);
   });
@@ -2491,12 +2524,18 @@ RegionAnalysisFunctionInfo::RegionAnalysisFunctionInfo(
 }
 
 RegionAnalysisFunctionInfo::~RegionAnalysisFunctionInfo() {
+  // If we had a non-supported function, we didn't create a translator, so we do
+  // not need to tear anything down.
+  if (!supportedFunction)
+    return;
+
   // Tear down translator before we tear down the allocator.
   translator->~PartitionOpTranslator();
 }
 
 bool RegionAnalysisFunctionInfo::isClosureCaptured(SILValue value,
                                                    Operand *op) {
+  assert(supportedFunction && "Unsupported Function?!");
   return translator->isClosureCaptured(value, op->getUser());
 }
 

lib/SILOptimizer/Mandatory/TransferNonSendable.cpp

@@ -663,34 +663,18 @@ class TransferNonSendable : public SILFunctionTransform {
   void run() override {
     SILFunction *function = getFunction();
 
-    if (!function->getASTContext().LangOpts.hasFeature(
-            Feature::RegionBasedIsolation))
-      return;
+    auto *functionInfo = getAnalysis<RegionAnalysis>()->get(function);
+    if (!functionInfo->isSupportedFunction()) {
+      LLVM_DEBUG(llvm::dbgs() << "===> SKIPPING UNSUPPORTED FUNCTION: "
+                              << function->getName() << '\n');
 
-    LLVM_DEBUG(llvm::dbgs()
-               << "===> PROCESSING: " << function->getName() << '\n');
-
-    // If this function does not correspond to a syntactic declContext and it
-    // doesn't have a parent module, don't check it since we cannot check if a
-    // type is sendable.
-    if (!function->getDeclContext() && !function->getParentModule()) {
-      LLVM_DEBUG(llvm::dbgs() << "No Decl Context! Skipping!\n");
       return;
     }
 
-    if (!function->hasOwnership()) {
-      LLVM_DEBUG(llvm::dbgs() << "Only runs on Ownership SSA, skipping!\n");
-      return;
-    }
-
-    // The sendable protocol should /always/ be available if TransferNonSendable
-    // is enabled. If not, there is a major bug in the compiler and we should
-    // fail loudly.
-    if (!function->getASTContext().getProtocol(KnownProtocolKind::Sendable))
-      llvm::report_fatal_error("Sendable protocol not available!");
+    LLVM_DEBUG(llvm::dbgs()
+               << "===> PROCESSING: " << function->getName() << '\n');
 
-    auto *analysis = this->getAnalysis<RegionAnalysis>()->get(function);
-    emitDiagnostics(analysis);
+    emitDiagnostics(functionInfo);
   }
 };