Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add DebugValue for invisible store in single_store_elim #4002

Merged
merged 3 commits into from
Nov 4, 2020

Conversation

jaebaek
Copy link
Contributor

@jaebaek jaebaek commented Oct 29, 2020

The front-end language compiler would simply emit DebugDeclare for
a variable when it is declared, which is effective through the variable's
scope. Since DebugDeclare only maps an OpVariable to a local variable,
the information can be removed when an optimization pass uses the
loaded value of the variable. DebugValue can be used to specify the
value of a variable. For each value update or phi instruction of a variable,
we can add DebugValue to help debugger inspect the variable at any
point of the program execution.
For example,

float a = 3;
... (complicated cfg) ...
foo(a); // <-- variable inspection: debugger can find DebugValue of float a in the nearest dominant

For the code with complicated CFG e.g., for-loop, if-statement, we
need help of ssa-rewrite to analyze the effective value of each variable
in each basic block.

If the value update of the variable happens only once and it dominates
all its uses, local-single-store-elim pass conducts the same value update
with ssa-rewrite and we have to let it add DebugValue for the value assignment.

One main issue is that we have to add DebugValue only when the value
update of a variable is visible to DebugDeclare. For example,

{  // scope1
   %stack = OpVariable %ptr_int %int_3
   {  // scope2
       DebugDeclare %foo %stack    <-- local variable "foo" in high-level language source code is declared as OpVariable "%stack"
       // add DebugValue "foo = 3"
       ...
       Store %stack %int_7   <-- foo = 7, add DebugValue "foo = 7"
       ...
       // debugger can inspect the value of "foo"
    }
    Store %stack %int_11   <-- out of "scope2" i.e., scope of "foo". DO NOT add DebugValue "foo = 11"
}

However, the initalization of a variable is an exception.
For example, an argument passing of an inlined function must be done out of
the function's scope, but we must add a DebugValue for it.

// in HLSL
bar(float arg) { ... }
...
float foo = 3;
bar(foo);

// in SPIR-V
%arg = OpVariable
OpStore %arg %foo   <-- Argument passing. Out of "float arg" scope, but we must add DebugValue for "float arg"
... body of function bar(float arg) ...

This PR handles the except case in local-single-store-elim pass. It adds
DebugValue for a store that is considered as an initialization.

The same exception handling code for ssa-rewrite is done by this commit: df4198e.

Copy link
Contributor

@kuhar kuhar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: you have a typo in the PR title / the first line of commit message, s/invisiable/invisible

source/opt/local_single_store_elim_pass.cpp Show resolved Hide resolved
source/opt/local_single_store_elim_pass.cpp Show resolved Hide resolved
source/opt/debug_info_manager.cpp Show resolved Hide resolved
Copy link
Contributor

@kuhar kuhar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but please wait for a second review before submitting.

@jaebaek
Copy link
Contributor Author

jaebaek commented Oct 30, 2020

Sure. I think @dnovillo can help.

@jaebaek jaebaek changed the title Add DebugValue for invisiable store in single_store_elim Add DebugValue for invisible store in single_store_elim Oct 30, 2020
@jaebaek jaebaek force-pushed the dbg_value_in_single_store_elim branch 2 times, most recently from e749268 to 79222be Compare November 2, 2020 14:57
Copy link
Contributor

@dnovillo dnovillo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add an example in the description? I'm having a hard time picturing what the problem is.

When a store instruction is not visible to DebugDeclare, we have to
conduct analysis to determine if the store instruction can be a value
assignment for the DebugValue. For example, if it is a function
argument passing for inlined function, it is possible to be a valid
value assignment, which we have to keep the information using
DebugValue.

For general cases, we need the ssa-rewrite analysis to determine if
the value assignment can be applied to the DebugDeclare. In the
local-single-store-elim pass, if a variable has a single store that
dominates all its uses, we can add a DebugValue with the value
assignment by the store because the value is valid for all the following
instructions.

This commit adds DebugValue for an invisible single store if it
is the only store for all its uses and dominates all of them.
@jaebaek jaebaek force-pushed the dbg_value_in_single_store_elim branch from 79222be to eb58fd2 Compare November 2, 2020 16:14
@jaebaek
Copy link
Contributor Author

jaebaek commented Nov 2, 2020

@dnovillo I updated the description. Because of its related background e.g., DebugValue distribution, the description may be hard to understand. I will explain it to you in offline.

Copy link
Contributor

@dnovillo dnovillo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the added description. It's now a bit more clear to me. One other question below.

