Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPIR-V] Emit DebugTypePointer from NonSemantic DI #109287

Merged
merged 3 commits into from
Oct 8, 2024

Conversation

bwlodarcz
Copy link
Contributor

Implementation of DebugTypePointer from NonSemantic.Shader.DebugInfo.100.

@llvmbot
Copy link
Member

llvmbot commented Sep 19, 2024

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

Author: None (bwlodarcz)

Changes

Implementation of DebugTypePointer from NonSemantic.Shader.DebugInfo.100.


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

3 Files Affected:

  • (modified) llvm/lib/Target/SPIRV/SPIRVEmitNonSemanticDI.cpp (+42-6)
  • (modified) llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp (+1-1)
  • (added) llvm/test/CodeGen/SPIRV/debug-info/debug-type-pointer.ll (+275)
diff --git a/llvm/lib/Target/SPIRV/SPIRVEmitNonSemanticDI.cpp b/llvm/lib/Target/SPIRV/SPIRVEmitNonSemanticDI.cpp
index b78f1c3f060a23..f9e6e08f5d75ad 100644
--- a/llvm/lib/Target/SPIRV/SPIRVEmitNonSemanticDI.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVEmitNonSemanticDI.cpp
@@ -88,6 +88,7 @@ bool SPIRVEmitNonSemanticDI::emitGlobalDI(MachineFunction &MF) {
   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
   {
@@ -129,8 +130,22 @@ bool SPIRVEmitNonSemanticDI::emitGlobalDI(MachineFunction &MF) {
           for (DbgVariableRecord &DVR : filterDbgVars(I.getDbgRecordRange())) {
             DILocalVariable *LocalVariable = DVR.getVariable();
             if (auto *BasicType =
-                    dyn_cast<DIBasicType>(LocalVariable->getType()))
+                    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()));
+                }
+              }
+            }
           }
         }
       }
@@ -160,7 +175,6 @@ bool SPIRVEmitNonSemanticDI::emitGlobalDI(MachineFunction &MF) {
       return StrReg;
     };
 
-    // Emit OpString with FilePath which is required by DebugSource
     const Register FilePathStrReg = EmitOpString(FilePath);
 
     const SPIRVType *VoidTy =
@@ -187,15 +201,12 @@ bool SPIRVEmitNonSemanticDI::emitGlobalDI(MachineFunction &MF) {
           return InstReg;
         };
 
-    // Emit DebugSource which is required by DebugCompilationUnit
     const Register DebugSourceResIdReg = EmitDIInstruction(
         SPIRV::NonSemanticExtInst::DebugSource, {FilePathStrReg});
 
     const SPIRVType *I32Ty =
         GR->getOrCreateSPIRVType(Type::getInt32Ty(*Context), MIRBuilder);
 
-    // Convert DwarfVersion, DebugInfo and SourceLanguage integers to OpConstant
-    // instructions required by DebugCompilationUnit
     const Register DwarfVersionReg =
         GR->buildConstantInt(DwarfVersion, MIRBuilder, I32Ty, false);
     const Register DebugInfoVersionReg =
