Preserve OpenCL.DebugInfo.100 through elim-dead-code-aggressive#3542
Preserve OpenCL.DebugInfo.100 through elim-dead-code-aggressive#3542jaebaek merged 1 commit intoKhronosGroup:masterfrom
Conversation
|
Thanks @greg-lunarg. I will take a look! |
|
FWIW, I did spend some time thinking about legalization. There are two different HLSL constructs that are problematic: local structured buffers and local structs with opaque types (eg textures and/or samplers). You can find actual problematic HLSL shaders in this GDC talk I presented in: I couldn't get dxc to cooperate* in creating a completely perfect example of what ADCE will see for these, so I did a thought experiment instead. I came to the conclusion that if the passes that precede ADCE in the legalization recipe did what I expected them to do for these shaders, that ADCE would eliminate the problematic local construct from the SPIR-V. But it would still leave the DebugInfo for the original HLSL construct so that the user could query in terms of the original shader. *I could not force dxc to create the invalid SPIR-V for these shaders; it would just not generate a SPIR-V file and would give no error message. Ultimately we will need to double back and make sure these shaders are handled correctly when all the pieces are in place. |
|
It looks mostly good but I want to check it with your legalization example. I found a few bugs for the DXC's debug info generation while testing it. |
|
The desirable result for this legalization example will depend on what scalar-replacement does. |
|
I agree. I just tested the above code with the consideration that we do not have the scalar-replacement. It works fine even without the scalar-replacement. The case I am worried is that we have DebugDeclare with invalid OpVariable or OpFunctionParameter. I think it would be fine because ssa-rewrite will replace the OpVariable to its value correctly using DebugValue. In addition, after function inlining pass, we will not have OpFunctionParameter. It looks good to me. Thank you for working on it! |
…nosGroup#3542) Essentially, it marks all DebugInfo instructions in functions (and their operands) as live. It treats DebugDeclare and DebugValue with Deref as loads and so marks Stores of their variables as live. It marks each DebugGlobalVariables as live except for its variable. After closure, it rechecks if the variable is live. If not, the DebugGlobalVariable instruction's variable operand is set to DebugInfoNone, per the DebugInfo spec.
KhronosGroup/glslang@9eef54b...3ee5f2f $ git log 9eef54b..3ee5f2f --date=short --no-merges --format='%ad %ae %s' 2020-07-22 greg Update spirv-tools known-good to most recent stable 2020-07-19 vkushwaha Add changes for SPV_EXT_shader_atomic_float_add 2020-07-14 bclayton Limit visibility of symbols for internal libraries 2020-07-20 john SPV: Update to the latest SPIR-V headers. 2020-07-14 bclayton Deprecate InitializeDll functions 2020-07-14 bclayton Simplify PoolAlloc with use of thread_local. 2020-07-14 malcolm also search global variables assignment for live variables 2020-07-20 bclayton Drop support for VS2013 2020-07-20 bclayton Start glslang 11.0.0 2020-07-20 bclayton Finalize glslang 10.15.3847 2020-07-20 bclayton build_info: Fix parsing of versions with no flavor Created with: roll-dep third_party/glslang Roll third_party/spirv-headers/ 7f2ae11..979924c (1 commit) KhronosGroup/SPIRV-Headers@7f2ae11...979924c $ git log 7f2ae11..979924c --date=short --no-merges --format='%ad %ae %s' 2020-07-21 alanbaker Support SPV_KHR_terminate_invocation (KhronosGroup#163) Created with: roll-dep third_party/spirv-headers Roll third_party/spirv-tools/ c10d6ce..969f028 (18 commits) KhronosGroup/SPIRV-Tools@c10d6ce...969f028 $ git log c10d6ce..969f028 --date=short --no-merges --format='%ad %ae %s' 2020-07-23 rharrison Change DEPS rolling script to point at external/ (KhronosGroup#3584) 2020-07-23 vasniktel spirv-fuzz: Create a helper in fuzzerutil to reuse function type (KhronosGroup#3572) 2020-07-23 vasniktel spirv-fuzz: Test usages of IdIsIrrelevant fact (KhronosGroup#3578) 2020-07-23 antonikarp spirv-fuzz: adds TransformationReplaceCopyMemoryWithLoadStore (KhronosGroup#3575) 2020-07-23 antonikarp spirv-fuzz: adds TransformationReplaceCopyObjectWithStoreLoad (KhronosGroup#3567) 2020-07-22 stevenperron Start SPIRV-Tools v2020.5 2020-07-22 stevenperron Finalize SPIRV-Tools v2020.4 2020-07-22 vasniktel spirv-fuzz: Fix usages of irrelevant constants (KhronosGroup#3566) 2020-07-22 stevenperron Update CHANGES 2020-07-22 alanbaker Support SPV_KHR_terminate_invocation (KhronosGroup#3568) 2020-07-22 stevenperron Sink pointer instructions in merge return (KhronosGroup#3569) 2020-07-21 greg Preserve OpenCL.DebugInfo.100 through elim-dead-code-aggressive (KhronosGroup#3542) 2020-07-21 vasniktel spirv-fuzz: TransformationReplaceParamsWithStruct (KhronosGroup#3455) 2020-07-21 38144211+vkushwaha-nv Add changes for SPV_EXT_shader_atomic_float (KhronosGroup#3562) 2020-07-21 vasniktel spirv-fuzz: Use irrelevant constants (KhronosGroup#3565) 2020-07-21 stefanomil spirv-fuzz: Extend TransformationRecordSynonymousConstants to allow composite constants (KhronosGroup#3537) 2020-07-21 vasniktel spirv-fuzz: Add is_irrelevant parameter (KhronosGroup#3563) 2020-07-20 vasniktel spirv-fuzz: Add IdIsIrrelevant fact (KhronosGroup#3561) Created with: roll-dep third_party/spirv-tools
Essentially, it marks all DebugInfo instructions in functions (and their operands) as live. It treats DebugDeclare and DebugValue with Deref as loads and so marks Stores of their variables as live.
It marks each DebugGlobalVariables as live except for its variable. After closure, it rechecks if the variable is live. If not, the DebugGlobalVariable instruction's variable operand is set to DebugInfoNone, per the DebugInfo spec.