From 19660b145d47f6708218e8346ba9071eae9bd739 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Nov 2025 20:19:17 +0000 Subject: [PATCH 1/4] Initial plan From 47c52a71ae19d313a477e78f3bea4495c32ee838 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Nov 2025 20:58:12 +0000 Subject: [PATCH 2/4] Fix DebugGlobalVariable source for #include files and partial fix for import Co-authored-by: zangold-nv <242329104+zangold-nv@users.noreply.github.com> --- source/slang/slang-emit-spirv.cpp | 5 +-- source/slang/slang-ir-link.cpp | 36 ++++++++++++++++--- source/slang/slang-lower-to-ir.cpp | 2 +- .../debug-global-variable-source-import.slang | 35 ++++++++++++++++++ .../spirv/debug-global-variable-source.slang | 35 ++++++++++++++++++ tests/spirv/shader-utils-module.slang | 16 +++++++++ 6 files changed, 122 insertions(+), 7 deletions(-) create mode 100644 tests/spirv/debug-global-variable-source-import.slang create mode 100644 tests/spirv/debug-global-variable-source.slang create mode 100644 tests/spirv/shader-utils-module.slang diff --git a/source/slang/slang-emit-spirv.cpp b/source/slang/slang-emit-spirv.cpp index 07aaf3d2708..da1ff80b92c 100644 --- a/source/slang/slang-emit-spirv.cpp +++ b/source/slang/slang-emit-spirv.cpp @@ -2356,10 +2356,11 @@ struct SPIRVEmitContext : public SourceEmitterBase, public SPIRVEmitSharedContex } auto moduleInst = inst->getModule()->getModuleInst(); - if (!m_defaultDebugSource) - m_defaultDebugSource = debugSource; // Only create DebugCompilationUnit for non-included files auto isIncludedFile = as(debugSource->getIsIncludedFile())->getValue(); + // Only set default debug source to non-included files + if (!m_defaultDebugSource && !isIncludedFile) + m_defaultDebugSource = debugSource; if (!m_mapIRInstToSpvDebugInst.containsKey(moduleInst) && !isIncludedFile) { IRBuilder builder(inst); diff --git a/source/slang/slang-ir-link.cpp b/source/slang/slang-ir-link.cpp index 8ba1f354d3c..e9f3925bb24 100644 --- a/source/slang/slang-ir-link.cpp +++ b/source/slang/slang-ir-link.cpp @@ -2155,6 +2155,7 @@ LinkedIR linkIR(CodeGenContext* codeGenContext) // Clone additional insts that should be included in the linked IR module // even if they are not being directly referenced. + bool isFirstUserModule = true; for (IRModule* irModule : userModules) { for (auto inst : irModule->getGlobalInsts()) @@ -2175,12 +2176,38 @@ LinkedIR linkIR(CodeGenContext* codeGenContext) // to the relevant parameters and cloned via `cloneExtraDecorations`. // In the long run we do not want to *ever* iterate over all the // instructions in all the input modules. - [[fallthrough]]; - case kIROp_DebugSource: - // Need to list all source files in the debug source file list, - // regardless if the source files participate in the line table or not. cloneValue(context, inst); break; + case kIROp_DebugSource: + { + // Need to list all source files in the debug source file list, + // regardless if the source files participate in the line table or not. + // For DebugSource from imported modules (not the first/main module), + // we need to mark them as included files so they don't become the + // default debug source for global variables. + auto debugSource = as(inst); + auto isIncludedFileLit = as(debugSource->getIsIncludedFile()); + + // If this is from an imported module and not already marked as included, + // create a new DebugSource with isIncludedFile=true + if (!isFirstUserModule && isIncludedFileLit && !isIncludedFileLit->getValue()) + { + auto fileName = as(debugSource->getFileName()); + auto source = as(debugSource->getSource()); + auto newDebugSource = context->builder->emitDebugSource( + fileName->getStringSlice(), + source->getStringSlice(), + true); + // Register this as the clone of the original so that references + // to the original will use the new one with isIncludedFile=true + registerClonedValue(context, newDebugSource, inst); + } + else + { + cloneValue(context, inst); + } + } + break; case kIROp_DebugBuildIdentifier: // The debug build identifier won't be referenced by anything, // but we still need to keep it around if it is in the IR. @@ -2188,6 +2215,7 @@ LinkedIR linkIR(CodeGenContext* codeGenContext) break; } } + isFirstUserModule = false; } bool shouldCopyGlobalParams = diff --git a/source/slang/slang-lower-to-ir.cpp b/source/slang/slang-lower-to-ir.cpp index 78be183c304..46dbdebcdad 100644 --- a/source/slang/slang-lower-to-ir.cpp +++ b/source/slang/slang-lower-to-ir.cpp @@ -8019,7 +8019,7 @@ IRInst* getOrEmitDebugSource(IRGenContext* context, PathInfo path) content = UnownedStringSlice((char*)outBlob->getBufferPointer(), outBlob->getBufferSize()); IRBuilder builder(*context->irBuilder); builder.setInsertInto(context->irBuilder->getModule()); - auto debugSrcInst = builder.emitDebugSource(path.foundPath.getUnownedSlice(), content, false); + auto debugSrcInst = builder.emitDebugSource(path.foundPath.getUnownedSlice(), content, true); context->shared->mapSourcePathToDebugSourceInst[path.foundPath] = debugSrcInst; return debugSrcInst; } diff --git a/tests/spirv/debug-global-variable-source-import.slang b/tests/spirv/debug-global-variable-source-import.slang new file mode 100644 index 00000000000..e010729c621 --- /dev/null +++ b/tests/spirv/debug-global-variable-source-import.slang @@ -0,0 +1,35 @@ +//TEST:SIMPLE(filecheck=CHECK):-target spirv -entry main -stage fragment -g2 -emit-spirv-directly + +// Test that DebugGlobalVariable references the correct source file (this main file) +// when using import syntax instead of #include. + +import shader_utils_module; + +struct VertexOutput +{ + float4 position : SV_Position; + float2 texCoord : TEXCOORD0; +} + +[shader("fragment")] +float4 main(VertexOutput input) : SV_Target +{ + float2 uv = input.texCoord; + float distanceFromCenter = calculateDistanceFromCenterModule(uv); + float radialFade = createRadialFadeModule(uv, 0.7); + float4 color = float4( + uv.x * (1.0 - distanceFromCenter * 0.5), + uv.y * radialFade, + uv.x * uv.y, + 1.0 + ); + return color; +} + +// Verify that DebugSource for the main file is defined +// CHECK-DAG: [[MAIN_FILE:%[0-9]+]] = OpString "{{.*}}debug-global-variable-source-import.slang" +// CHECK-DAG: [[SOURCE_MAIN:%[0-9]+]] = OpExtInst %void %{{[0-9]+}} DebugSource [[MAIN_FILE]] %{{[0-9]+}} + +// Verify that DebugGlobalVariable for entry point parameters references the main file source +// The key fix: DebugGlobalVariable should reference the main file source, not the imported file source +// CHECK-DAG: OpExtInst %void %{{[0-9]+}} DebugGlobalVariable {{%[0-9]+}} {{%[0-9]+}} [[SOURCE_MAIN]] %uint_0 %uint_0 diff --git a/tests/spirv/debug-global-variable-source.slang b/tests/spirv/debug-global-variable-source.slang new file mode 100644 index 00000000000..417f65320a0 --- /dev/null +++ b/tests/spirv/debug-global-variable-source.slang @@ -0,0 +1,35 @@ +//TEST:SIMPLE(filecheck=CHECK):-target spirv -entry main -stage fragment -g2 -emit-spirv-directly + +// Test that DebugGlobalVariable references the correct source file (this main file) +// rather than an included header file. + +#include "shader-utils.slang" + +struct VertexOutput +{ + float4 position : SV_Position; + float2 texCoord : TEXCOORD0; +} + +[shader("fragment")] +float4 main(VertexOutput input) : SV_Target +{ + float2 uv = input.texCoord; + float distanceFromCenter = calculateDistanceFromCenter(uv); + float radialFade = createRadialFade(uv, 0.7); + float4 color = float4( + uv.x * (1.0 - distanceFromCenter * 0.5), + uv.y * radialFade, + uv.x * uv.y, + 1.0 + ); + return color; +} + +// Verify that DebugSource for the main file is defined +// CHECK-DAG: [[MAIN_FILE:%[0-9]+]] = OpString "{{.*}}debug-global-variable-source.slang" +// CHECK-DAG: [[SOURCE_MAIN:%[0-9]+]] = OpExtInst %void %{{[0-9]+}} DebugSource [[MAIN_FILE]] %{{[0-9]+}} + +// Verify that DebugGlobalVariable for entry point parameters references the main file source +// The key fix: DebugGlobalVariable should reference the main file source, not the included file source +// CHECK-DAG: OpExtInst %void %{{[0-9]+}} DebugGlobalVariable {{%[0-9]+}} {{%[0-9]+}} [[SOURCE_MAIN]] %uint_0 %uint_0 diff --git a/tests/spirv/shader-utils-module.slang b/tests/spirv/shader-utils-module.slang new file mode 100644 index 00000000000..d055ffbd97b --- /dev/null +++ b/tests/spirv/shader-utils-module.slang @@ -0,0 +1,16 @@ +// Module containing utility functions for debug tests +module shader_utils_module; + +// Helper function to calculate distance from center +public float calculateDistanceFromCenterModule(float2 uv) +{ + float2 center = float2(0.5, 0.5); + return length(uv - center); +} + +// Additional utility function - simple smoothstep fade +public float createRadialFadeModule(float2 uv, float radius) +{ + float dist = calculateDistanceFromCenterModule(uv); + return smoothstep(radius, 0.0, dist); +} From 1cee201760ffce8c5be14e0a6c7fb88dc003a341 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Nov 2025 21:02:51 +0000 Subject: [PATCH 3/4] Fix DebugGlobalVariable source for import syntax by using entry point debug location Co-authored-by: zangold-nv <242329104+zangold-nv@users.noreply.github.com> --- source/slang/slang-emit-spirv.cpp | 33 ++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/source/slang/slang-emit-spirv.cpp b/source/slang/slang-emit-spirv.cpp index da1ff80b92c..ea6134a5b79 100644 --- a/source/slang/slang-emit-spirv.cpp +++ b/source/slang/slang-emit-spirv.cpp @@ -1642,6 +1642,9 @@ struct SPIRVEmitContext : public SourceEmitterBase, public SPIRVEmitSharedContex } IRInst* m_defaultDebugSource = nullptr; + IRInst* m_entryPointDebugSource = nullptr; + + void setEntryPointDebugSource(IRInst* source) { m_entryPointDebugSource = source; } Dictionary m_extensionInsts; SpvInst* ensureExtensionDeclaration(UnownedStringSlice name) @@ -2358,9 +2361,15 @@ struct SPIRVEmitContext : public SourceEmitterBase, public SPIRVEmitSharedContex auto moduleInst = inst->getModule()->getModuleInst(); // Only create DebugCompilationUnit for non-included files auto isIncludedFile = as(debugSource->getIsIncludedFile())->getValue(); - // Only set default debug source to non-included files - if (!m_defaultDebugSource && !isIncludedFile) - m_defaultDebugSource = debugSource; + // Prefer the entry point's debug source as the default. + // Otherwise, only set default debug source to non-included files. + if (!m_defaultDebugSource) + { + if (m_entryPointDebugSource && debugSource == m_entryPointDebugSource) + m_defaultDebugSource = debugSource; + else if (!m_entryPointDebugSource && !isIncludedFile) + m_defaultDebugSource = debugSource; + } if (!m_mapIRInstToSpvDebugInst.containsKey(moduleInst) && !isIncludedFile) { IRBuilder builder(inst); @@ -9832,6 +9841,24 @@ SlangResult emitSPIRVFromIR( CompilerOptionName::PreserveParameters); auto generateWholeProgram = codeGenContext->getTargetProgram()->getOptionSet().getBoolOption( CompilerOptionName::GenerateWholeProgram); + + // Pre-identify the entry point's debug source to use as the default debug source. + // This ensures that global variables without explicit debug locations use the + // correct source file (the main shader file where the entry point is defined, + // not imported module files). + for (auto entryPoint : irEntryPoints) + { + if (auto debugLoc = entryPoint->findDecoration()) + { + auto entryPointDebugSource = as(debugLoc->getSource()); + if (entryPointDebugSource) + { + context.setEntryPointDebugSource(entryPointDebugSource); + break; + } + } + } + for (auto inst : irModule->getGlobalInsts()) { if (as(inst)) From 387cf80e4644a9f02e5f7b1fb7f2354e2dd38bc4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Nov 2025 21:06:39 +0000 Subject: [PATCH 4/4] Add null pointer checks for code review feedback Co-authored-by: zangold-nv <242329104+zangold-nv@users.noreply.github.com> --- source/slang/slang-emit-spirv.cpp | 3 ++- source/slang/slang-ir-link.cpp | 21 ++++++++++++++------- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/source/slang/slang-emit-spirv.cpp b/source/slang/slang-emit-spirv.cpp index ea6134a5b79..757d27e301d 100644 --- a/source/slang/slang-emit-spirv.cpp +++ b/source/slang/slang-emit-spirv.cpp @@ -2360,7 +2360,8 @@ struct SPIRVEmitContext : public SourceEmitterBase, public SPIRVEmitSharedContex auto moduleInst = inst->getModule()->getModuleInst(); // Only create DebugCompilationUnit for non-included files - auto isIncludedFile = as(debugSource->getIsIncludedFile())->getValue(); + auto isIncludedFileLit = as(debugSource->getIsIncludedFile()); + bool isIncludedFile = isIncludedFileLit ? isIncludedFileLit->getValue() : false; // Prefer the entry point's debug source as the default. // Otherwise, only set default debug source to non-included files. if (!m_defaultDebugSource) diff --git a/source/slang/slang-ir-link.cpp b/source/slang/slang-ir-link.cpp index e9f3925bb24..8e1b4bb6b5a 100644 --- a/source/slang/slang-ir-link.cpp +++ b/source/slang/slang-ir-link.cpp @@ -2194,13 +2194,20 @@ LinkedIR linkIR(CodeGenContext* codeGenContext) { auto fileName = as(debugSource->getFileName()); auto source = as(debugSource->getSource()); - auto newDebugSource = context->builder->emitDebugSource( - fileName->getStringSlice(), - source->getStringSlice(), - true); - // Register this as the clone of the original so that references - // to the original will use the new one with isIncludedFile=true - registerClonedValue(context, newDebugSource, inst); + if (fileName && source) + { + auto newDebugSource = context->builder->emitDebugSource( + fileName->getStringSlice(), + source->getStringSlice(), + true); + // Register this as the clone of the original so that references + // to the original will use the new one with isIncludedFile=true + registerClonedValue(context, newDebugSource, inst); + } + else + { + cloneValue(context, inst); + } } else {