@@ -214,6 +225,11 @@ bool SPIRVEmitNonSemanticDI::emitGlobalDI(MachineFunction &MF) {
     const Register I32ZeroReg =
         GR->buildConstantInt(0, MIRBuilder, I32Ty, false);
 
+    // We need to store pairs because further instructions reference
+    // the DIBasicTypes and size will be always small so there isn't
+    // need for any kind of map
+    SmallVector<std::pair<const DIBasicType *const, const Register>, 12>
+        BasicTypeRegPairs;
     for (auto *BasicType : BasicTypes) {
       const Register BasicTypeStrReg = EmitOpString(BasicType->getName());
 
@@ -247,11 +263,31 @@ bool SPIRVEmitNonSemanticDI::emitGlobalDI(MachineFunction &MF) {
       const Register AttributeEncodingReg =
           GR->buildConstantInt(AttributeEncoding, MIRBuilder, I32Ty, false);
 
-      [[maybe_unused]]
       const Register BasicTypeReg =
           EmitDIInstruction(SPIRV::NonSemanticExtInst::DebugTypeBasic,
                             {BasicTypeStrReg, ConstIntBitwidthReg,
                              AttributeEncodingReg, I32ZeroReg});
+      BasicTypeRegPairs.emplace_back(BasicType, BasicTypeReg);
+    }
+
+    if (PointerDerivedTypes.size()) {
+      const Register GenericStorageClass =
+          GR->buildConstantInt(8, MIRBuilder, I32Ty, false);
+      for (const auto *PointerDerivedType : PointerDerivedTypes) {
+        // If the Pointer is representing a void type it's getBaseType
+        // is a nullptr
+        const auto *MaybeBT =
+            cast_or_null<DIBasicType>(PointerDerivedType->getBaseType());
+        for (const auto &BasicTypeRegPair : BasicTypeRegPairs) {
+          const auto &[SBT, Reg] = BasicTypeRegPair;
+          if (SBT == MaybeBT) {
+            [[maybe_unused]]
+            const Register DebugPointerTypeReg =
+                EmitDIInstruction(SPIRV::NonSemanticExtInst::DebugTypePointer,
+                                  {Reg, GenericStorageClass, I32ZeroReg});
+          }
+        }
+      }
     }
   }
   return true;
diff --git a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
index fa71223a341b16..46bc9bbce05644 100644
--- a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
@@ -436,7 +436,7 @@ void SPIRVModuleAnalysis::processOtherInstrs(const Module &M) {
           namespace NS = SPIRV::NonSemanticExtInst;
           static constexpr int64_t GlobalNonSemanticDITy[] = {
               NS::DebugSource, NS::DebugCompilationUnit, NS::DebugInfoNone,
-              NS::DebugTypeBasic};
+              NS::DebugTypeBasic, NS::DebugTypePointer};
           bool IsGlobalDI = false;
           for (unsigned Idx = 0; Idx < std::size(GlobalNonSemanticDITy); ++Idx)
             IsGlobalDI |= Ins.getImm() == GlobalNonSemanticDITy[Idx];
diff --git a/llvm/test/CodeGen/SPIRV/debug-info/debug-type-pointer.ll b/llvm/test/CodeGen/SPIRV/debug-info/debug-type-pointer.ll
new file mode 100644
index 00000000000000..bdc43d892e02c0
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/debug-info/debug-type-pointer.ll
@@ -0,0 +1,275 @@
+; RUN: llc --verify-machineinstrs --spv-emit-nonsemantic-debug-info --spirv-ext=+SPV_KHR_non_semantic_info --print-after=spirv-nonsemantic-debug-info -O0 -mtriple=spirv64-unknown-unknown %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-MIR
+; RUN: llc --verify-machineinstrs --spv-emit-nonsemantic-debug-info --spirv-ext=+SPV_KHR_non_semantic_info -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s --check-prefix=CHECK-SPIRV
+; RUN: llc --verify-machineinstrs -O0 -mtriple=spirv64-unknown-unknown --spirv-ext=+SPV_KHR_non_semantic_info %s -o - | FileCheck %s --check-prefix=CHECK-OPTION
+; RUN: %if spirv-tools %{ llc --verify-machineinstrs --spv-emit-nonsemantic-debug-info --spirv-ext=+SPV_KHR_non_semantic_info -O0 -mtriple=spirv64-unknown-unknown %s -o - -filetype=obj | spirv-val %}
+
+; CHECK-MIR-DAG:   [[i32type:%[0-9]+\:type]] = OpTypeInt 32, 0
+; CHECK-MIR-DAG:   [[void_type:%[0-9]+\:type\(s64\)]] = OpTypeVoid
+; CHECK-MIR-DAG:   [[i32_8:%[0-9]+\:iid]] = OpConstantI [[i32type]], 8
+; CHECK-MIR-DAG:   [[i32_0:%[0-9]+\:iid]] = OpConstantI [[i32type]], 0
+; CHECK-MIR-DAG:   [[enc_signed_char:%[0-9]+\:iid\(s32\)]] = OpConstantI [[i32type]], 5
+; CHECK-MIR-DAG:   [[enc_float:%[0-9]+\:iid\(s32\)]] = OpConstantI [[i32type]], 3
+; CHECK-MIR-DAG:   [[enc_boolean:%[0-9]+\:iid\(s32\)]] = OpConstantI [[i32type]], 2
+; CHECK-MIR-DAG:   [[bool:%[0-9]+\:id\(s32\)]] = OpExtInst [[void_type]], 3, 2, {{%[0-9]+\:[a-z0-9\(\)]+}}, [[i32_8]], [[enc_boolean]], [[i32_0]]
+; CHECK-MIR-DAG:   [[i32_16:%[0-9]+\:iid\(s32\)]] = OpConstantI [[i32type]], 16
+; CHECK-MIR-DAG:   [[enc_signed:%[0-9]+\:iid\(s32\)]] = OpConstantI [[i32type]], 4
+; CHECK-MIR-DAG:   [[short:%[0-9]+\:id\(s32\)]] = OpExtInst [[void_type]], 3, 2, {{%[0-9]+\:[a-z0-9\(\)]+}}, [[i32_16]], [[enc_signed]], [[i32_0]]
+; CHECK-MIR-DAG:   [[char:%[0-9]+\:id\(s32\)]] = OpExtInst [[void_type]], 3, 2, {{%[0-9]+\:[a-z0-9\(\)]+}}, [[i32_8]], [[enc_signed_char]], [[i32_0]]
+; CHECK-MIR-DAG:   [[i32_64:%[0-9]+\:iid\(s32\)]] = OpConstantI [[i32type]], 64
+; CHECK-MIR-DAG:   [[long:%[0-9]+\:id\(s32\)]] = OpExtInst [[void_type]], 3, 2, {{%[0-9]+\:[a-z0-9\(\)]+}}, [[i32_64]], [[enc_signed]], [[i32_0]]
+; CHECK-MIR-DAG:   [[i32_32:%[0-9]+\:iid\(s32\)]] = OpConstantI [[i32type]], 32
+; CHECK-MIR-DAG:   [[enc_unsigned:%[0-9]+\:iid\(s32\)]] = OpConstantI [[i32type]], 6
+; CHECK-MIR-DAG:   [[unsigned_int:%[0-9]+\:id\(s32\)]] = OpExtInst [[void_type]], 3, 2, {{%[0-9]+\:[a-z0-9\(\)]+}}, [[i32_32]], [[enc_unsigned]], [[i32_0]]
+; CHECK-MIR-DAG:   [[unsigned_short:%[0-9]+\:id\(s32\)]] = OpExtInst [[void_type]], 3, 2, {{%[0-9]+\:[a-z0-9\(\)]+}}, [[i32_16]], [[enc_unsigned]], [[i32_0]]
+; CHECK-MIR-DAG:   [[enc_unsigned_char:%[0-9]+\:iid\(s32\)]] = OpConstantI [[i32type]], 7
+; CHECK-MIR-DAG:   [[unsigned_char:%[0-9]+\:id\(s32\)]] = OpExtInst [[void_type]], 3, 2, {{%[0-9]+\:[a-z0-9\(\)]+}}, [[i32_8]], [[enc_unsigned_char]], [[i32_0]]
+; CHECK-MIR-DAG:   [[unsigned_long:%[0-9]+\:id\(s32\)]] = OpExtInst [[void_type]], 3, 2, {{%[0-9]+\:[a-z0-9\(\)]+}}, [[i32_64]], [[enc_unsigned]], [[i32_0]]
+; CHECK-MIR-DAG:   [[float:%[0-9]+\:id\(s32\)]] = OpExtInst [[void_type]], 3, 2, {{%[0-9]+\:[a-z0-9\(\)]+}}, [[i32_32]], [[enc_float]], [[i32_0]]
+; CHECK-MIR-DAG:   [[double:%[0-9]+\:id\(s32\)]] = OpExtInst [[void_type]], 3, 2, {{%[0-9]+\:[a-z0-9\(\)]+}}, [[i32_64]], [[enc_float]], [[i32_0]]
+; CHECK-MIR-DAG:   [[int:%[0-9]+\:id\(s32\)]] = OpExtInst [[void_type]], 3, 2, {{%[0-9]+\:[a-z0-9\(\)]+}}, [[i32_32]], [[enc_signed]], [[i32_0]]
+; CHECK-MIR:   OpExtInst [[void_type]], 3, 3, [[bool]], [[i32_8]], [[i32_0]]
+; CHECK-MIR:   OpExtInst [[void_type]], 3, 3, [[short]], [[i32_8]], [[i32_0]]
+; CHECK-MIR:   OpExtInst [[void_type]], 3, 3, [[char]], [[i32_8]], [[i32_0]]
+; CHECK-MIR:   OpExtInst [[void_type]], 3, 3, [[long]], [[i32_8]], [[i32_0]]
+; CHECK-MIR:   OpExtInst [[void_type]], 3, 3, [[unsigned_int]], [[i32_8]], [[i32_0]]
+; CHECK-MIR:   OpExtInst [[void_type]], 3, 3, [[unsigned_short]], [[i32_8]], [[i32_0]]
+; CHECK-MIR:   OpExtInst [[void_type]], 3, 3, [[unsigned_char]], [[i32_8]], [[i32_0]]
+; CHECK-MIR:   OpExtInst [[void_type]], 3, 3, [[unsigned_long]], [[i32_8]], [[i32_0]]
+; CHECK-MIR:   OpExtInst [[void_type]], 3, 3, [[float]], [[i32_8]], [[i32_0]]
+; CHECK-MIR:   OpExtInst [[void_type]], 3, 3, [[double]], [[i32_8]], [[i32_0]]
+; CHECK-MIR:   OpExtInst [[void_type]], 3, 3, [[int]], [[i32_8]], [[i32_0]]
+
+; CHECK-SPIRV:	[[i32type:%[0-9]+]] = OpTypeInt 32 0
+; CHECK-SPIRV-DAG:	[[i32_8:%[0-9]+]] = OpConstant [[i32type]] 8
+; CHECK-SPIRV-DAG:	[[i32_0:%[0-9]+]] = OpConstant [[i32type]] 0
+; CHECK-SPIRV-DAG:	[[enc_signed_char:%[0-9]+]] = OpConstant [[i32type]] 5
+; CHECK-SPIRV-DAG:	[[enc_float:%[0-9]+]] = OpConstant [[i32type]] 3
+; CHECK-SPIRV-DAG:	[[enc_boolean:%[0-9]+]] = OpConstant [[i32type]] 2
+; CHECK-SPIRV-DAG:	[[i32_16:%[0-9]+]] = OpConstant [[i32type]] 16
+; CHECK-SPIRV-DAG:	[[enc_signed:%[0-9]+]] = OpConstant [[i32type]] 4
+; CHECK-SPIRV-DAG:	[[i32_64:%[0-9]+]] = OpConstant [[i32type]] 64
+; CHECK-SPIRV-DAG:	[[i32_32:%[0-9]+]] = OpConstant [[i32type]] 32
+; CHECK-SPIRV-DAG:	[[enc_unsigned:%[0-9]+]] = OpConstant [[i32type]] 6
+; CHECK-SPIRV-DAG:	[[enc_unsigned_char:%[0-9]+]] = OpConstant [[i32type]] 7
+; CHECK-SPIRV-DAG:	[[bool:%[0-9]+]] = OpExtInst {{%[0-9]+ %[0-9]+}} DebugTypeBasic {{%[0-9]+}} [[i32_8]] [[enc_boolean]] [[i32_0]]
+; CHECK-SPIRV-DAG:	[[short:%[0-9]+]] = OpExtInst {{%[0-9]+ %[0-9]+}} DebugTypeBasic {{%[0-9]+}} [[i32_16]] [[enc_signed]] [[i32_0]]
+; CHECK-SPIRV-DAG:	[[char:%[0-9]+]] = OpExtInst {{%[0-9]+ %[0-9]+}} DebugTypeBasic {{%[0-9]+}} [[i32_8]] [[enc_signed_char]] [[i32_0]]
+; CHECK-SPIRV-DAG:	[[long:%[0-9]+]] = OpExtInst {{%[0-9]+ %[0-9]+}} DebugTypeBasic {{%[0-9]+}} [[i32_64]] [[enc_signed]] [[i32_0]]
+; CHECK-SPIRV-DAG:	[[unsigned_int:%[0-9]+]] = OpExtInst {{%[0-9]+ %[0-9]+}} DebugTypeBasic {{%[0-9]+}} [[i32_32]] [[enc_unsigned]] [[i32_0]]
+; CHECK-SPIRV-DAG:	[[unsigned_short:%[0-9]+]] = OpExtInst {{%[0-9]+ %[0-9]+}} DebugTypeBasic {{%[0-9]+}} [[i32_16]] [[enc_unsigned]] [[i32_0]]
+; CHECK-SPIRV-DAG:	[[unsigned_char:%[0-9]+]] = OpExtInst {{%[0-9]+ %[0-9]+}} DebugTypeBasic {{%[0-9]+}} [[i32_8]] [[enc_unsigned_char]] [[i32_0]]
+; CHECK-SPIRV-DAG:	[[unsigned_long:%[0-9]+]] = OpExtInst {{%[0-9]+ %[0-9]+}} DebugTypeBasic {{%[0-9]+}} [[i32_64]] [[enc_unsigned]] [[i32_0]]
+; CHECK-SPIRV-DAG:	[[float:%[0-9]+]] = OpExtInst {{%[0-9]+ %[0-9]+}} DebugTypeBasic {{%[0-9]+}} [[i32_32]] [[enc_float]] [[i32_0]]
+; CHECK-SPIRV-DAG:	[[double:%[0-9]+]] = OpExtInst {{%[0-9]+ %[0-9]+}} DebugTypeBasic {{%[0-9]+}} [[i32_64]] [[enc_float]] [[i32_0]]
+; CHECK-SPIRV-DAG:	[[int:%[0-9]+]] = OpExtInst {{%[0-9]+ %[0-9]+}} DebugTypeBasic {{%[0-9]+}} [[i32_32]] [[enc_signed]] [[i32_0]]
+; CHECK-SPIRV-DAG:	OpExtInst {{%[0-9]+ %[0-9]+}} DebugTypePointer [[bool]] [[i32_8]] [[i32_0]]
+; CHECK-SPIRV-DAG:	OpExtInst {{%[0-9]+ %[0-9]+}} DebugTypePointer [[short]] [[i32_8]] [[i32_0]]
+; CHECK-SPIRV-DAG:	OpExtInst {{%[0-9]+ %[0-9]+}} DebugTypePointer [[char]] [[i32_8]] [[i32_0]]
+; CHECK-SPIRV-DAG:	OpExtInst {{%[0-9]+ %[0-9]+}} DebugTypePointer [[long]] [[i32_8]] [[i32_0]]
+; CHECK-SPIRV-DAG:	OpExtInst {{%[0-9]+ %[0-9]+}} DebugTypePointer [[unsigned_int]] [[i32_8]] [[i32_0]]
+; CHECK-SPIRV-DAG:	OpExtInst {{%[0-9]+ %[0-9]+}} DebugTypePointer [[unsigned_short]] [[i32_8]] [[i32_0]]
+; CHECK-SPIRV-DAG:	OpExtInst {{%[0-9]+ %[0-9]+}} DebugTypePointer [[unsigned_char]] [[i32_8]] [[i32_0]]
+; CHECK-SPIRV-DAG:	OpExtInst {{%[0-9]+ %[0-9]+}} DebugTypePointer [[unsigned_long]] [[i32_8]] [[i32_0]]
+; CHECK-SPIRV-DAG:	OpExtInst {{%[0-9]+ %[0-9]+}} DebugTypePointer [[float]] [[i32_8]] [[i32_0]]
+; CHECK-SPIRV-DAG:	OpExtInst {{%[0-9]+ %[0-9]+}} DebugTypePointer [[double]] [[i32_8]] [[i32_0]]
+; CHECK-SPIRV-DAG:	OpExtInst {{%[0-9]+ %[0-9]+}} DebugTypePointer [[int]] [[i32_8]] [[i32_0]]
+
+; CHECK-OPTION-NOT: DebugTypePointer
+
+@gi0 = dso_local addrspace(1) global ptr addrspace(4) null, align 4, !dbg !0
+@gv0 = dso_local addrspace(1) global ptr addrspace(4) null, align 4, !dbg !5
+
+define spir_func i32 @test0() !dbg !17 {
+  %1 = alloca ptr addrspace(4), align 4
+  %2 = alloca ptr addrspace(4), align 4
+  %3 = alloca ptr addrspace(4), align 4
+  %4 = alloca ptr addrspace(4), align 4
+  %5 = alloca ptr addrspace(4), align 4
+  %6 = alloca ptr addrspace(4), align 4
+  %7 = alloca ptr addrspace(4), align 4
+  %8 = alloca ptr addrspace(4), align 4
+  %9 = alloca ptr addrspace(4), align 4
+  %10 = alloca ptr addrspace(4), align 4
+  %11 = alloca ptr addrspace(4), align 4
+  %12 = alloca ptr addrspace(4), align 4
+  %13 = alloca [8 x i32], align 4
+    #dbg_declare(ptr %1, !21, !DIExpression(DW_OP_constu, 0, DW_OP_swap, DW_OP_xderef), !24)
+  store ptr addrspace(4) null, ptr %1, align 4, !dbg !24
+    #dbg_declare(ptr %2, !25, !DIExpression(DW_OP_constu, 0, DW_OP_swap, DW_OP_xderef), !28)
+  store ptr addrspace(4) null, ptr %2, align 4, !dbg !28
+    #dbg_declare(ptr %3, !29, !DIExpression(DW_OP_constu, 0, DW_OP_swap, DW_OP_xderef), !32)
+  store ptr addrspace(4) null, ptr %3, align 4, !dbg !32
+    #dbg_declare(ptr %4, !33, !DIExpression(DW_OP_constu, 0, DW_OP_swap, DW_OP_xderef), !36)
+  store ptr addrspace(4) null, ptr %4, align 4, !dbg !36
+    #dbg_declare(ptr %5, !37, !DIExpression(DW_OP_constu, 0, DW_OP_swap, DW_OP_xderef), !40)
+  store ptr addrspace(4) null, ptr %5, align 4, !dbg !40
+    #dbg_declare(ptr %6, !41, !DIExpression(DW_OP_constu, 0, DW_OP_swap, DW_OP_xderef), !44)
+  store ptr addrspace(4) null, ptr %6, align 4, !dbg !44
+    #dbg_declare(ptr %7, !45, !DIExpression(DW_OP_constu, 0, DW_OP_swap, DW_OP_xderef), !48)
+  store ptr addrspace(4) null, ptr %7, align 4, !dbg !48
+    #dbg_declare(ptr %8, !49, !DIExpression(DW_OP_constu, 0, DW_OP_swap, DW_OP_xderef), !52)
+  store ptr addrspace(4) null, ptr %8, align 4, !dbg !52
+    #dbg_declare(ptr %9, !53, !DIExpression(DW_OP_constu, 0, DW_OP_swap, DW_OP_xderef), !56)
+  store ptr addrspace(4) null, ptr %9, align 4, !dbg !56
+    #dbg_declare(ptr %10, !57, !DIExpression(DW_OP_constu, 0, DW_OP_swap, DW_OP_xderef), !60)
+  store ptr addrspace(4) null, ptr %10, align 4, !dbg !60
+    #dbg_declare(ptr %11, !61, !DIExpression(DW_OP_constu, 0, DW_OP_swap, DW_OP_xderef), !62)
+  store ptr addrspace(4) null, ptr %11, align 4, !dbg !62
+    #dbg_declare(ptr %12, !63, !DIExpression(DW_OP_constu, 0, DW_OP_swap, DW_OP_xderef), !64)
+  %14 = load ptr addrspace(4), ptr %11, align 4, !dbg !65
+  store ptr addrspace(4) %14, ptr %12, align 4, !dbg !64
+    #dbg_declare(ptr %13, !66, !DIExpression(DW_OP_constu, 0, DW_OP_swap, DW_OP_xderef), !70)
+  ret i32 0, !dbg !71
+}
+
+define spir_func i32 @test1() !dbg !72 {
+  %1 = alloca ptr addrspace(4), align 4
+  %2 = alloca ptr addrspace(4), align 4
+  %3 = alloca ptr addrspace(4), align 4
+  %4 = alloca ptr addrspace(4), align 4
+  %5 = alloca ptr addrspace(4), align 4
+  %6 = alloca ptr addrspace(4), align 4
+  %7 = alloca ptr addrspace(4), align 4
+  %8 = alloca ptr addrspace(4), align 4
+  %9 = alloca ptr addrspace(4), align 4
+  %10 = alloca ptr addrspace(4), align 4
+  %11 = alloca ptr addrspace(4), align 4
+  %12 = alloca ptr addrspace(4), align 4
+  %13 = alloca [8 x i32], align 4
+    #dbg_declare(ptr %1, !73, !DIExpression(DW_OP_constu, 0, DW_OP_swap, DW_OP_xderef), !74)
+  store ptr addrspace(4) null, ptr %1, align 4, !dbg !74
+    #dbg_declare(ptr %2, !75, !DIExpression(DW_OP_constu, 0, DW_OP_swap, DW_OP_xderef), !76)
+  store ptr addrspace(4) null, ptr %2, align 4, !dbg !76
+    #dbg_declare(ptr %3, !77, !DIExpression(DW_OP_constu, 0, DW_OP_swap, DW_OP_xderef), !78)
+  store ptr addrspace(4) null, ptr %3, align 4, !dbg !78
+    #dbg_declare(ptr %4, !79, !DIExpression(DW_OP_constu, 0, DW_OP_swap, DW_OP_xderef), !80)
+  store ptr addrspace(4) null, ptr %4, align 4, !dbg !80
+    #dbg_declare(ptr %5, !81, !DIExpression(DW_OP_constu, 0, DW_OP_swap, DW_OP_xderef), !82)
+  store ptr addrspace(4) null, ptr %5, align 4, !dbg !82
+    #dbg_declare(ptr %6, !83, !DIExpression(DW_OP_constu, 0, DW_OP_swap, DW_OP_xderef), !84)
+  store ptr addrspace(4) null, ptr %6, align 4, !dbg !84
+    #dbg_declare(ptr %7, !85, !DIExpression(DW_OP_constu, 0, DW_OP_swap, DW_OP_xderef), !86)
+  store ptr addrspace(4) null, ptr %7, align 4, !dbg !86
+    #dbg_declare(ptr %8, !87, !DIExpression(DW_OP_constu, 0, DW_OP_swap, DW_OP_xderef), !88)
+  store ptr addrspace(4) null, ptr %8, align 4, !dbg !88
+    #dbg_declare(ptr %9, !89, !DIExpression(DW_OP_constu, 0, DW_OP_swap, DW_OP_xderef), !90)
+  store ptr addrspace(4) null, ptr %9, align 4, !dbg !90
+    #dbg_declare(ptr %10, !91, !DIExpression(DW_OP_constu, 0, DW_OP_swap, DW_OP_xderef), !92)
+  store ptr addrspace(4) null, ptr %10, align 4, !dbg !92
+    #dbg_declare(ptr %11, !93, !DIExpression(DW_OP_constu, 0, DW_OP_swap, DW_OP_xderef), !94)
+  store ptr addrspace(4) null, ptr %11, align 4, !dbg !94
+    #dbg_declare(ptr %12, !95, !DIExpression(DW_OP_constu, 0, DW_OP_swap, DW_OP_xderef), !96)
+  %14 = load ptr addrspace(4), ptr %11, align 4, !dbg !97
+  store ptr addrspace(4) %14, ptr %12, align 4, !dbg !96
+    #dbg_declare(ptr %13, !98, !DIExpression(DW_OP_constu, 0, DW_OP_swap, DW_OP_xderef), !99)
+  ret i32 0, !dbg !100
+}
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!10, !11, !12, !13}
+!opencl.ocl.version = !{!14}
+!opencl.cxx.version = !{!15}
+!opencl.spir.version = !{!14}
+!llvm.ident = !{!16}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression(DW_OP_constu, 1, DW_OP_swap, DW_OP_xderef))
+!1 = distinct !DIGlobalVariable(name: "gi0", scope: !2, file: !3, line: 1, type: !8, isLocal: false, isDefinition: true)
+!2 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !3, producer: "clang version XX.X.XXXX (FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, globals: !4, splitDebugInlining: false, nameTableKind: None)
+!3 = !DIFile(filename: "example.cpp", directory: "/AAAAAAAAAA/BBBBBBBB/CCCCCCCCC", checksumkind: CSK_MD5, checksum: "FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF")
+!4 = !{!0, !5}
+!5 = !DIGlobalVariableExpression(var: !6, expr: !DIExpression(DW_OP_constu, 1, DW_OP_swap, DW_OP_xderef))
+!6 = distinct !DIGlobalVariable(name: "gv0", scope: !2, file: !3, line: 3, type: !7, isLocal: false, isDefinition: true)
+!7 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: null, size: 32, dwarfAddressSpace: 4)
+!8 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !9, size: 32, dwarfAddressSpace: 4)
+!9 = !DIBasicType(name: "int", size: 32, encodi...
[truncated]

@bwlodarcz
Copy link
Contributor Author

@michalpaszkowski @VyacheslavLevytskyy @Keenuts @MrSidims Please review. Thanks.

@@ -160,7 +175,6 @@ bool SPIRVEmitNonSemanticDI::emitGlobalDI(MachineFunction &MF) {
return StrReg;
};

// Emit OpString with FilePath which is required by DebugSource
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: if we are about to remove the comments - lets do it in a single PR aka here

llvm/lib/Target/SPIRV/SPIRVEmitNonSemanticDI.cpp Outdated Show resolved Hide resolved

if (PointerDerivedTypes.size()) {
const Register GenericStorageClass =
GR->buildConstantInt(8, MIRBuilder, I32Ty, false);
Copy link
Contributor

@MrSidims MrSidims Sep 24, 2024

Choose a reason for hiding this comment

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

Generic storage class is enabled with a Addresses capability aka non-default. I'm fine with such behavior, but would be nice to hear from people work on Shader SPIR-V what would they prefer as the default, @VyacheslavLevytskyy may I ask you to tag them here?

also a nit: 8 -> StorageClass::Generic (or other appropriate enum value defined for SPIR-V backend)

@bwlodarcz bwlodarcz force-pushed the spirv_debug_type_pointer branch 2 times, most recently from 80d33eb to 428e06e Compare September 27, 2024 12:17
Implementation of DI instruction from NonSemantic.Shader.DebugInfo.100.
@VyacheslavLevytskyy
Copy link
Contributor

We have all implementation within just one long SPIRVEmitNonSemanticDI::emitGlobalDI(), and it becomes hard to trace all changes and the logic overall, so probably it makes sense to refactor it, breaking into smaller functions, structuring the code?

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 am okay with this pull request being merged without the major refactoring -- considering there may be more points for discussion and there are changes building upon this pull request. @bwlodarcz Would you be able to refactor emitGlobalDI in the next pull request? I think it would be best if you would refactor the function since you have the best orientation on what would be needed next and how to split up the logic for later reuse.

; RUN: llc --verify-machineinstrs --spv-emit-nonsemantic-debug-info --spirv-ext=+SPV_KHR_non_semantic_info --print-after=spirv-nonsemantic-debug-info -O0 -mtriple=spirv64-unknown-unknown %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-MIR
; RUN: llc --verify-machineinstrs --spv-emit-nonsemantic-debug-info --spirv-ext=+SPV_KHR_non_semantic_info -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s --check-prefix=CHECK-SPIRV
; RUN: llc --verify-machineinstrs -O0 -mtriple=spirv64-unknown-unknown --spirv-ext=+SPV_KHR_non_semantic_info %s -o - | FileCheck %s --check-prefix=CHECK-OPTION
; When type is void * the spirv-val incorrectly raises an error when DebugInfoNone is set as <id> Base Type argument of DebugTypePointer.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see a way to use DebugInfoNone as <id> Base Type argument of DebugTypePointer. In the specification we read Base Type is <id> of debugging instruction which represents the pointee type.. There is no explicit statement in the description of the Instruction that Id may be substituted by DebugInfoNone.

Please see KhronosGroup/SPIRV-Registry#280 (comment) for detailed comments from the author of the specification and #106980 (comment) for a similar discussion in one of previous PR's.

I don't object against proceeding with this PR as is, given that this issue will be addressed in the next PR. Please change the description of the issue above from "... spirv-val incorrectly raises an error ..." to a brief TODO statement, so that the test case holds that it's a TODO.

Copy link
Contributor Author

@bwlodarcz bwlodarcz Oct 4, 2024

Choose a reason for hiding this comment

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

I don't see a way to use DebugInfoNone as <id> Base Type argument of DebugTypePointer. In the specification we read Base Type is <id> of debugging instruction which represents the pointee type.. There is no explicit statement in the description of the Instruction that Id may be substituted by DebugInfoNone.

Please see KhronosGroup/SPIRV-Registry#280 (comment) for detailed comments from the author of the specification and #106980 (comment) for a similar discussion in one of previous PR's.

I don't object against proceeding with this PR as is, given that this issue will be addressed in the next PR. Please change the description of the issue above from "... spirv-val incorrectly raises an error ..." to a brief TODO statement, so that the test case holds that it's a TODO.

@VyacheslavLevytskyy I don't see any possible sensible solution to avoid it.
The problem is that's basically impossible to handle void * scenario if we don't utilize DebugInfoNone. The void * is basically a type erasure in C and C++ and just plainly doesn't have BaseType.
If we want to create composite Null BaseType then what BitLength or StorageClass should it have?
Isn't it exactly the purpose of DebugInfoNone to be used in a cases like this?

DebugInfoNone

Other instructions can refer to this one in case the debugging information is unknown, not available, or not applicable.

Matches exact definition of the problem which we have here.
DI info is unknown, not available and even not applicable - all three.

From DebugTypeEnum

Underlying Type is a debugging instruction that describes the underlying type of the enum in the source program. If the underlying type is not specified in the source program, this operand must refer to DebugInfoNone.

So we have precedence. The only difference is that in DebugTypeEnum we have it described and in this we aren't.
This is a somewhat misguided approach taken by spec. If you have null type then you should create a general rule in which cases it's applicable - if you want to specify every one instruction or argument where it can or can not be needed then you are certainly missing some unforeseen cases and making holes in spec and making divergence between implementations.
Which comes back to the linked Registry discussion and pointed out by @MrSidims. If what the author saying is true then the spec needs a LOT more default cases and null substitutions to handle.

Copy link
Contributor

Choose a reason for hiding this comment

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

@MrSidims What would you advise in this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

It seems like the spec is ambiguous here. Personally, I think using DebugInfoNone makes most sense, but I understand Baldur's point and it would be poor idea for consumers to guess. Let's maybe just add TODO(#109287): to this line to revisit the problem in the future. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

For me it also looks good to change this line into TODO and postpone addressing it to the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@michalpaszkowski @VyacheslavLevytskyy I submitted a PR to the standard to allow for the substitution - KhronosGroup/SPIRV-Registry#287.
Following your suggestion, I change it to TODO.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you!

@bwlodarcz
Copy link
Contributor Author

@VyacheslavLevytskyy @michalpaszkowski It isn't an accident that the next instruction which I'm implementing is DebugLine. This is an equivalent of .debug_line from DWARF and require architectural changes which will trigger necessary refactoring as well.
The abstraction level will raise matching the increasing complexity of the problem.

@michalpaszkowski michalpaszkowski merged commit ae5ee97 into llvm:main Oct 8, 2024
8 of 10 checks passed
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.

5 participants