[sil][debug-info] Refactor from the SILVerifier structural verification of debug info on SILInstructions into a method on SILInstruction.

The verifier just invokes this method, so we aren't losing any verification in
the SILVerifier itself.

The reason why I am extracting this information into a helper is that often
times one hits these structural assertions in the verifier making one have to
track down where in a pass the bad location was actually inserted. To make these
easier to find, I am going to change the SILBuilder to invoke these structural
comparisons so that we can catch these problems at the call site making it
easier to fix code.

Author: gottesmm

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/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