Skip to content

Conversation

@bwlodarcz
Copy link
Contributor

@bwlodarcz bwlodarcz commented Oct 24, 2024

This commit adds support for the DebugLine instruction from the NonSemantic.Shader.DebugInfo.100 standard in the backend.

Additionally, it introduces a significant architectural change to SPIRVEmitNonSemanticDI.cpp by implementing a safe local abstraction layer for state management. This enhancement aims to improve code maintainability and clarity.

@github-actions
Copy link

github-actions bot commented Oct 24, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@bwlodarcz
Copy link
Contributor Author

bwlodarcz commented Oct 24, 2024

TODO:

  • Rename functions
  • Provide comments with descriptions why such design choices was made
  • Remove static from Back and Value in DebugTypeContainer to attach resources lifetime to runMachineFunction lifetime.
  • Restrict access to the state abstraction internals
  • Implement emitDebugLine method
  • Add lit tests for DebugLine

@bwlodarcz bwlodarcz marked this pull request as ready for review October 31, 2024 14:53
@llvmbot
Copy link
Member

llvmbot commented Oct 31, 2024

@llvm/pr-subscribers-backend-spir-v

Author: None (bwlodarcz)

Changes

This commit adds support for the DebugLine instruction from the NonSemantic.Shader.DebugInfo.100 standard in the backend.

Additionally, it introduces a significant architectural change to SPIRVEmitNonSemanticDI.cpp by implementing a safe local abstraction layer for state management. This enhancement aims to improve code maintainability and clarity.


Patch is 48.16 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/113541.diff

3 Files Affected:

  • (modified) llvm/lib/Target/SPIRV/SPIRVEmitNonSemanticDI.cpp (+552-253)
  • (added) llvm/test/CodeGen/SPIRV/debug-info/debug-line.ll (+144)
  • (modified) llvm/test/CodeGen/SPIRV/debug-info/debug-type-pointer.ll (+19-13)
diff --git a/llvm/lib/Target/SPIRV/SPIRVEmitNonSemanticDI.cpp b/llvm/lib/Target/SPIRV/SPIRVEmitNonSemanticDI.cpp
index d3e323efaee91b..8d8d348d8c9130 100644
--- a/llvm/lib/Target/SPIRV/SPIRVEmitNonSemanticDI.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVEmitNonSemanticDI.cpp
@@ -5,17 +5,15 @@
 #include "SPIRVRegisterInfo.h"
 #include "SPIRVTargetMachine.h"
 #include "SPIRVUtils.h"
-#include "llvm/ADT/SmallPtrSet.h"
+
 #include "llvm/ADT/SmallString.h"
 #include "llvm/BinaryFormat/Dwarf.h"
 #include "llvm/CodeGen/GlobalISel/MachineIRBuilder.h"
 #include "llvm/CodeGen/MachineBasicBlock.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineFunctionPass.h"
-#include "llvm/CodeGen/MachineInstr.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
 #include "llvm/CodeGen/MachineModuleInfo.h"
-#include "llvm/CodeGen/MachineOperand.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/CodeGen/Register.h"
 #include "llvm/IR/DebugInfoMetadata.h"
@@ -27,42 +25,9 @@
 
 #define DEBUG_TYPE "spirv-nonsemantic-debug-info"
 
-namespace llvm {
-struct SPIRVEmitNonSemanticDI : public MachineFunctionPass {
-  static char ID;
-  SPIRVTargetMachine *TM;
-  SPIRVEmitNonSemanticDI(SPIRVTargetMachine *TM);
-  SPIRVEmitNonSemanticDI();
-
-  bool runOnMachineFunction(MachineFunction &MF) override;
-
-private:
-  bool IsGlobalDIEmitted = false;
-  bool emitGlobalDI(MachineFunction &MF);
-};
-} // namespace llvm
-
+namespace {
 using namespace llvm;
 
-INITIALIZE_PASS(SPIRVEmitNonSemanticDI, DEBUG_TYPE,
-                "SPIRV NonSemantic.Shader.DebugInfo.100 emitter", false, false)
-
-char SPIRVEmitNonSemanticDI::ID = 0;
-
-MachineFunctionPass *
-llvm::createSPIRVEmitNonSemanticDIPass(SPIRVTargetMachine *TM) {
-  return new SPIRVEmitNonSemanticDI(TM);
-}
-
-SPIRVEmitNonSemanticDI::SPIRVEmitNonSemanticDI(SPIRVTargetMachine *TM)
-    : MachineFunctionPass(ID), TM(TM) {
-  initializeSPIRVEmitNonSemanticDIPass(*PassRegistry::getPassRegistry());
-}
-
-SPIRVEmitNonSemanticDI::SPIRVEmitNonSemanticDI() : MachineFunctionPass(ID) {
-  initializeSPIRVEmitNonSemanticDIPass(*PassRegistry::getPassRegistry());
-}
-
 enum BaseTypeAttributeEncoding {
   Unspecified = 0,
   Address = 1,
@@ -90,149 +55,372 @@ enum SourceLanguage {
   Zig = 12
 };
 
-bool SPIRVEmitNonSemanticDI::emitGlobalDI(MachineFunction &MF) {
-  // If this MachineFunction doesn't have any BB repeat procedure
-  // for the next
-  if (MF.begin() == MF.end()) {
-    IsGlobalDIEmitted = false;
-    return false;
+enum TypesMapping {
+  PrimitiveIntArray,
+  PrimitiveStringArray,
+  DebugSourceArray,
+  DebugCompilationUnitArray,
+  DebugTypeBasicArray,
+  DebugTypePointerArray,
+  DebugInfoNoneArray,
+  DebugLineArray
+};
+
+struct DebugSource {
+  size_t FileId;
+
+  explicit constexpr DebugSource(const size_t FileId) noexcept
+      : FileId(FileId) {}
+
+  explicit constexpr DebugSource(const ArrayRef<size_t> AR) : FileId(AR[0]) {}
+
+  friend bool operator==(const DebugSource &Lhs, const DebugSource &Rhs) {
+    return Lhs.FileId == Rhs.FileId;
   }
+};
 
-  // Required variables to get from metadata search
-  LLVMContext *Context;
-  SmallVector<SmallString<128>> FilePaths;
-  SmallVector<int64_t> LLVMSourceLanguages;
-  int64_t DwarfVersion = 0;
-  int64_t DebugInfoVersion = 0;
-  SmallPtrSet<DIBasicType *, 12> BasicTypes;
-  SmallPtrSet<DIDerivedType *, 12> PointerDerivedTypes;
-  // Searching through the Module metadata to find nescessary
-  // information like DwarfVersion or SourceLanguage
-  {
-    const MachineModuleInfo &MMI =
-        getAnalysis<MachineModuleInfoWrapperPass>().getMMI();
-    const Module *M = MMI.getModule();
-    Context = &M->getContext();
-    const NamedMDNode *DbgCu = M->getNamedMetadata("llvm.dbg.cu");
-    if (!DbgCu)
-      return false;
-    for (const auto *Op : DbgCu->operands()) {
-      if (const auto *CompileUnit = dyn_cast<DICompileUnit>(Op)) {
-        DIFile *File = CompileUnit->getFile();
-        FilePaths.emplace_back();
-        sys::path::append(FilePaths.back(), File->getDirectory(),
-                          File->getFilename());
-        LLVMSourceLanguages.push_back(CompileUnit->getSourceLanguage());
-      }
+struct DebugCompilationUnit {
+  size_t DebugInfoVersionId;
+  size_t DwarfVersionId;
+  size_t DebugSourceId;
+  size_t LanguageId;
+
+  constexpr DebugCompilationUnit(const size_t DebugInfoVersionId,
+                                 const size_t DwarfVersionId,
+                                 const size_t DebugSourceId,
+                                 const size_t LanguageId) noexcept
+      : DebugInfoVersionId(DebugInfoVersionId), DwarfVersionId(DwarfVersionId),
+        DebugSourceId(DebugSourceId), LanguageId(LanguageId) {}
+
+  explicit constexpr DebugCompilationUnit(const ArrayRef<size_t> AR) noexcept
+      : DebugInfoVersionId(AR[0]), DwarfVersionId(AR[1]), DebugSourceId(AR[2]),
+        LanguageId(AR[3]) {}
+
+  friend bool operator==(const DebugCompilationUnit &Lhs,
+                         const DebugCompilationUnit &Rhs) {
+    return Lhs.DebugInfoVersionId == Rhs.DebugInfoVersionId &
+           Lhs.DwarfVersionId == Rhs.DwarfVersionId &
+           Lhs.DebugSourceId == Rhs.DebugSourceId &
+           Lhs.LanguageId == Rhs.LanguageId;
+  }
+};
+
+struct DebugTypeBasic {
+  size_t NameId;
+  size_t SizeId;
+  size_t BaseTypeEncodingId;
+  size_t FlagsId;
+
+  explicit constexpr DebugTypeBasic(const ArrayRef<size_t> AR)
+      : NameId(AR[0]), SizeId(AR[1]), BaseTypeEncodingId(AR[2]),
+        FlagsId(AR[3]) {}
+
+  friend bool operator==(const DebugTypeBasic &Lhs, const DebugTypeBasic &Rhs) {
+    return Lhs.NameId == Rhs.NameId && Lhs.SizeId == Rhs.SizeId &&
+           Lhs.BaseTypeEncodingId == Rhs.BaseTypeEncodingId &&
+           Lhs.FlagsId == Rhs.FlagsId;
+  }
+};
+
+struct DebugTypePointer {
+  size_t BaseTypeId;
+  size_t StorageClassId;
+  size_t FlagsId;
+
+  DebugTypePointer(const size_t BaseTypeId, const size_t StorageClassId,
+                   const size_t FlagsId) noexcept
+      : BaseTypeId(BaseTypeId), StorageClassId(StorageClassId),
+        FlagsId(FlagsId) {}
+
+  explicit DebugTypePointer(const ArrayRef<size_t> AR) noexcept
+      : BaseTypeId(AR[0]), StorageClassId(AR[1]), FlagsId(AR[2]) {}
+
+  friend bool operator==(const DebugTypePointer &Lhs,
+                         const DebugTypePointer &Rhs) {
+    return Lhs.BaseTypeId == Rhs.BaseTypeId &&
+           Lhs.StorageClassId == Rhs.StorageClassId &&
+           Lhs.FlagsId == Rhs.FlagsId;
+  }
+};
+
+struct DebugLine {
+  size_t DebugSourceId;
+  size_t LineStartId;
+  size_t LineEndId;
+  size_t ColumnStartId;
+  size_t ColumnEndId;
+
+  explicit DebugLine(const ArrayRef<size_t> AR)
+      : DebugSourceId(AR[0]), LineStartId(AR[1]), LineEndId(AR[2]),
+        ColumnStartId(AR[3]), ColumnEndId(AR[4]) {}
+
+  friend bool operator==(const DebugLine &Lhs, const DebugLine &Rhs) {
+    return Lhs.DebugSourceId == Rhs.DebugSourceId &&
+           Lhs.LineStartId == Rhs.LineStartId &&
+           Lhs.LineEndId == Rhs.LineEndId &&
+           Lhs.ColumnStartId == Rhs.ColumnStartId &&
+           Lhs.ColumnEndId == Rhs.ColumnEndId;
+  }
+};
+
+struct DebugInfoNone;
+
+template <typename T> struct DebugTypeContainer;
+
+template <> struct DebugTypeContainer<int64_t> {
+  static constexpr TypesMapping TM = PrimitiveIntArray;
+};
+
+template <> struct DebugTypeContainer<StringRef> {
+  static constexpr TypesMapping TM = PrimitiveStringArray;
+};
+
+template <> struct DebugTypeContainer<DebugSource> {
+  static constexpr TypesMapping TM = DebugSourceArray;
+  static constexpr SPIRV::NonSemanticExtInst::NonSemanticExtInst Inst =
+      SPIRV::NonSemanticExtInst::DebugSource;
+};
+
+template <> struct DebugTypeContainer<DebugCompilationUnit> {
+  static constexpr TypesMapping TM = DebugCompilationUnitArray;
+  static constexpr SPIRV::NonSemanticExtInst::NonSemanticExtInst Inst =
+      SPIRV::NonSemanticExtInst::DebugCompilationUnit;
+};
+
+template <> struct DebugTypeContainer<DebugTypeBasic> {
+  static constexpr TypesMapping TM = DebugTypeBasicArray;
+  static constexpr SPIRV::NonSemanticExtInst::NonSemanticExtInst Inst =
+      SPIRV::NonSemanticExtInst::DebugTypeBasic;
+};
+
+template <> struct DebugTypeContainer<DebugTypePointer> {
+  static constexpr TypesMapping TM = DebugTypePointerArray;
+  static constexpr SPIRV::NonSemanticExtInst::NonSemanticExtInst Inst =
+      SPIRV::NonSemanticExtInst::DebugTypePointer;
+};
+
+template <> struct DebugTypeContainer<DebugInfoNone> {
+  static constexpr TypesMapping TM = DebugInfoNoneArray;
+  static constexpr SPIRV::NonSemanticExtInst::NonSemanticExtInst Inst =
+      SPIRV::NonSemanticExtInst::DebugInfoNone;
+};
+
+template <> struct DebugTypeContainer<DebugLine> {
+  static constexpr TypesMapping TM = DebugLineArray;
+  static constexpr SPIRV::NonSemanticExtInst::NonSemanticExtInst Inst =
+      SPIRV::NonSemanticExtInst::DebugLine;
+};
+
+template <typename T> struct DebugLiveContainer {
+  SmallVector<T> Values;
+  SmallVector<size_t> BackIdx;
+};
+
+class LiveRepository {
+  DebugLiveContainer<int64_t> PrimitiveInts;
+  DebugLiveContainer<StringRef> PrimitiveStrings;
+  DebugLiveContainer<DebugSource> DebugSources;
+  DebugLiveContainer<DebugLine> DebugLines;
+  DebugLiveContainer<DebugCompilationUnit> DebugCompilationUnits;
+  DebugLiveContainer<DebugTypeBasic> DebugTypeBasics;
+  DebugLiveContainer<DebugTypePointer> DebugTypePointers;
+
+  SmallVector<Register> Registers;
+  SmallVector<std::pair<TypesMapping, unsigned>> Instructions;
+
+  template <typename T> constexpr SmallVector<T> &values() {
+    if constexpr (std::is_same_v<T, int64_t>) {
+      return PrimitiveInts.Values;
+    } else if constexpr (std::is_same_v<T, StringRef>) {
+      return PrimitiveStrings.Values;
+    } else if constexpr (std::is_same_v<T, DebugLine>) {
+      return DebugLines.Values;
+    } else if constexpr (std::is_same_v<T, DebugSource>) {
+      return DebugSources.Values;
+    } else if constexpr (std::is_same_v<T, DebugCompilationUnit>) {
+      return DebugCompilationUnits.Values;
+    } else if constexpr (std::is_same_v<T, DebugTypeBasic>) {
+      return DebugTypeBasics.Values;
+    } else if constexpr (std::is_same_v<T, DebugTypePointer>) {
+      return DebugTypePointers.Values;
     }
-    const NamedMDNode *ModuleFlags = M->getNamedMetadata("llvm.module.flags");
-    for (const auto *Op : ModuleFlags->operands()) {
-      const MDOperand &MaybeStrOp = Op->getOperand(1);
-      if (MaybeStrOp.equalsStr("Dwarf Version"))
-        DwarfVersion =
-            cast<ConstantInt>(
-                cast<ConstantAsMetadata>(Op->getOperand(2))->getValue())
-                ->getSExtValue();
-      else if (MaybeStrOp.equalsStr("Debug Info Version"))
-        DebugInfoVersion =
-            cast<ConstantInt>(
-                cast<ConstantAsMetadata>(Op->getOperand(2))->getValue())
-                ->getSExtValue();
+    llvm_unreachable("unreachable");
+  }
+
+  template <typename T> constexpr SmallVector<size_t> &backIdx() {
+    if constexpr (std::is_same_v<T, int64_t>) {
+      return PrimitiveInts.BackIdx;
+    } else if constexpr (std::is_same_v<T, StringRef>) {
+      return PrimitiveStrings.BackIdx;
+    } else if constexpr (std::is_same_v<T, DebugLine>) {
+      return DebugLines.BackIdx;
+    } else if constexpr (std::is_same_v<T, DebugSource>) {
+      return DebugSources.BackIdx;
+    } else if constexpr (std::is_same_v<T, DebugCompilationUnit>) {
+      return DebugCompilationUnits.BackIdx;
+    } else if constexpr (std::is_same_v<T, DebugTypeBasic>) {
+      return DebugTypeBasics.BackIdx;
+    } else if constexpr (std::is_same_v<T, DebugTypePointer>) {
+      return DebugTypePointers.BackIdx;
     }
+    llvm_unreachable("unreachable");
+  }
 
-    // This traversal is the only supported way to access
-    // instruction related DI metadata like DIBasicType
-    for (auto &F : *M) {
-      for (auto &BB : F) {
-        for (auto &I : BB) {
-          for (DbgVariableRecord &DVR : filterDbgVars(I.getDbgRecordRange())) {
-            DILocalVariable *LocalVariable = DVR.getVariable();
-            if (auto *BasicType =
-                    dyn_cast<DIBasicType>(LocalVariable->getType())) {
-              BasicTypes.insert(BasicType);
-            } else if (auto *DerivedType =
-                           dyn_cast<DIDerivedType>(LocalVariable->getType())) {
-              if (DerivedType->getTag() == dwarf::DW_TAG_pointer_type) {
-                PointerDerivedTypes.insert(DerivedType);
-                // DIBasicType can be unreachable from DbgRecord and only
-                // pointed on from other DI types
-                // DerivedType->getBaseType is null when pointer
-                // is representing a void type
-                if (DerivedType->getBaseType())
-                  BasicTypes.insert(
-                      cast<DIBasicType>(DerivedType->getBaseType()));
-              }
-            }
-          }
-        }
+  template <typename T>
+  static std::pair<size_t, bool>
+  emplaceOrReturnDuplicate(T Val, SmallVectorImpl<T> &SV) {
+    for (unsigned Idx = 0; Idx < SV.size(); ++Idx) {
+      if (Val == SV[Idx]) {
+        return {Idx, true};
       }
     }
+    SV.emplace_back(Val);
+    return {SV.size() - 1, false};
+  }
+
+  static Register emitOpString(const StringRef SR,
+                               MachineIRBuilder &MIRBuilder) {
+    MachineRegisterInfo *MRI = MIRBuilder.getMRI();
+    const Register StrReg = MRI->createVirtualRegister(&SPIRV::IDRegClass);
+    MRI->setType(StrReg, LLT::scalar(32));
+    MachineInstrBuilder MIB = MIRBuilder.buildInstr(SPIRV::OpString);
+    MIB.addDef(StrReg);
+    addStringImm(SR, MIB);
+    return StrReg;
   }
-  // NonSemantic.Shader.DebugInfo.100 global DI instruction emitting
-  {
-    // Required LLVM variables for emitting logic
+
+  Register emitDIInstruction(SPIRV::NonSemanticExtInst::NonSemanticExtInst Inst,
+                             const ArrayRef<size_t> Ids,
+                             MachineIRBuilder &MIRBuilder,
+                             const SPIRVTargetMachine *const TM) {
     const SPIRVInstrInfo *TII = TM->getSubtargetImpl()->getInstrInfo();
     const SPIRVRegisterInfo *TRI = TM->getSubtargetImpl()->getRegisterInfo();
     const RegisterBankInfo *RBI = TM->getSubtargetImpl()->getRegBankInfo();
     SPIRVGlobalRegistry *GR = TM->getSubtargetImpl()->getSPIRVGlobalRegistry();
-    MachineRegisterInfo &MRI = MF.getRegInfo();
-    MachineBasicBlock &MBB = *MF.begin();
-
-    // To correct placement of a OpLabel instruction during SPIRVAsmPrinter
-    // emission all new instructions needs to be placed after OpFunction
-    // and before first terminator
-    MachineIRBuilder MIRBuilder(MBB, MBB.getFirstTerminator());
-
-    const auto EmitOpString = [&](StringRef SR) {
-      const Register StrReg = MRI.createVirtualRegister(&SPIRV::IDRegClass);
-      MRI.setType(StrReg, LLT::scalar(32));
-      MachineInstrBuilder MIB = MIRBuilder.buildInstr(SPIRV::OpString);
-      MIB.addDef(StrReg);
-      addStringImm(SR, MIB);
-      return StrReg;
-    };
-
-    const SPIRVType *VoidTy =
-        GR->getOrCreateSPIRVType(Type::getVoidTy(*Context), MIRBuilder);
-
-    const auto EmitDIInstruction =
-        [&](SPIRV::NonSemanticExtInst::NonSemanticExtInst Inst,
-            std::initializer_list<Register> Registers) {
-          const Register InstReg =
-              MRI.createVirtualRegister(&SPIRV::IDRegClass);
-          MRI.setType(InstReg, LLT::scalar(32));
-          MachineInstrBuilder MIB =
-              MIRBuilder.buildInstr(SPIRV::OpExtInst)
-                  .addDef(InstReg)
-                  .addUse(GR->getSPIRVTypeID(VoidTy))
-                  .addImm(static_cast<int64_t>(
-                      SPIRV::InstructionSet::NonSemantic_Shader_DebugInfo_100))
-                  .addImm(Inst);
-          for (auto Reg : Registers) {
-            MIB.addUse(Reg);
-          }
-          MIB.constrainAllUses(*TII, *TRI, *RBI);
-          GR->assignSPIRVTypeToVReg(VoidTy, InstReg, MF);
-          return InstReg;
-        };
+    MachineRegisterInfo *MRI = MIRBuilder.getMRI();
+    const SPIRVType *VoidTy = GR->getOrCreateSPIRVType(
+        Type::getVoidTy(MIRBuilder.getContext()), MIRBuilder);
+    const Register InstReg = MRI->createVirtualRegister(&SPIRV::IDRegClass);
+    MRI->setType(InstReg, LLT::scalar(32));
+    MachineInstrBuilder MIB =
+        MIRBuilder.buildInstr(SPIRV::OpExtInst)
+            .addDef(InstReg)
+            .addUse(GR->getSPIRVTypeID(VoidTy))
+            .addImm(SPIRV::InstructionSet::NonSemantic_Shader_DebugInfo_100)
+            .addImm(Inst);
+    for (const auto Idx : Ids) {
+      MIB.addUse(Registers[Idx]);
+    }
+    MIB.constrainAllUses(*TII, *TRI, *RBI);
+    GR->assignSPIRVTypeToVReg(VoidTy, InstReg, MIRBuilder.getMF());
+    return InstReg;
+  }
 
-    const SPIRVType *I32Ty =
-        GR->getOrCreateSPIRVType(Type::getInt32Ty(*Context), MIRBuilder);
+public:
+  size_t push(const int64_t Val, MachineIRBuilder &MIRBuilder,
+              const SPIRVTargetMachine *TM) {
+    auto &SV = values<int64_t>();
+    const auto [ConcreteIdx, IsDuplicate] = emplaceOrReturnDuplicate(Val, SV);
+    if (IsDuplicate) {
+      return backIdx<int64_t>()[ConcreteIdx];
+    }
+    Instructions.emplace_back(DebugTypeContainer<int64_t>::TM, ConcreteIdx);
+    SPIRVGlobalRegistry *GR = TM->getSubtargetImpl()->getSPIRVGlobalRegistry();
+    const SPIRVType *I32Ty = GR->getOrCreateSPIRVType(
+        Type::getInt32Ty(MIRBuilder.getContext()), MIRBuilder);
+    Registers.emplace_back(GR->buildConstantInt(Val, MIRBuilder, I32Ty, false));
+    backIdx<int64_t>().emplace_back(Instructions.size() - 1);
+    return Instructions.size() - 1;
+  }
 
-    const Register DwarfVersionReg =
-        GR->buildConstantInt(DwarfVersion, MIRBuilder, I32Ty, false);
+  size_t push(const StringRef Val, MachineIRBuilder &MIRBuilder) {
+    auto &SV = values<StringRef>();
+    const auto [ConcreteIdx, IsDuplicate] = emplaceOrReturnDuplicate(Val, SV);
+    if (IsDuplicate) {
+      return backIdx<StringRef>()[ConcreteIdx];
+    }
+    Instructions.emplace_back(DebugTypeContainer<StringRef>::TM, ConcreteIdx);
+    Registers.emplace_back(emitOpString(Val, MIRBuilder));
+    backIdx<StringRef>().emplace_back(Instructions.size() - 1);
+    return Instructions.size() - 1;
+  }
 
-    const Register DebugInfoVersionReg =
-        GR->buildConstantInt(DebugInfoVersion, MIRBuilder, I32Ty, false);
+  template <typename T>
+  constexpr size_t push(ArrayRef<size_t> Args, MachineIRBuilder &MIRBuilder,
+                        const SPIRVTargetMachine *TM) {
+    auto &SV = values<T>();
+    const auto [ConcreteIdx, IsDuplicate] =
+        emplaceOrReturnDuplicate(T(Args), SV);
+    if (IsDuplicate) {
+      return backIdx<T>()[ConcreteIdx];
+    }
+    Instructions.emplace_back(DebugTypeContainer<T>::TM, ConcreteIdx);
+    Registers.emplace_back(
+        emitDIInstruction(DebugTypeContainer<T>::Inst, Args, MIRBuilder, TM));
+    backIdx<T>().emplace_back(Instructions.size() - 1);
+    return Instructions.size() - 1;
+  }
+};
 
-    for (unsigned Idx = 0; Idx < LLVMSourceLanguages.size(); ++Idx) {
-      const Register FilePathStrReg = EmitOpString(FilePaths[Idx]);
+template <>
+size_t LiveRepository::push<DebugInfoNone>(ArrayRef<size_t>,
+                                           MachineIRBuilder &MIRBuilder,
+                                           const SPIRVTargetMachine *TM) {
+  static std::optional<size_t> DebugInfoNoneIdx = std::nullopt;
+  if (!DebugInfoNoneIdx.has_value()) {
+    Instructions.emplace_back(DebugTypeContainer<DebugInfoNone>::TM, 0);
+    Registers.emplace_back(emitDIInstruction(
+        DebugTypeContainer<DebugInfoNone>::Inst, {}, MIRBuilder, TM));
+    DebugInfoNoneIdx.emplace(Instructions.size() - 1);
+  }
+  return DebugInfoNoneIdx.value();
+}
 
-      const Register DebugSourceResIdReg = EmitDIInstruction(
-          SPIRV::NonSemanticExtInst::DebugSource, {FilePathStrReg});
+size_t emitDebugSource(const DIFile *File, MachineIRBuilder &MIRBuilder,
+                       SPIRVTargetMachine *TM, LiveRepository &LR) {
+  SmallString<128> FilePath;
+  sys::path::append(FilePath, File->getDirectory(), File->getFilename());
+  const size_t FilePathId = LR.push(StringRef(FilePath.c_str()), MIRBuilder);
+  return LR.push<DebugSource>({FilePathId},...
[truncated]

@bwlodarcz
Copy link
Contributor Author

@michalpaszkowski @VyacheslavLevytskyy @Keenuts Please review.
Errors in CI unrelated.

Copy link
Member

@michalpaszkowski michalpaszkowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the PR looks good overall, added three comments below. I will take one more look tomorrow morning.

LiveRepository &LR) {
std::optional<size_t> DwarfVersionId = std::nullopt;
std::optional<size_t> DebugInfoVersionId = std::nullopt;
const NamedMDNode *ModuleFlags = M->getNamedMetadata("llvm.module.flags");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should a null check be added for ModuleFlags?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory yes. The problem is that lack of llvm.module.flags means that DI and overall Module metadata is malformed and there is no sensible recover from that. In that case optionals will rise exception or we get segfault - effect is the same.

}
}
assert(DwarfVersionId.has_value() && DebugInfoVersionId.has_value());
const NamedMDNode *DbgCu = M->getNamedMetadata("llvm.dbg.cu");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should a null check be added for DbgCu?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Presence of DbgCu is checked in runOnMachineFunction to evaluate if DI is present so here it should always succeed.

; CHECK-MIR-DAG: [[debug_info_none:%[0-9]+\:id\(s32\)]] = OpExtInst [[void_type]], 3, 0
; CHECK-MIR-DAG: OpExtInst [[void_type]], 3, 3, [[debug_info_none]], [[i32_5]], [[i32_0]]

; Duplicates check
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe you could check (or ensure) that there are no duplicates using CHECK-COUNT, but I am not sure if this would only then fail in case duplicates appear next to each other.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CHECK-MIR-NOT in this case have a benefit of single line. In a case of excessive instruction CHECK-MIR-NOT will blow anyway.

@bwlodarcz bwlodarcz closed this Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants