[SPIR-V] Fix debug instruction with cbuffer + FXC (microsoft#6531)

When the requested constant buffer layout is DX, some type lowering can
become complex (Nx1 matrices for ex). To simplify the lowering, the
backend "clones" those variables:
 - on one end, we expose the correct layout
- but then, we copy this to a local variable to have a different layout
compatible with the operations we usually do on such types.

This means the type can sometimes be an HybridStructType, which is not
completely lowered, or a normal lowerer SPIR-V type. Both should be
allowed, but some codepaths had to be updated to reflect this.

---------

Signed-off-by: Nathan Gauër <[email protected]>

Author: Keenuts

tools/clang/lib/SPIRV/DeclResultIdMapper.cpp

@@ -1481,12 +1481,28 @@ SpirvVariable *DeclResultIdMapper::createCTBuffer(const HLSLBufferDecl *decl) {
         decl->getAttr<VKBindingAttr>(), decl->getAttr<VKCounterBindingAttr>());
   }
 
+  if (!spirvOptions.debugInfoRich) {
+    return bufferVar;
+  }
+
   auto *dbgGlobalVar = createDebugGlobalVariable(
       bufferVar, QualType(), decl->getLocation(), decl->getName());
-  if (dbgGlobalVar != nullptr) {
-    // C/TBuffer needs HLSLBufferDecl for debug type lowering.
-    spvContext.registerStructDeclForSpirvType(bufferVar->getResultType(), decl);
-  }
+  assert(dbgGlobalVar);
+  (void)dbgGlobalVar; // For NDEBUG builds.
+
+  auto *resultType = bufferVar->getResultType();
+  // Depending on the requested layout (DX or VK), constant buffers is either a
+  // struct containing every constant fields, or a pointer to the type. This is
+  // caused by the workaround we implemented to support FXC/DX layout. See #3672
+  // for more details.
+  assert(isa<SpirvPointerType>(resultType) ||
+         isa<HybridStructType>(resultType));
+  if (auto *ptr = dyn_cast<SpirvPointerType>(resultType))
+    resultType = ptr->getPointeeType();
+  // Debug type lowering requires the HLSLBufferDecl. Updating the type<>decl
+  // mapping.
+  spvContext.registerStructDeclForSpirvType(resultType, decl);
+
   return bufferVar;
 }
 

tools/clang/lib/SPIRV/LowerTypeVisitor.cpp

@@ -163,15 +163,16 @@ bool LowerTypeVisitor::visitInstruction(SpirvInstruction *instr) {
       debugInstruction->setDebugSpirvType(spirvType);
     } else if (const auto *debugSpirvType =
                    debugInstruction->getDebugSpirvType()) {
-      // When it does not have a QualType, SpirvEmitter or DeclResultIdMapper
-      // generates a hybrid type. In that case, we keep the hybrid type for the
-      // DebugGlobalVariable, not QualType. We have to lower the hybrid type and
-      // update the SpirvType for the DebugGlobalVariable.
-      assert(isa<SpirvDebugGlobalVariable>(debugInstruction) &&
-             isa<HybridType>(debugSpirvType));
-      const SpirvType *loweredSpirvType = lowerType(
-          debugSpirvType, instr->getLayoutRule(), instr->getSourceLocation());
-      debugInstruction->setDebugSpirvType(loweredSpirvType);
+      // When it does not have a QualType, either the type is already lowered,
+      // or it's an HybridStructType we should lower.
+      assert(isa<SpirvDebugGlobalVariable>(debugInstruction));
+      if (isa<HybridType>(debugSpirvType)) {
+        const SpirvType *loweredSpirvType = lowerType(
+            debugSpirvType, instr->getLayoutRule(), instr->getSourceLocation());
+        debugInstruction->setDebugSpirvType(loweredSpirvType);
+      } else {
+        debugInstruction->setDebugSpirvType(debugSpirvType);
+      }
     }
   }
 

tools/clang/test/CodeGenSPIRV/rich.debug.cbuffer.dxlayout.hlsl

@@ -0,0 +1,25 @@
+// RUN: %dxc -T vs_6_6 -spirv -fvk-use-dx-layout -fspv-target-env=vulkan1.3  -fspv-extension=SPV_KHR_non_semantic_info -fspv-debug=vulkan-with-source -fcgl %s | FileCheck %s
+
+// CHECK: [[dbg_ty_name:%[0-9]+]] = OpString "type.cbuff"
+// CHECK:    [[dbg_name:%[0-9]+]] = OpString "cbuff"
+
+cbuffer cbuff
+{
+    float4 a;
+};
+// CHECK: [[type_cbuff:%[0-9a-zA-Z_]+]] = OpTypeStruct %v4float
+// CHECK: [[ptr_cbuff:%[0-9a-zA-Z_]+]]  = OpTypePointer Uniform [[type_cbuff]]
+// CHECK: [[cbuff:%[0-9a-zA-Z_]+]]      = OpVariable [[ptr_cbuff]] Uniform
+
+
+// CHECK:  [[dbg_member:%[0-9]+]] = OpExtInst %void {{%[0-9]+}} DebugTypeMember {{%[0-9]+}} {{%[0-9]+}} {{%[0-9]+}} %uint_8 %uint_12 %uint_0 %uint_128 %uint_3
+// CHECK:      [[dbg_ty:%[0-9]+]] = OpExtInst %void {{%[0-9]+}} DebugTypeComposite [[dbg_ty_name]] %uint_1 {{%[0-9]+}} %uint_6 %uint_9 {{%[0-9]+}} [[dbg_ty_name]] %uint_128 %uint_3 [[dbg_member]]
+// CHECK:                           OpExtInst %void {{%[0-9]+}} DebugGlobalVariable [[dbg_name]] [[dbg_ty]] {{%[0-9]+}} %uint_6 %uint_9 {{%[0-9]+}} [[dbg_name]] %cbuff %uint_8
+
+
+float4 main() : SV_Position
+{
+// CHECK: [[ptr:%[0-9]+]] = OpAccessChain %_ptr_Uniform_v4float [[cbuff]] %int_0
+// CHECK:                   OpLoad %v4float [[ptr]]
+    return a;
+}