[SPIRV] Generate debug info from original source

docs/ReleaseNotes.md

@@ -74,6 +74,12 @@ line upon naming the release. Refer to previous for appropriate section names.
 - Fixed an optimizer crash when scalarizing an out-of-bounds vector access
   [#8940](https://github.com/microsoft/DirectXShaderCompiler/issues/8940).
 
+#### SPIR-V
+
+- SPIR-V debug info now reports source columns from the original source
+  rather than from preprocessed output
+  [#9022](https://github.com/microsoft/DirectXShaderCompiler/issues/9022).
+
 ### Upcoming Preview Release
 
 These changes apply to experimental preview shader models only and will not be

tools/clang/lib/SPIRV/SpirvEmitter.cpp

@@ -31,6 +31,7 @@
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringSet.h"
 #include "llvm/Support/Casting.h"
 
 #ifdef SUPPORT_QUERY_GIT_COMMIT_INFO
@@ -847,18 +848,29 @@ void SpirvEmitter::HandleTranslationUnit(ASTContext &context) {
   // Add source instruction(s)
   if (spirvOptions.debugInfoSource || spirvOptions.debugInfoFile) {
     std::vector<llvm::StringRef> fileNames;
-    fileNames.clear();
+    llvm::StringSet<> seenFiles;
     const auto &sm = context.getSourceManager();
-    // Add each include file from preprocessor output
-    for (unsigned int i = 0; i < sm.getNumLineTableFilenames(); i++) {
-      llvm::StringRef file = sm.getLineTableFilename(i);
-      if (spirvOptions.debugInfoVulkan) {
-        getOrCreateRichDebugInfoImpl(file);
-      } else {
+    auto addFile = [&](llvm::StringRef file) {
+      if (!file.empty() && seenFiles.insert(file).second)
         fileNames.push_back(file);
-      }
-    }
-    if (!spirvOptions.debugInfoVulkan) {
+    };
+    // Add every file entered by the preprocessor.
+    for (unsigned i = 0, e = sm.local_sloc_entry_size(); i != e; ++i) {
+      const SrcMgr::SLocEntry &entry = sm.getLocalSLocEntry(i);
+      if (!entry.isFile())
+        continue;
+      const SrcMgr::ContentCache *cache = entry.getFile().getContentCache();
+      if (cache && cache->OrigEntry)
+        addFile(cache->OrigEntry->getName());
+    }
+    // Add each file named by a #line directive.
+    if (sm.hasLineTable())
+      for (unsigned int i = 0; i < sm.getNumLineTableFilenames(); i++)
+        addFile(sm.getLineTableFilename(i));
+    if (spirvOptions.debugInfoVulkan) {
+      for (llvm::StringRef file : fileNames)
+        getOrCreateRichDebugInfoImpl(file);
+    } else {
       spvBuilder.setDebugSource(spvContext.getMajorVersion(),
                                 spvContext.getMinorVersion(), fileNames);
     }

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

@@ -25,15 +25,15 @@ cbuffer AnotherCBuffer : register(b2) {
 
 // CHECK: [[S:%[0-9]+]] = OpExtInst %void [[ext]] DebugTypeComposite {{%[0-9]+}} Structure {{%[0-9]+}} 3 8 {{%[0-9]+}} {{%[0-9]+}} %uint_128 FlagIsProtected|FlagIsPrivate [[f1:%[0-9]+]] [[f2:%[0-9]+]]
 // CHECK: [[f2]] = OpExtInst %void [[ext]] DebugTypeMember {{%[0-9]+}} {{%[0-9]+}} {{%[0-9]+}} 5 12 [[S]] %uint_32 %uint_96
-// CHECK: [[f1]] = OpExtInst %void [[ext]] DebugTypeMember {{%[0-9]+}} {{%[0-9]+}} {{%[0-9]+}} 4 11 [[S]] %uint_0 %uint_32
+// CHECK: [[f1]] = OpExtInst %void [[ext]] DebugTypeMember {{%[0-9]+}} {{%[0-9]+}} {{%[0-9]+}} 4 12 [[S]] %uint_0 %uint_32
 
 // CHECK: [[MyCbuffer:%[0-9]+]] = OpExtInst %void [[ext]] DebugTypeComposite {{%[0-9]+}} Structure {{%[0-9]+}} 8 9 {{%[0-9]+}} {{%[0-9]+}} %uint_1280 FlagIsProtected|FlagIsPrivate [[a:%[0-9]+]] [[b:%[0-9]+]] [[c:%[0-9]+]] [[d:%[0-9]+]] [[s:%[0-9]+]] [[t:%[0-9]+]]
-// CHECK: [[t]] = OpExtInst %void [[ext]] DebugTypeMember {{%[0-9]+}} {{%[0-9]+}} {{%[0-9]+}} 14 11 [[MyCbuffer]] %uint_768 %uint_512
-// CHECK: [[s]] = OpExtInst %void [[ext]] DebugTypeMember {{%[0-9]+}} [[S]] {{%[0-9]+}} 13 7 [[MyCbuffer]] %uint_640 %uint_128
+// CHECK: [[t]] = OpExtInst %void [[ext]] DebugTypeMember {{%[0-9]+}} {{%[0-9]+}} {{%[0-9]+}} 14 14 [[MyCbuffer]] %uint_768 %uint_512
+// CHECK: [[s]] = OpExtInst %void [[ext]] DebugTypeMember {{%[0-9]+}} [[S]] {{%[0-9]+}} 13 14 [[MyCbuffer]] %uint_640 %uint_128
 // CHECK: [[d]] = OpExtInst %void [[ext]] DebugTypeMember {{%[0-9]+}} {{%[0-9]+}} {{%[0-9]+}} 12 14 [[MyCbuffer]] %uint_128 %uint_512
-// CHECK: [[c]] = OpExtInst %void [[ext]] DebugTypeMember {{%[0-9]+}} {{%[0-9]+}} {{%[0-9]+}} 11 11 [[MyCbuffer]] %uint_64 %uint_64
-// CHECK: [[b]] = OpExtInst %void [[ext]] DebugTypeMember {{%[0-9]+}} {{%[0-9]+}} {{%[0-9]+}} 10 9 [[MyCbuffer]] %uint_32 %uint_32
-// CHECK: [[a]] = OpExtInst %void [[ext]] DebugTypeMember {{%[0-9]+}} {{%[0-9]+}} {{%[0-9]+}} 9 10 [[MyCbuffer]] %uint_0 %uint_32
+// CHECK: [[c]] = OpExtInst %void [[ext]] DebugTypeMember {{%[0-9]+}} {{%[0-9]+}} {{%[0-9]+}} 11 14 [[MyCbuffer]] %uint_64 %uint_64
+// CHECK: [[b]] = OpExtInst %void [[ext]] DebugTypeMember {{%[0-9]+}} {{%[0-9]+}} {{%[0-9]+}} 10 14 [[MyCbuffer]] %uint_32 %uint_32
+// CHECK: [[a]] = OpExtInst %void [[ext]] DebugTypeMember {{%[0-9]+}} {{%[0-9]+}} {{%[0-9]+}} 9 14 [[MyCbuffer]] %uint_0 %uint_32
 
 // CHECK: {{%[0-9]+}} = OpExtInst %void [[ext]] DebugGlobalVariable {{%[0-9]+}} {{%[0-9]+}} {{%[0-9]+}} 17 9 {{%[0-9]+}} {{%[0-9]+}} %AnotherCBuffer
 // CHECK: {{%[0-9]+}} = OpExtInst %void [[ext]] DebugGlobalVariable {{%[0-9]+}} {{%[0-9]+}} {{%[0-9]+}} 8 9 {{%[0-9]+}} {{%[0-9]+}} %MyCbuffer

tools/clang/test/CodeGenSPIRV/spirv.debug.columns.hlsl

@@ -0,0 +1,33 @@
+// RUN: %dxc -T ps_6_0 -E main -spirv -fspv-debug=vulkan-with-source %s | FileCheck %s
+
+#include "spirv.debug.columns.include.hlsli"
+#include "spirv.debug.columns.include.hlsli"
+
+#define DEFINE_CB(n)\
+cbuffer CB##n : register(b0, space##n) {\
+    float4 dummy_1;\
+}
+
+DEFINE_CB(1)
+
+float4 main(float4 c : COLOR) : SV_Target0 {
+  /* a comment that preprocessing removes */ float v = SCALE_AND_BIAS_THE_VALUE(c.x); float w = v;
+  return OUT_COL * w + dummy_1;
+}
+
+// Debug columns refer to the original source, including on lines that contain
+// a macro expansion or a comment, a cbuffer declared through a macro is located
+// at its expansion, and the define-only header included twice gets one
+// DebugSource.
+
+// CHECK:     [[inc:%[0-9]+]] = OpString "{{.*}}spirv.debug.columns.include.hlsli"
+// CHECK:     [[w:%[0-9]+]] = OpString "w"
+// CHECK:     [[v:%[0-9]+]] = OpString "v"
+// CHECK:     [[member:%[0-9]+]] = OpString "dummy_1"
+// CHECK:     [[type:%[0-9]+]] = OpString "type.CB1"
+// CHECK:     DebugSource [[inc]]
+// CHECK-NOT: DebugSource [[inc]]
+// CHECK:     DebugLocalVariable [[w]] {{%[0-9]+}} {{%[0-9]+}} %uint_14 %uint_93
+// CHECK:     DebugLocalVariable [[v]] {{%[0-9]+}} {{%[0-9]+}} %uint_14 %uint_52
+// CHECK:     DebugTypeMember [[member]] {{%[0-9]+}} {{%[0-9]+}} %uint_11 %uint_1
+// CHECK:     DebugTypeComposite [[type]] %uint_1 {{%[0-9]+}} %uint_11 %uint_1

tools/clang/test/CodeGenSPIRV/spirv.debug.columns.include.hlsli

@@ -0,0 +1,2 @@
+#define SCALE_AND_BIAS_THE_VALUE(x) (x * 2.0f + 1.0f)
+#define OUT_COL float4(1, 0, 1, 1)

tools/clang/tools/dxcompiler/dxcompilerobj.cpp

@@ -609,10 +609,8 @@ class DxcCompiler : public IDxcCompiler3,
 
 #ifdef ENABLE_SPIRV_CODEGEN
       // We want to embed the original source code in the final SPIR-V if
-      // debug information is enabled. But the compiled source requires
-      // pre-seeding with #line directives. We invoke Preprocess() here
-      // first for such case. Then we invoke the compilation process over the
-      // preprocessed source code.
+      // debug information is enabled. In-memory source cannot be re-read
+      // from the file system, so keep a copy of it.
       if (!isPreprocessing && opts.GenSPIRV && opts.DebugInfo) {
         // Convert source code encoding
         CComPtr<IDxcBlobUtf8> pOrigUtf8Source;
@@ -621,23 +619,6 @@ class DxcCompiler : public IDxcCompiler3,
         opts.SpirvOptions.origSource.assign(
             static_cast<const char *>(pOrigUtf8Source->GetStringPointer()),
             pOrigUtf8Source->GetStringLength());
-
-        CComPtr<IDxcResult> pSrcCodeResult;
-        std::vector<LPCWSTR> PreprocessArgs;
-        PreprocessArgs.reserve(argCount + 1);
-        PreprocessArgs.assign(pArguments, pArguments + argCount);
-        PreprocessArgs.push_back(L"-P");
-        PreprocessArgs.push_back(L"-Fi");
-        PreprocessArgs.push_back(L"preprocessed.hlsl");
-        IFT(Compile(pSource, PreprocessArgs.data(), PreprocessArgs.size(),
-                    pIncludeHandler, IID_PPV_ARGS(&pSrcCodeResult)));
-        HRESULT status;
-        IFT(pSrcCodeResult->GetStatus(&status));
-        if (SUCCEEDED(status)) {
-          pSourceEncoding.Release();
-          IFT(pSrcCodeResult->GetOutput(
-              DXC_OUT_HLSL, IID_PPV_ARGS(&pSourceEncoding), nullptr));
-        }
       }
 #endif // ENABLE_SPIRV_CODEGEN