Merge pull request #235 from frasercrmck/spirv-ll-debug-info-scopes

[spirv-ll] Refactor how debug ranges/scopes are handled

modules/compiler/spirv-ll/include/spirv-ll/builder.h

@@ -21,6 +21,7 @@
 
 #include <llvm/ADT/StringSwitch.h>
 #include <llvm/IR/DIBuilder.h>
+#include <llvm/IR/DebugInfoMetadata.h>
 #include <llvm/IR/IRBuilder.h>
 #include <multi_llvm/multi_llvm.h>
 #include <multi_llvm/vector_type_helper.h>
@@ -230,8 +231,57 @@ class Builder {
   /// being translated
   void accessChain(OpCode const &opc);
 
-  /// @brief Check if an OpLine range is in progress and end it if there is
-  void checkEndOpLineRange();
+  /// @brief Represents a lexical scope, used for debug information.
+  struct LexicalScopeTy {
+    /// @brief The scope, represented in LLVM metadata; could be a function or
+    /// block scope but is not specified here. Must not be nullptr in a valid
+    /// scope.
+    llvm::Metadata *scope = nullptr;
+    /// @brief An optional scope, representing where the scope was inlined. May
+    /// be nullptr.
+    llvm::Metadata *inlined_at = nullptr;
+  };
+
+  /// @brief Get the currently active debug scope in the current function the
+  /// builder is currently working on
+  ///
+  /// @return The function if one has been declared, otherwise std::nullopt
+  std::optional<LexicalScopeTy> getCurrentFunctionLexicalScope() const;
+
+  /// @brief Set the currently active lexical scope in the current function the
+  /// builder is currently working on; std::nullopt signals no active scope, or
+  /// the closing of an open one.
+  void setCurrentFunctionLexicalScope(std::optional<LexicalScopeTy>);
+
+  /// @brief Called at the end of a lexical scope for book-keeping.
+  /// @param closing_line_range True if any open line range should be closed at
+  /// the same time.
+  void closeCurrentLexicalScope(bool closing_line_range = true);
+
+  /// @brief A type containing an OpLine line range and the beginning of the
+  /// range it corresponds to.
+  struct LineRangeBeginTy {
+    /// @brief A pointer to the OpLine that this line range corresponds to.
+    const OpLine *op_line = nullptr;
+    /// @brief An optional iterator pointing to the first instruction the range
+    /// applies to. Ranges may be open before a block has begun, in which case
+    /// this will be std::nullopt.
+    std::optional<llvm::BasicBlock::iterator> range_begin = std::nullopt;
+  };
+
+  /// @brief Get the currently active debug scope in the current function the
+  /// builder is currently working on
+  ///
+  /// @return The function if one has been declared, otherwise std::nullopt
+  std::optional<LineRangeBeginTy> getCurrentOpLineRange() const;
+
+  /// @brief Set the currently active OpLine range; std::nullopt signals no
+  /// active OpLine range, or the closing of an open one.
+  void setCurrentOpLineRange(std::optional<LineRangeBeginTy>);
+
+  /// @brief At the closing of a scope, apply debug information to instructions
+  /// within the closed scope.
+  void applyDebugInfoAtClosedRangeOrScope();
 
   /// @brief Return a `DIType` object that represents the given type
   ///
@@ -248,13 +298,13 @@ class Builder {
 
   /// @brief Gets (or creates) a DISubprogram for the given function and
   /// OpLine.
-  llvm::DISubprogram *getOrCreateDebugFunctionScope(llvm::Function *function,
+  llvm::DISubprogram *getOrCreateDebugFunctionScope(llvm::Function &function,
                                                     const OpLine *op_line);
 
-  /// @brief Creates the beginning of a line range for the given OpLine,
-  /// contained within the basic block.
-  Module::LineRangeBeginTy createLineRangeBegin(const OpLine *op_line,
-                                                llvm::BasicBlock &bb);
+  /// @brief Gets (or creates) a DILexicalBlock for the given function and
+  /// OpLine.
+  llvm::DILexicalBlock *getOrCreateDebugBasicBlockScope(llvm::BasicBlock &bb,
+                                                        const OpLine *op_line);
 
   /// @brief Called once all instructions in the module have been visited in
   /// order during the first pass through the SPIR-V binary.
@@ -756,6 +806,12 @@ class Builder {
   llvm::Function *CurrentFunction;
   /// @brief A copy of the current function's argument list
   llvm::SmallVector<llvm::Value *, 8> CurrentFunctionArgs;
+  /// @brief Current debug scope of the function the builder is currently
+  /// working on (or std::nullopt if no debug scope is active)
+  std::optional<LexicalScopeTy> CurrentFunctionLexicalScope;
+  /// @brief Current line range - marked by the beginning of an OpLine
+  /// instruction - (or std::nullopt if no line range is active)
+  std::optional<LineRangeBeginTy> CurrentOpLineRange;
   /// @brief A list of the builtin IDs specified at `CurrentFunction`'s creation
   BuiltinIDList CurrentFunctionBuiltinIDs;
   /// @brief Registered extended instruction set handlers.

modules/compiler/spirv-ll/include/spirv-ll/module.h

@@ -395,50 +395,6 @@ class Module : public ModuleHeader {
   /// @return The string or std::nullopt if the ID isn't found.
   std::optional<std::string> getDebugString(spv::Id id) const;
 
-  /// @brief A type containing an LLVM debug location and the beginning of the
-  /// range it corresponds to.
-  struct LineRangeBeginTy {
-    const OpLine *op_line;
-    llvm::DILocation *loc;
-    llvm::BasicBlock::iterator range_begin = llvm::BasicBlock::iterator();
-  };
-
-  /// @brief Opens up a new OpLine range, setting it to the current one.
-  ///
-  /// Does *not* close the current one. Call closeCurrentOpLineRange() first.
-  ///
-  /// @param range_begin LineRangeBeginTy information about the range to begin.
-  void setCurrentOpLineRange(const LineRangeBeginTy &range_begin);
-
-  /// @brief Closes (i.e., clears) the current OpLine range.
-  void closeCurrentOpLineRange();
-
-  /// @brief Add a completed OpLine range to the module.
-  ///
-  /// @param location DILocation that represents the OpLine instruction.
-  /// @param range Iterator range over which the debug metadata will be applied.
-  void addCompleteOpLineRange(
-      llvm::DILocation *location,
-      std::pair<llvm::BasicBlock::iterator, llvm::BasicBlock::iterator> range);
-
-  /// @brief A list of basic block ranges (begin/end).
-  using OpLineRangeVec = std::vector<
-      std::pair<llvm::BasicBlock::iterator, llvm::BasicBlock::iterator>>;
-
-  /// @brief A map from a debug location to the list of ranges it covers.
-  using OpLineRangeMap = llvm::MapVector<llvm::DILocation *, OpLineRangeVec>;
-
-  /// @brief Get reference to complete OpLine range list.
-  ///
-  /// @return Reference to map of `DILocation`/iterator range pairs
-  OpLineRangeMap &getOpLineRanges();
-
-  /// @brief Get `DILocation`/iterator pair for current OpLine range
-  ///
-  /// @return Pair containing `DILocation` and iterator range, location will be
-  /// nullptr if there isn't an ongoing range
-  std::optional<LineRangeBeginTy> getCurrentOpLineRange() const;
-
   /// @brief Add a basic block and associated lexical block to the module.
   ///
   /// @param b_block Basic block to associate the lexical block with.
@@ -1047,16 +1003,6 @@ class Module : public ModuleHeader {
   llvm::DenseMap<spv::Id, std::string> DebugStrings;
   /// @brief `DIFile` object specified by the module currently being translated.
   llvm::DIFile *File;
-  /// @brief Map of DILocation to sets of basic block iterator ranges.
-  ///
-  /// Storing `std::pair`s of iterators instead of `llvm::iterator_range`
-  /// because the iterators need to be manipulated after the module has been
-  /// translated in its entirety, so we can't construct the `iterator_range`
-  /// until that's happened but we still need to store the range.
-  OpLineRangeMap OpLineRanges;
-  /// @brief DILocation/basic block iterator pair to store the beginning of the
-  /// current OpLine range.
-  std::optional<LineRangeBeginTy> CurrentOpLineRange;
   /// @brief Map of BasicBlock to associated `DILexicalBlock`.
   llvm::DenseMap<llvm::BasicBlock *, llvm::DILexicalBlock *> LexicalBlocks;
   /// @brief Map of function IDs to their associated `DISubprogram`s.

modules/compiler/spirv-ll/source/builder.cpp

@@ -36,7 +36,8 @@ spirv_ll::Builder::Builder(spirv_ll::Context &context, spirv_ll::Module &module,
       deviceInfo(deviceInfo),
       IRBuilder(*context.llvmContext),
       DIBuilder(*module.llvmModule),
-      CurrentFunction(nullptr) {}
+      CurrentFunction(nullptr),
+      CurrentFunctionLexicalScope(std::nullopt) {}
 
 llvm::IRBuilder<> &spirv_ll::Builder::getIRBuilder() { return IRBuilder; }
 
@@ -155,26 +156,32 @@ llvm::Error spirv_ll::Builder::finishModuleProcessing() {
   return llvm::Error::success();
 }
 
+std::optional<spirv_ll::Builder::LexicalScopeTy>
+spirv_ll::Builder::getCurrentFunctionLexicalScope() const {
+  return CurrentFunctionLexicalScope;
+}
+
+void spirv_ll::Builder::setCurrentFunctionLexicalScope(
+    std::optional<LexicalScopeTy> scope) {
+  CurrentFunctionLexicalScope = scope;
+}
+
+std::optional<spirv_ll::Builder::LineRangeBeginTy>
+spirv_ll::Builder::getCurrentOpLineRange() const {
+  return CurrentOpLineRange;
+}
+
+void spirv_ll::Builder::setCurrentOpLineRange(
+    std::optional<LineRangeBeginTy> scope) {
+  CurrentOpLineRange = scope;
+}
+
 void spirv_ll::Builder::addDebugInfoToModule() {
   // If any debug info was added to the module we will have at least a
   // `DICompileUnit`
   if (module.getCompileUnit()) {
     DIBuilder.finalize();
   }
-
-  for (auto &[loc, op_line_ranges] : module.getOpLineRanges()) {
-    llvm::DebugLoc location(loc);
-    for (auto [range_begin, range_end] : op_line_ranges) {
-      ++range_begin;
-      if (range_end != range_begin->getParent()->end()) {
-        ++range_end;
-      }
-
-      for (auto &inst : make_range(range_begin, range_end)) {
-        inst.setDebugLoc(location);
-      }
-    }
-  }
 }
 
 namespace {