test/opt/local_single_store_elim_test.cpp Outdated Show resolved Hide resolved
@jaebaek jaebaek merged commit c2b2b57 into KhronosGroup:master Nov 4, 2020
@jaebaek jaebaek deleted the dbg_value_in_single_store_elim branch November 4, 2020 18:44
dneto0 added a commit to dneto0/SPIRV-Tools that referenced this pull request Sep 14, 2024
KhronosGroup/glslang@142cb87...7f6559d

$ git log 142cb87..7f6559d --date=short --no-merges --format='%ad %ae %s'
2020-11-16 ShabbyX Compile out code for GL_EXT_shader_image_int64 for ANGLE (KhronosGroup#2463)
2020-11-12 mbechard tweak local_size comparison a bit (KhronosGroup#2456)
2020-11-12 dneto Avoid spuriously adding Geometry capability for vert, tesc, tese (KhronosGroup#2462)
2020-11-12 greg New nonuniform analysis (KhronosGroup#2457)
2020-11-09 jhall1024 Implement GL_EXT_terminate_invocation (KhronosGroup#2454)
2020-11-06 rdb Fix token-pasting macros not working in preprocessor directives. (KhronosGroup#2453)
2020-11-06 laddoc Fix warning in iomapper. (KhronosGroup#2449)
2020-11-04 TobyHector Add GL_EXT_shader_image_int64 support (KhronosGroup#2409)
2020-11-04 laddoc 8. io mapping refine & qualifier member check & resolver expand (KhronosGroup#2396)
2020-11-02 courtneygo Fix build error with Chromium & ANGLE (KhronosGroup#2446)
2020-11-02 dev Add new SpirvToolsDisassemble API interface + Improve Doc on existing API interface (KhronosGroup#2442)
2020-11-02 justsid Support for CapabilityShaderViewportIndex and CapabilityShaderLayer (KhronosGroup#2432)
2020-11-02 jaebaek Do not use PropagateLineInfoPass and RedundantLineInfoElimPass (KhronosGroup#2440)

Created with:
  roll-dep third_party/glslang

Roll third_party/googletest/ 0555b0eac..36d8eb532 (41 commits)

google/googletest@0555b0e...36d8eb5

$ git log 0555b0eac..36d8eb532 --date=short --no-merges --format='%ad %ae %s'
2020-11-13 vlee Initialize TestInfo member is_in_another_shard_ in constructor.
2020-11-12 absl-team Googletest export
2020-11-12 absl-team Googletest export
2020-11-12 absl-team Googletest export
2020-11-11 dmauro Googletest export
2020-11-11 dmauro Googletest export
2020-11-11 dmauro Googletest export
2020-11-11 absl-team Googletest export
2020-11-06 absl-team Googletest export
2020-11-06 absl-team Googletest export
2020-10-29 knut Only save original working directory if death tests are enabled
2020-11-08 hyuk.myeong fix typos
2020-11-06 absl-team Googletest export
2020-11-05 ofats Googletest export
2020-10-27 absl-team Googletest export
2020-10-27 elliott.brossard Add instructions for sanitizer integration
2020-10-26 absl-team Googletest export
2020-10-20 sonzogniarthur Fix typo "definedin in" => "defined in"
2020-10-15 absl-team Googletest export
2020-10-15 absl-team Googletest export
2020-10-15 dmauro Googletest export
2020-10-14 absl-team Googletest export
2020-10-14 dmauro Googletest export
2020-10-14 dmauro Googletest export
2020-10-14 absl-team Googletest export
2020-10-14 dmauro Googletest export
2020-10-14 absl-team Googletest export
2020-10-13 dmauro Googletest export
2020-10-13 dmauro Googletest export
2020-10-13 absl-team Googletest export
2020-10-13 absl-team Googletest export
2020-10-09 ofats Googletest export
2020-10-12 peternewman Fix a typo
2020-10-07 manavrion Improve FilePath::Normalize method
2020-10-07 pravin1992 Issue 2135: Change template args in NiceMock, NaggyMock and StrictMock from A1, A2, ... to TArg1, TArg2,... to avoid clash with legacy header files
2020-09-16 hyuk.myeong Remove spaces between Google Test and Google Mock
2020-09-15 hyuk.myeong Add follow-up patch for more natural reading
2020-09-15 hyuk.myeong Apply the reviewed comment
2020-09-15 hyuk.myeong Remove a space
2020-09-15 hyuk.myeong Improve the tutorial that may be confusing
2020-02-17 krystian.kuzniarek remove a duplicated include

Created with:
  roll-dep third_party/googletest

Roll third_party/re2/ 166dbbeb3..9e5430536 (2 commits)

google/re2@166dbbe...9e54305

$ git log 166dbbeb3..9e5430536 --date=short --no-merges --format='%ad %ae %s'
2020-11-17 junyer Make benchmarks use substrings of a random text buffer.
2020-11-15 twpayne Add note about Go Unicode character classes

Created with:
  roll-dep third_party/re2

Roll third_party/spirv-headers/ 7845730..5ab5c96 (2 commits)

KhronosGroup/SPIRV-Headers@7845730...5ab5c96

$ git log 7845730..5ab5c96 --date=short --no-merges --format='%ad %ae %s'
2020-11-04 michael.kinsner Reserve additional loop control bit for Intel extension (NoFusionINTEL) (KhronosGroup#175)
2020-11-02 4464295+XAMPPRocky Add EmbarkStudios/rust-gpu to vendor list. (KhronosGroup#174)

Created with:
  roll-dep third_party/spirv-headers

Roll third_party/spirv-tools/ f7da527..671914c (15 commits)

KhronosGroup/SPIRV-Tools@f7da527...671914c

$ git log f7da527..671914c --date=short --no-merges --format='%ad %ae %s'
2020-11-18 greg Fix buffer oob instrumentation for matrix refs (KhronosGroup#4025)
2020-11-13 afdx spirv-opt: Set parent when adding basic block (KhronosGroup#4021)
2020-11-13 jaebaek spirv-opt: properly preserve DebugValue indexes operand (KhronosGroup#4022)
2020-11-11 dneto Use less stack space when validating Vulkan builtins (KhronosGroup#4019)
2020-11-05 46493288+sfricke-samsung spirv-val: Fix SPV_KHR_fragment_shading_rate VUID label (KhronosGroup#4014)
2020-11-05 46493288+sfricke-samsung spirv-val: Label Layer and ViewportIndex VUIDs (KhronosGroup#4013)
2020-11-05 alanbaker Add dead function elimination to -O (KhronosGroup#4015)
2020-11-04 jaebaek Add DebugValue for invisible store in single_store_elim (KhronosGroup#4002)
2020-11-04 dnovillo Fix SSA re-writing in the presence of variable pointers. (KhronosGroup#4010)
2020-11-04 afdx spirv-fuzz: Fixes to pass management (KhronosGroup#4011)
2020-11-03 afdx spirv-fuzz: Add support for reining in rogue fuzzer passes (KhronosGroup#3987)
2020-11-03 vasniktel spirv-fuzz: Fix assertion failure in FuzzerPassAddCompositeExtract (KhronosGroup#3995)
2020-11-03 vasniktel spirv-fuzz: Fix invalid equation facts (KhronosGroup#4009)
2020-11-03 vasniktel spirv-fuzz: Fix bugs in TransformationFlattenConditionalBranch (KhronosGroup#4006)
2020-11-03 andreperezmaselco.developer spirv-fuzz: Fix bug related to transformation applicability (KhronosGroup#3990)

Created with:
  roll-dep third_party/spirv-tools
dneto0 pushed a commit to dneto0/SPIRV-Tools that referenced this pull request Sep 14, 2024
Roll third_party/glslang/ 142cb87..c594de2 (18 commits)

KhronosGroup/glslang@142cb87...c594de2

$ git log 142cb87..c594de2 --date=short --no-merges --format='%ad %ae %s'
2020-12-07 greg Update spirv-tools known-good KhronosGroup#2 - Pick up ray tracing terminator fix (KhronosGroup#2478)
2020-12-03 greg Update spirv-tools known-good (KhronosGroup#2473)
2020-11-30 dgkoch update spirv-headers and fix handling of gl_HitTEXT (KhronosGroup#2471)
2020-11-24 dgkoch Add ray query capability if acceleration structure or ray query types declared (KhronosGroup#2469)
2020-11-23 dgkoch Updates for final Vulkan ray tracing extensions (KhronosGroup#2466)
2020-11-16 ShabbyX Compile out code for GL_EXT_shader_image_int64 for ANGLE (KhronosGroup#2463)
2020-11-12 mbechard tweak local_size comparison a bit (KhronosGroup#2456)
2020-11-12 dneto Avoid spuriously adding Geometry capability for vert, tesc, tese (KhronosGroup#2462)
2020-11-12 greg New nonuniform analysis (KhronosGroup#2457)
2020-11-09 jhall1024 Implement GL_EXT_terminate_invocation (KhronosGroup#2454)
2020-11-06 rdb Fix token-pasting macros not working in preprocessor directives. (KhronosGroup#2453)
2020-11-06 laddoc Fix warning in iomapper. (KhronosGroup#2449)
2020-11-04 TobyHector Add GL_EXT_shader_image_int64 support (KhronosGroup#2409)
2020-11-04 laddoc 8. io mapping refine & qualifier member check & resolver expand (KhronosGroup#2396)
2020-11-02 courtneygo Fix build error with Chromium & ANGLE (KhronosGroup#2446)
2020-11-02 dev Add new SpirvToolsDisassemble API interface + Improve Doc on existing API interface (KhronosGroup#2442)
2020-11-02 justsid Support for CapabilityShaderViewportIndex and CapabilityShaderLayer (KhronosGroup#2432)
2020-11-02 jaebaek Do not use PropagateLineInfoPass and RedundantLineInfoElimPass (KhronosGroup#2440)

Created with:
  roll-dep third_party/glslang

Roll third_party/googletest/ 282877317..e5644f5f1 (35 commits)

google/googletest@2828773...e5644f5

$ git log 282877317..e5644f5f1 --date=short --no-merges --format='%ad %ae %s'
2020-12-08 absl-team Googletest export
2020-12-07 absl-team Googletest export
2020-12-07 absl-team Googletest export
2020-12-07 absl-team Googletest export
2020-12-05 paul.malcolm Fix typo in CLI help message
2020-12-03 absl-team Googletest export
2020-12-02 absl-team Googletest export
2020-12-02 absl-team Googletest export
2020-12-01 absl-team Googletest export
2020-11-30 dmauro Googletest export
2020-11-24 absl-team Googletest export
2020-11-23 absl-team Googletest export
2020-11-19 absl-team Googletest export
2020-11-13 vlee Initialize TestInfo member is_in_another_shard_ in constructor.
2020-11-12 absl-team Googletest export
2020-11-12 absl-team Googletest export
2020-11-12 absl-team Googletest export
2020-11-11 dmauro Googletest export
2020-11-11 dmauro Googletest export
2020-11-11 dmauro Googletest export
2020-11-11 absl-team Googletest export
2020-11-11 marius.brehler Refactor finding python
2020-11-06 absl-team Googletest export
2020-11-06 absl-team Googletest export
2020-10-29 knut Only save original working directory if death tests are enabled
2020-11-08 hyuk.myeong fix typos
2020-11-06 absl-team Googletest export
2020-11-05 ofats Googletest export
2020-10-27 elliott.brossard Add instructions for sanitizer integration
2020-09-16 hyuk.myeong Remove spaces between Google Test and Google Mock
2020-09-15 hyuk.myeong Add follow-up patch for more natural reading
2020-09-15 hyuk.myeong Apply the reviewed comment
2020-09-15 hyuk.myeong Remove a space
2020-09-15 hyuk.myeong Improve the tutorial that may be confusing
2020-02-17 krystian.kuzniarek remove a duplicated include

Created with:
  roll-dep third_party/googletest

Roll third_party/re2/ 166dbbeb3..91420e899 (3 commits)

google/re2@166dbbe...91420e8

$ git log 166dbbeb3..91420e899 --date=short --no-merges --format='%ad %ae %s'
2020-11-25 junyer Remove a double space from mksyntaxgo for Go folks.
2020-11-17 junyer Make benchmarks use substrings of a random text buffer.
2020-11-15 twpayne Add note about Go Unicode character classes

Created with:
  roll-dep third_party/re2

Roll third_party/spirv-headers/ 7845730..f027d53 (7 commits)

KhronosGroup/SPIRV-Headers@7845730...f027d53

$ git log 7845730..f027d53 --date=short --no-merges --format='%ad %ae %s'
2020-11-26 dkoch remove HitTKHR
2020-11-12 dneto MeshShadingNV enables builtins PrimitiveId, Layer, and ViewportIndex
2020-10-16 dkoch de-alias/reassign OpIgnoreIntersectionKHR/OpTerminateRayKHR
2020-06-29 alele Raytracing and Rayquery updates for final
2020-06-15 alele Updated headers for new trace/executeCallable and acceleration structure cast.
2020-11-04 michael.kinsner Reserve additional loop control bit for Intel extension (NoFusionINTEL) (KhronosGroup#175)
2020-11-02 4464295+XAMPPRocky Add EmbarkStudios/rust-gpu to vendor list. (KhronosGroup#174)

Created with:
  roll-dep third_party/spirv-headers

Roll third_party/spirv-tools/ f7da527..3b85234 (39 commits)

KhronosGroup/SPIRV-Tools@f7da527...3b85234

$ git log f7da527..3b85234 --date=short --no-merges --format='%ad %ae %s'
2020-12-08 46493288+sfricke-samsung spirv-val: Add last TessLevelOuter and TessLevelInner VUID (KhronosGroup#4055)
2020-12-08 46493288+sfricke-samsung spirv-val: Add last ClipDistance and CullDistance VUID (KhronosGroup#4054)
2020-12-08 46493288+sfricke-samsung spirv-val: Add last ViewportIndex and Layer VUID (KhronosGroup#4053)
2020-12-08 46493288+sfricke-samsung spirv-val: Add last Position VUID (KhronosGroup#4052)
2020-12-08 alanbaker Allow forward pointer to be used in types generally (KhronosGroup#4044)
2020-12-07 marijns95 opt: Run DCE when SPV_KHR_shader_clock is used (KhronosGroup#4049)
2020-12-07 dnovillo Update CHANGES to include latest ray tacing fixes.
2020-12-07 ehsannas Take new (raytracing) termination instructions into account. (KhronosGroup#4050)
2020-12-03 dnovillo Start SPIRV-Tools v2020.7
2020-12-03 dnovillo Finalize SPIRV-Tools v2020.6
2020-12-02 dnovillo Update CHANGES
2020-12-02 ehsannas Do run DCE if SPV_KHR_ray_query is used. (KhronosGroup#4047)
2020-12-02 dnovillo Update CHANGES
2020-12-01 greg Change ref_analysis to RefAnalysis to follow coding standards. (KhronosGroup#4045)
2020-12-01 stevenperron Handle 8-bit index in elim dead member (KhronosGroup#4043)
2020-12-01 dgkoch Add validation support for the ray tracing built-in variables (KhronosGroup#4041)
2020-12-01 greg Add texel buffer out-of-bounds checking instrumentation (KhronosGroup#4038)
2020-11-30 dgkoch Update spirv-header deps (KhronosGroup#4040)
2020-11-27 afdx Reject SPIR-V that applies void to OpUndef, OpCopyObject, OpPhi (KhronosGroup#4036)
2020-11-25 dneto BuildModule: optionally avoid adding new OpLine instructions (KhronosGroup#4033)
2020-11-25 dneto Remove prototype for unimplemented method (KhronosGroup#4031)
2020-11-25 afdx spirv-fuzz: Fix facts arising from CompositeConstruct (KhronosGroup#4034)
2020-11-24 afdx spirv-fuzz: Do not flatten conditionals that create synonyms (KhronosGroup#4030)
2020-11-23 dneto Update MeshShadingNV dependencies (and land Ray tracing updates) (KhronosGroup#4028)
2020-11-18 greg Fix buffer oob instrumentation for matrix refs (KhronosGroup#4025)
2020-11-13 afdx spirv-opt: Set parent when adding basic block (KhronosGroup#4021)
2020-11-13 jaebaek spirv-opt: properly preserve DebugValue indexes operand (KhronosGroup#4022)
2020-11-11 dneto Use less stack space when validating Vulkan builtins (KhronosGroup#4019)
2020-11-05 46493288+sfricke-samsung spirv-val: Fix SPV_KHR_fragment_shading_rate VUID label (KhronosGroup#4014)
2020-11-05 46493288+sfricke-samsung spirv-val: Label Layer and ViewportIndex VUIDs (KhronosGroup#4013)
2020-11-05 alanbaker Add dead function elimination to -O (KhronosGroup#4015)
2020-11-04 jaebaek Add DebugValue for invisible store in single_store_elim (KhronosGroup#4002)
2020-11-04 dnovillo Fix SSA re-writing in the presence of variable pointers. (KhronosGroup#4010)
2020-11-04 afdx spirv-fuzz: Fixes to pass management (KhronosGroup#4011)
2020-11-03 afdx spirv-fuzz: Add support for reining in rogue fuzzer passes (KhronosGroup#3987)
2020-11-03 vasniktel spirv-fuzz: Fix assertion failure in FuzzerPassAddCompositeExtract (KhronosGroup#3995)
2020-11-03 vasniktel spirv-fuzz: Fix invalid equation facts (KhronosGroup#4009)
2020-11-03 vasniktel spirv-fuzz: Fix bugs in TransformationFlattenConditionalBranch (KhronosGroup#4006)
2020-11-03 andreperezmaselco.developer spirv-fuzz: Fix bug related to transformation applicability (KhronosGroup#3990)

Created with:
  roll-dep third_party/spirv-tools
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants