From cd50974490b82dd7e0f9ac0dd5ce9c17e390ff1f Mon Sep 17 00:00:00 2001 From: Yong He Date: Thu, 9 Oct 2025 23:27:57 -0700 Subject: Update debug var when in-param proxy var is being updated. (#8671) Closes #8664. The problem is that when there is an `in` parameter, Slang will create a local variable to proxy the parameter, copy the value of the parameter into the proxy variable, and replace all uses of the parameter in the function body to use the proxy variable instead. This way all writes to the parameter become writes to the proxy variable. However, when there is debug info enabled, we are also going to create a "debugVariable" corresponding to the parameter, but this debugVariable isn't updated when the proxy variable is updated. The fix is to map the proxy var instead of the original param to the debug var during the `insertDebugValueStore` pass, so that any changes to the proxy var will result in additional stores being inserted to the debug var. Allowing function body to modify an `in` parameter is a bad legacy behavior we inherited from HLSL that we should really be moving away from. I would like us to completely treat an `in` parameter as immutable by default in the next language version (Slang 2026), and make it an error if the user tries to do so. This will allow us to generate much cleaner code and in many cases would help with performance. --- source/slang/slang-emit-spirv.cpp | 13 +++++++++ source/slang/slang-ir-insert-debug-value-store.cpp | 13 ++++++++- source/slang/slang-ir-insts-stable-names.lua | 1 + source/slang/slang-ir-insts.lua | 3 ++ source/slang/slang-ir-strip.cpp | 1 + source/slang/slang-lower-to-ir.cpp | 4 +++ tests/spirv/debug-store.slang | 33 ++++++++++++++++++++++ 7 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 tests/spirv/debug-store.slang diff --git a/source/slang/slang-emit-spirv.cpp b/source/slang/slang-emit-spirv.cpp index 73a83f9c0..3a8a913ec 100644 --- a/source/slang/slang-emit-spirv.cpp +++ b/source/slang/slang-emit-spirv.cpp @@ -3114,6 +3114,18 @@ struct SPIRVEmitContext : public SourceEmitterBase, public SPIRVEmitSharedContex } } + void maybeEmitName(SpvInst* spvInst, IRInst* irInst, UnownedStringSlice prefix) + { + if (auto nameDecor = irInst->findDecoration()) + { + emitOpName( + getSection(SpvLogicalSectionID::DebugNames), + nullptr, + spvInst, + (StringBuilder() << prefix << nameDecor->getName()).getUnownedSlice()); + } + } + /// Emit a specialization constant. SpvInst* emitSpecializationConstant(IRGlobalParam* param, IRVarOffsetAttr* offset) { @@ -3961,6 +3973,7 @@ struct SPIRVEmitContext : public SourceEmitterBase, public SPIRVEmitSharedContex auto debugVarPtrType = builder.getPtrType(varType, AddressSpace::Function); auto actualHelperVar = emitOpVariable(parent, debugVar, debugVarPtrType, SpvStorageClassFunction); + maybeEmitName(actualHelperVar, debugVar, toSlice("_dbgvar_")); maybeEmitPointerDecoration(actualHelperVar, debugVar); return actualHelperVar; } diff --git a/source/slang/slang-ir-insert-debug-value-store.cpp b/source/slang/slang-ir-insert-debug-value-store.cpp index 91b7738f1..aeeb744ef 100644 --- a/source/slang/slang-ir-insert-debug-value-store.cpp +++ b/source/slang/slang-ir-insert-debug-value-store.cpp @@ -140,6 +140,17 @@ void DebugValueStoreContext::insertDebugValueStore(IRFunc* func) mapVarToDebugVar[param] = debugVar; + // Map any in-param proxy vars to the debug var. + bool hasProxyVar = false; + for (auto use = param->firstUse; use; use = use->nextUse) + { + if (auto inParamProxyVarDecor = as(use->getUser())) + { + mapVarToDebugVar[inParamProxyVarDecor->parent] = debugVar; + hasProxyVar = true; + } + } + // Store the initial value of the parameter into the debug var. IRInst* paramVal = nullptr; if (!isRefParam) @@ -153,7 +164,7 @@ void DebugValueStoreContext::insertDebugValueStore(IRFunc* func) paramVal = builder.emitLoad(param); } - if (paramVal) + if (paramVal && !hasProxyVar) { builder.emitDebugValue(debugVar, paramVal); } diff --git a/source/slang/slang-ir-insts-stable-names.lua b/source/slang/slang-ir-insts-stable-names.lua index 7032f2016..fefc7a956 100644 --- a/source/slang/slang-ir-insts-stable-names.lua +++ b/source/slang/slang-ir-insts-stable-names.lua @@ -678,4 +678,5 @@ return { ["Decoration.TempCallArgImmutableVar"] = 674, ["CastResourceToDescriptorHandle"] = 675, ["SymbolAlias"] = 676, + ["Decoration.InParamProxyVar"] = 677, } diff --git a/source/slang/slang-ir-insts.lua b/source/slang/slang-ir-insts.lua index 4093db8fa..a4bb4a6f2 100644 --- a/source/slang/slang-ir-insts.lua +++ b/source/slang/slang-ir-insts.lua @@ -1479,6 +1479,9 @@ local insts = { struct_name = "RequireFullQuadsDecoration", }, }, + -- Marks a var as a temporary local variable to replace references to a `in` parameter from the function body + -- This is to support legacy code that modifies an `in` parameter as if it is copied to a local variable. + { InParamProxyVar = { struct_name = "InParamProxyVarDecoration", min_operands = 1 } }, { TempCallArgImmutableVar = { struct_name = "TempCallArgImmutableVarDecoration" } }, { TempCallArgVar = { struct_name = "TempCallArgVarDecoration" } }, { diff --git a/source/slang/slang-ir-strip.cpp b/source/slang/slang-ir-strip.cpp index 6c64bfca2..85fd2da2b 100644 --- a/source/slang/slang-ir-strip.cpp +++ b/source/slang/slang-ir-strip.cpp @@ -16,6 +16,7 @@ static bool _shouldStripInst(IRInst* inst, IRStripOptions const& options) return false; case kIROp_HighLevelDeclDecoration: + case kIROp_InParamProxyVarDecoration: return true; case kIROp_NameHintDecoration: diff --git a/source/slang/slang-lower-to-ir.cpp b/source/slang/slang-lower-to-ir.cpp index c4492cfbb..80aa6066a 100644 --- a/source/slang/slang-lower-to-ir.cpp +++ b/source/slang/slang-lower-to-ir.cpp @@ -11054,6 +11054,10 @@ struct DeclLoweringVisitor : DeclVisitor // of the function. // auto irLocal = subBuilder->emitVar(irParamType); + subBuilder->addDecoration( + irLocal, + kIROp_InParamProxyVarDecoration, + irParam); auto localVal = LoweredValInfo::ptr(irLocal); assign(subContext, localVal, paramVal); // diff --git a/tests/spirv/debug-store.slang b/tests/spirv/debug-store.slang new file mode 100644 index 000000000..dd1847f56 --- /dev/null +++ b/tests/spirv/debug-store.slang @@ -0,0 +1,33 @@ +//TEST:SIMPLE(filecheck=CHECK): -target spirv -g2 -O0 + +// Test that writes to stand-in proxy variables for an `in` parameter are +// properly updating the corresponding debug variable for the parameter when +// debug info is enabled. + +//CHECK-COUNT-4: OpStore %_dbgvar_func_value + +cbuffer CBuffer +{ + float x; +} cb; + +float Func(float func_value) +{ + func_value = func_value > 10.0f ? func_value + 1.0f : func_value - 1.0f; + func_value = sqrt(func_value); + func_value = sin(func_value); + + return func_value; +} + +RWBuffer outbuf; + +[numthreads(1, 1, 1)] +void main() +{ + float value = cb.x; + + value = Func(cb.x); + + outbuf[0] = value; +} -- cgit v1.2.3