spirv-opt: HLSL legalization moves function parameter DebugValue to wrong line/scope after inlining

[sajjadmirzanv]

spirv-opt converts the DebugDeclare instruction of a function parameter into a DebugValue with the actual value of the argument at the call site. However when it does this, it does not retain the DebugScope and DebugLine instructions for that DebugValue.

In the original HLSL I have a function "my_func" which is called from "main":

float4 my_func(Texture2DArray tex, float2 uv) { // line 8
	return tex.Load(float4(uv, 0, 0));
}

float4 main(PSInput psin) : SV_Target
{
	float2 my_uv = psin.position.xy;
	float z = my_func(depthTex, my_uv).x; // line 15
	float4 oColor = psin.color * z;
	return oColor;
}

Pre-legalization SPIR-V:

    %my_func = OpFunction %v4float None %160
        %tex = OpFunctionParameter %_ptr_Function_type_2d_image_array
         %uv = OpFunctionParameter %_ptr_Function_v2float
 %bb_entry_0 = OpLabel
        %164 = OpExtInst %void %1 DebugScope %44 ;; points to DebugFunction for "my_func"
        %165 = OpExtInst %void %1 DebugLine %28 %uint_8 %uint_8 %uint_16 %uint_31 // line 8
        %166 = OpExtInst %void %1 DebugDeclare %52 %tex %78

After legalization inlines everything into one function:

        %333 = OpExtInst %void %1 DebugScope %68 ;; points to DebugLexicalBlock which points to DebugFunction for "main"
;; ... omitted for legibility ...
        %258 = OpExtInst %void %1 DebugLine %28 %uint_15 %uint_15 %uint_20 %uint_20 // line 15
        %241 = OpLoad %type_2d_image_array %depthTex
        %309 = OpExtInst %void %1 DebugValue %52 %241 %78

Now the value is at line 15 instead of line 8, and the scope has changed to the "main" function.

The correct behavior would be for the DebugValue to keep the same position as the DebugDeclare even after inlining, and use a DebugScope with a DebugInlinedAt operand to represent that inlining happened.

Attaching the original HLSL shader for reference, but the main files to compare are dxc_release_2025_02_20.notlegalized.spv which has pre-legalization SPIR-V vs. dxc_release_2025_02_20.spv and spvopt.legalized.spv. The latter I generated with spirv-opt --legalize-hlsl to confirm that the problem is specifically in spirv-opt.

debug_legalization.zip

[s-perron]

The problem is not inlining. Inlining correctly add the inlined at on the scope. The problem is that local single store elimination changes the DebugDeclare to a DebugValue because it is removing the variable. When doing that it uses the scope of the store. I believe this is wrong, but it is not obvious where it should get the scope from. The variable itself does not have a scope. We probably need to get it from the debug declare, but it is not immediately available. It is also unclear what to do if we have multiple debug declares. Off the top of my head I can't recall what the spec says about this.

The problem is in this function:

SPIRV-Tools/source/opt/local_single_store_elim_pass.cpp

Lines 187 to 194 in feba06f

	bool LocalSingleStoreElimPass::RewriteDebugDeclares(Instruction* store_inst,
	uint32_t var_id) {
	uint32_t value_id = store_inst->GetSingleWordInOperand(1);
	bool modified = context()->get_debug_info_mgr()->AddDebugValueForVariable(
	store_inst, var_id, value_id, store_inst);
	modified |= context()->get_debug_info_mgr()->KillDebugDeclares(var_id);
	return modified;
	}

My first though is to modify this function to replace every debug declare with a separate debug value with the same scope. This happens at the point of the store.

[s-perron]

@sajjadmirzanv You probably have been insight than I do. Let me know what you think will work. Also, there are lots of DXC issues, with few maintainers. I don't know when I will get back to this.

[sajjadmirzanv]

I agree, the pass should put the DebugValue where the DebugDeclare was, rather than where the OpStore was. Then it would automatically have the correct scope and line number.

It is also unclear what to do if we have multiple debug declares. Off the top of my head I can't recall what the spec says about this.

Unfortunately, the spec doesn't say anything. But I think this change should be safe, since it would just convert them to DebugValue.

[s-perron]

@SteveUrquhart will be looking into this issue.