[sil][debug-info] Refactor SILInstruction SILLocation verification out of SILVerifier onto a helper/use to verify in SILBuilder

include/swift/SIL/SILBuilder.h

@@ -2622,6 +2622,11 @@ class SILBuilder {
     C.notifyInserted(TheInst);
 
 #ifndef NDEBUG
+    // If we are inserting into a specific function (rather than a block for a
+    // global_addr), verify that our instruction/the associated location are in
+    // sync. We don't care if an instruction is used in global_addr.
+    if (F)
+      TheInst->verifyDebugInfo();
     TheInst->verifyOperandOwnership();
 #endif
   }

include/swift/SIL/SILDebugScope.h

@@ -95,7 +95,7 @@ class SILDebugScope : public SILAllocated<SILDebugScope> {
 };
 
 /// Determine whether an instruction may not have a SILDebugScope.
-bool maybeScopeless(SILInstruction &I);
+bool maybeScopeless(const SILInstruction &inst);
 
 /// Knows how to make a deep copy of a debug scope.
 class ScopeCloner {

include/swift/SIL/SILInstruction.h

@@ -733,6 +733,10 @@ class SILInstruction : public llvm::ilist_node<SILInstruction> {
   /// with this instruction.
   void verifyOperandOwnership() const;
 
+  /// Verify that this instruction and its associated debug information follow
+  /// all SIL debug info invariants.
+  void verifyDebugInfo() const;
+
   /// Get the number of created SILInstructions.
   static int getNumCreatedInstructions() {
     return NumCreatedInstructions;

lib/SIL/IR/SILDebugScope.cpp

@@ -57,8 +57,8 @@ SILFunction *SILDebugScope::getParentFunction() const {
 }
 
 /// Determine whether an instruction may not have a SILDebugScope.
-bool swift::maybeScopeless(SILInstruction &I) {
-  if (I.getFunction()->isBare())
+bool swift::maybeScopeless(const SILInstruction &inst) {
+  if (inst.getFunction()->isBare())
     return true;
-  return !isa<DebugValueInst>(I) && !isa<DebugValueAddrInst>(I);
+  return !isa<DebugValueInst>(inst) && !isa<DebugValueAddrInst>(inst);
 }

lib/SIL/Verifier/CMakeLists.txt

@@ -1,4 +1,5 @@
 target_sources(swiftSIL PRIVATE
+  DebugInfoVerifier.cpp
   LoadBorrowImmutabilityChecker.cpp
   LinearLifetimeChecker.cpp
   MemoryLifetimeVerifier.cpp

lib/SIL/Verifier/DebugInfoVerifier.cpp

@@ -0,0 +1,57 @@
+//===--- DebugInfoVerifier.cpp --------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2021 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+///
+/// Utility verifier code for validating debug info.
+///
+//===----------------------------------------------------------------------===//
+
+#include "swift/SIL/SILDebugScope.h"
+#include "swift/SIL/SILInstruction.h"
+
+using namespace swift;
+
+//===----------------------------------------------------------------------===//
+//                   MARK: Verify SILInstruction Debug Info
+//===----------------------------------------------------------------------===//
+
+void SILInstruction::verifyDebugInfo() const {
+  auto require = [&](bool reqt, StringRef message) {
+    if (!reqt) {
+      llvm::errs() << message << "\n";
+      assert(false && "invoking standard assertion failure");
+    }
+  };
+
+  // Check the location kind.
+  SILLocation loc = getLoc();
+  SILLocation::LocationKind locKind = loc.getKind();
+  SILInstructionKind instKind = getKind();
+
+  // Regular locations are allowed on all instructions.
+  if (locKind == SILLocation::RegularKind)
+    return;
+
+  if (locKind == SILLocation::ReturnKind ||
+      locKind == SILLocation::ImplicitReturnKind)
+    require(
+        instKind == SILInstructionKind::BranchInst ||
+            instKind == SILInstructionKind::ReturnInst ||
+            instKind == SILInstructionKind::UnreachableInst,
+        "return locations are only allowed on branch and return instructions");
+
+  if (locKind == SILLocation::ArtificialUnreachableKind)
+    require(
+        instKind == SILInstructionKind::UnreachableInst,
+        "artificial locations are only allowed on Unreachable instructions");
+}

lib/SIL/Verifier/SILVerifier.cpp

@@ -1204,67 +1204,55 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
     }
   }
 
-  void checkInstructionsSILLocation(SILInstruction *I) {
-    // Check the debug scope.
-    auto *DS = I->getDebugScope();
-    if (DS && !maybeScopeless(*I))
-      require(DS, "instruction has a location, but no scope");
+  void checkInstructionsSILLocation(SILInstruction *inst) {
+    // First verify structural debug info information.
+    inst->verifyDebugInfo();
 
-    require(!DS || DS->getParentFunction() == I->getFunction(),
+    // Check the debug scope.
+    auto *debugScope = inst->getDebugScope();
+    if (debugScope && !maybeScopeless(*inst))
+      require(debugScope, "instruction has a location, but no scope");
+    require(!debugScope ||
+                debugScope->getParentFunction() == inst->getFunction(),
             "debug scope of instruction belongs to a different function");
 
-    // Check the location kind.
-    SILLocation L = I->getLoc();
-    SILLocation::LocationKind LocKind = L.getKind();
-    SILInstructionKind InstKind = I->getKind();
-
-    // Check that there is at most one debug variable defined
-    // for each argument slot. This catches SIL transformations
-    // that accidentally remove inline information (stored in the SILDebugScope)
-    // from debug-variable-carrying instructions.
-    if (!DS->InlinedCallSite) {
-      Optional<SILDebugVariable> VarInfo;
-      if (auto *DI = dyn_cast<AllocStackInst>(I))
-        VarInfo = DI->getVarInfo();
-      else if (auto *DI = dyn_cast<AllocBoxInst>(I))
-        VarInfo = DI->getVarInfo();
-      else if (auto *DI = dyn_cast<DebugValueInst>(I))
-        VarInfo = DI->getVarInfo();
-      else if (auto *DI = dyn_cast<DebugValueAddrInst>(I))
-        VarInfo = DI->getVarInfo();
-
-      if (VarInfo)
-        if (unsigned ArgNo = VarInfo->ArgNo) {
-          // It is a function argument.
-          if (ArgNo < DebugVars.size() && !DebugVars[ArgNo].empty() && !VarInfo->Name.empty()) {
-            require(
-                DebugVars[ArgNo] == VarInfo->Name,
+    // Check that there is at most one debug variable defined for each argument
+    // slot if our debug scope is not an inlined call site.
+    //
+    // This catches SIL transformations that accidentally remove inline
+    // information (stored in the SILDebugScope) from debug-variable-carrying
+    // instructions.
+    if (debugScope->InlinedCallSite)
+      return;
+
+    Optional<SILDebugVariable> varInfo;
+    if (auto *di = dyn_cast<AllocStackInst>(inst))
+      varInfo = di->getVarInfo();
+    else if (auto *di = dyn_cast<AllocBoxInst>(inst))
+      varInfo = di->getVarInfo();
+    else if (auto *di = dyn_cast<DebugValueInst>(inst))
+      varInfo = di->getVarInfo();
+    else if (auto *di = dyn_cast<DebugValueAddrInst>(inst))
+      varInfo = di->getVarInfo();
+
+    if (!varInfo)
+      return;
+
+    if (unsigned argNum = varInfo->ArgNo) {
+      // It is a function argument.
+      if (argNum < DebugVars.size() && !DebugVars[argNum].empty() &&
+          !varInfo->Name.empty()) {
+        require(DebugVars[argNum] == varInfo->Name,
                 "Scope contains conflicting debug variables for one function "
                 "argument");
-          } else {
-            // Reserve enough space.
-            while (DebugVars.size() <= ArgNo) {
-              DebugVars.push_back(StringRef());
-            }
-          }
-          DebugVars[ArgNo] = VarInfo->Name;
+      } else {
+        // Reserve enough space.
+        while (DebugVars.size() <= argNum) {
+          DebugVars.push_back(StringRef());
+        }
       }
+      DebugVars[argNum] = varInfo->Name;
     }
-
-    // Regular locations are allowed on all instructions.
-    if (LocKind == SILLocation::RegularKind)
-      return;
-
-    if (LocKind == SILLocation::ReturnKind ||
-        LocKind == SILLocation::ImplicitReturnKind)
-      require(InstKind == SILInstructionKind::BranchInst ||
-              InstKind == SILInstructionKind::ReturnInst ||
-              InstKind == SILInstructionKind::UnreachableInst,
-        "return locations are only allowed on branch and return instructions");
-
-    if (LocKind == SILLocation::ArtificialUnreachableKind)
-      require(InstKind == SILInstructionKind::UnreachableInst,
-        "artificial locations are only allowed on Unreachable instructions");
   }
 
   /// Check that the types of this value producer are all legal in the function