Merge pull request #232 from frasercrmck/spirv-ll-handler-hook

[spirv-ll] Better encapsulate the builder's finalization

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

@@ -66,6 +66,15 @@ class ExtInstSetHandler {
 
   virtual ~ExtInstSetHandler() {}
 
+  /// @brief A hook called once all instructions in the module have been
+  /// visited and the IR has been finalized by the 'main' builder.
+  ///
+  /// Note that handlers may further alter the IR and the order in which they
+  /// are called is not deterministic.
+  virtual llvm::Error finishModuleProcessing() {
+    return llvm::Error::success();
+  }
+
   /// @brief Create an OpenCL extended instruction transformation to LLVM IR.
   ///
   /// @param opc The OpCode object to translate.
@@ -247,12 +256,9 @@ class Builder {
   Module::LineRangeBeginTy createLineRangeBegin(const OpLine *op_line,
                                                 llvm::BasicBlock &bb);
 
-  /// @brief Add debug metadata to the appropriate instructions
-  void addDebugInfoToModule();
-
-  /// @brief Finalizes and adds any metadata to LLVM that was generated by
-  /// SpvBuilder
-  void finalizeMetadata();
+  /// @brief Called once all instructions in the module have been visited in
+  /// order during the first pass through the SPIR-V binary.
+  llvm::Error finishModuleProcessing();
 
   /// @brief Gets (or creates) the BasicBlock for a spv::Id OpLabel.
   llvm::BasicBlock *getOrCreateBasicBlock(spv::Id label);
@@ -279,10 +285,6 @@ class Builder {
   bool replaceBuiltinUsesWithCalls(llvm::GlobalVariable *builtinGlobal,
                                    spv::BuiltIn kind);
 
-  /// @brief Replaces all references to global builtin variables with a thread
-  /// safe function local definition
-  void replaceBuiltinGlobals();
-
   /// @brief Creates a call to a builtin function.
   ///
   /// No function name mangling is performed, see
@@ -714,6 +716,16 @@ class Builder {
       builder = std::make_unique<Handler>(*this, module);
     }
   }
+  /// @brief Add debug metadata to the appropriate instructions
+  void addDebugInfoToModule();
+
+  /// @brief Replaces all references to global builtin variables with a
+  /// thread-safe function local definition
+  void replaceBuiltinGlobals();
+
+  /// @brief Finalizes and adds any metadata to LLVM that was generated by
+  /// SpvBuilder
+  void finalizeMetadata();
 
   /// @brief Returns an extended instruction set handler for the instruction
   /// set ID.

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

@@ -18,6 +18,7 @@
 #include <llvm/BinaryFormat/Dwarf.h>
 #include <llvm/IR/Attributes.h>
 #include <llvm/Support/Debug.h>
+#include <llvm/Support/Error.h>
 #include <llvm/Support/type_traits.h>
 #include <multi_llvm/vector_type_helper.h>
 #include <spirv-ll/assert.h>
@@ -124,6 +125,36 @@ llvm::DIType *spirv_ll::Builder::getDIType(spv::Id tyID) {
   llvm_unreachable("unsupported debug type");
 }
 
+llvm::Error spirv_ll::Builder::finishModuleProcessing() {
+  // Add debug info, before we start replacing global builtin vars; the
+  // instruction ranges we've recorded are on the current state of the basic
+  // blocks. Replacing the global builtins will invalidate the iterators.
+  addDebugInfoToModule();
+
+  // Replace all global builtin vars with function local versions
+  replaceBuiltinGlobals();
+
+  // Set some default attributes on functions we've created.
+  for (auto &function : module.llvmModule->functions()) {
+    // We don't use exceptions
+    if (!function.hasFnAttribute(llvm::Attribute::NoUnwind)) {
+      function.addFnAttr(llvm::Attribute::NoUnwind);
+    }
+  }
+
+  // Add any remaining metadata to llvm module
+  finalizeMetadata();
+
+  // Notify handlers that the module has been finished.
+  for (auto &[set, handler] : ext_inst_handlers) {
+    if (auto err = handler->finishModuleProcessing()) {
+      return err;
+    }
+  }
+
+  return llvm::Error::success();
+}
+
 void spirv_ll::Builder::addDebugInfoToModule() {
   // If any debug info was added to the module we will have at least a
   // `DICompileUnit`

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

@@ -1121,25 +1121,10 @@ cargo::expected<spirv_ll::Module, spirv_ll::Error> spirv_ll::Context::translate(
     }
   }
 
-  // Add debug info, before we start replacing global builtin vars; the
-  // instruction ranges we've recorded are on the current state of the basic
-  // blocks. Replacing the global builtins will invalidate the iterators.
-  builder.addDebugInfoToModule();
-
-  // Replace all global builtin vars with function local versions
-  builder.replaceBuiltinGlobals();
-
-  // Set some default attributes on functions we've created.
-  for (auto &function : module.llvmModule->functions()) {
-    // We don't use exceptions
-    if (!function.hasFnAttribute(llvm::Attribute::NoUnwind)) {
-      function.addFnAttr(llvm::Attribute::NoUnwind);
-    }
+  if (auto err = builder.finishModuleProcessing()) {
+    return cargo::make_unexpected(llvm::toString(std::move(err)));
   }
 
-  // Add any remaining metadata to llvm module
-  builder.finalizeMetadata();
-
 #if defined(__GNUC__) && !defined(__clang__) && __GNUC__ < 9
   // GCC <9 requires this redundant move, this branch of the #if can be
   // deleted once the minimum supported version of GCC is at least 9.