Skip to content

Let ADCE pass check DebugScope#3703

Merged
s-perron merged 2 commits intoKhronosGroup:masterfrom
jaebaek:adce_check_dbg_scope
Aug 18, 2020
Merged

Let ADCE pass check DebugScope#3703
s-perron merged 2 commits intoKhronosGroup:masterfrom
jaebaek:adce_check_dbg_scope

Conversation

@jaebaek
Copy link
Copy Markdown
Contributor

@jaebaek jaebaek commented Aug 14, 2020

In the existing code, ADCE pass does not check DebugScope of an
instruction when it checks the users of each instruction, which results
in removing OpenCL.Debug.100 instructions that are only used by
DebugScope. This commit lets ADCE pass add DebugScope of an instruction
to the live instruction set when the instruction is added to the live
instruction set.

@jaebaek jaebaek requested a review from s-perron August 14, 2020 19:10
@jaebaek jaebaek self-assigned this Aug 14, 2020
if (inst->GetDebugInlinedAt() != kNoInlinedAt) {
auto* inlined_at = get_def_use_mgr()->GetDef(inst->GetDebugInlinedAt());
AddToWorklist(inlined_at);
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is not the right place for this. This should be in the loop where we process and instruction. Probably somewhere around here. The design is that the work list is the list of instructions that need to be processed. We do not process it when adding it to the work list, but when we remove it from the work list.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

@jaebaek
Copy link
Copy Markdown
Contributor Author

jaebaek commented Aug 17, 2020

Thank you for the code review! PTAL

In the existing code, ADCE pass does not check DebugScope of an
instruction when it checks the users of each instruction, which results
in removing OpenCL.Debug.100 instructions that are only used by
DebugScope. This commit lets ADCE pass add DebugScope of an instruction
to the live instruction set when the instruction is added to the live
instruction set.
@jaebaek jaebaek force-pushed the adce_check_dbg_scope branch from a9b031b to 52e908d Compare August 17, 2020 15:05
@s-perron s-perron merged commit 3434cb0 into KhronosGroup:master Aug 18, 2020
@jaebaek jaebaek deleted the adce_check_dbg_scope branch August 18, 2020 15:33
dnovillo pushed a commit to dnovillo/SPIRV-Tools that referenced this pull request Aug 19, 2020
In the existing code, ADCE pass does not check DebugScope of an
instruction when it checks the users of each instruction, which results
in removing OpenCL.Debug.100 instructions that are only used by
DebugScope. This commit lets ADCE pass add DebugScope of an instruction
to the live instruction set when the instruction is added to the live
instruction set.
dneto0 pushed a commit to dneto0/SPIRV-Tools that referenced this pull request Sep 14, 2024
Roll third_party/glslang/ f257e0e..983698b (2 commits)

KhronosGroup/glslang@f257e0e...983698b

$ git log f257e0e..983698b --date=short --no-merges --format='%ad %ae %s'
2020-08-23 john Revert "Merge pull request KhronosGroup#2371 from RafaelMarinheiro/master"
2020-08-21 julius.ikkala Obey ENABLE_PCH CMake option

Created with:
  roll-dep third_party/glslang

Roll third_party/googletest/ adeef1929..1e315c5b1 (14 commits)

google/googletest@adeef19...1e315c5

$ git log adeef1929..1e315c5b1 --date=short --no-merges --format='%ad %ae %s'
2020-08-20 absl-team Googletest export
2020-08-17 absl-team Googletest export
2020-08-12 robert.earhart Export LICENSE
2020-08-03 amatanhead Remove ThrowsMessageHasSubstr and fix some nits after review
2020-07-13 amatanhead Cleanup a bulky expression, document implementation details
2020-07-07 amatanhead Fix build under msvc
2020-07-07 amatanhead Update tests after changing an error message
2020-07-07 amatanhead Fix build under msvc
2020-07-07 amatanhead Add a test to ensure that the `Throws` matcher only invokes its argument once.
2020-07-07 amatanhead Add a test for duplicate catch clauses in throw matchers, fix a couple of nitpicks.
2020-07-03 amatanhead Add missing documentation piece
2020-06-20 amatanhead Small improvements: code style and property name
2020-06-20 amatanhead Add matchers for testing exception properties
2018-05-01 lantw44 Avoid using environ on FreeBSD

Created with:
  roll-dep third_party/googletest

Roll third_party/spirv-cross/ 4c7944bb4..685f86471 (6 commits)

KhronosGroup/SPIRV-Cross@4c7944b...685f864

$ git log 4c7944bb4..685f86471 --date=short --no-merges --format='%ad %ae %s'
2020-08-24 post Run format_all.sh.
2020-08-24 post Work around annoying warning on GCC 10.2.
2020-08-21 post Overhaul how we deal with reserved identifiers.
2020-08-20 post HLSL: Fix FragCoord.w.
2020-08-20 post HLSL: Deal with partially filled 16-byte word in cbuffers.
2020-08-20 post HLSL: Fix bug in is_packing_standard for cbuffer.

Created with:
  roll-dep third_party/spirv-cross

Roll third_party/spirv-tools/ b8de4f5..4dd1223 (9 commits)

KhronosGroup/SPIRV-Tools@b8de4f5...4dd1223

$ git log b8de4f5..4dd1223 --date=short --no-merges --format='%ad %ae %s'
2020-08-21 andreperezmaselco.developer spirv-fuzz: Add words instead of logical operands (KhronosGroup#3728)
2020-08-20 dnovillo CCP should mark IR changed if it created new constants. (KhronosGroup#3732)
2020-08-19 antonikarp spirv-fuzz: add FuzzerPassAddCompositeInserts (KhronosGroup#3606)
2020-08-19 antonikarp spirv-fuzz: Support pointer types in FuzzerPassAddParameters (KhronosGroup#3627)
2020-08-18 jaebaek Let ADCE pass check DebugScope (KhronosGroup#3703)
2020-08-18 andreperezmaselco.developer spirv-opt: Implement opt::Function::HasEarlyReturn function (KhronosGroup#3711)
2020-08-17 andreperezmaselco.developer spirv-fuzz: Check termination instructions when donating modules (KhronosGroup#3710)
2020-08-17 jackoalan Fix -Wrange-loop-analysis warning (KhronosGroup#3712)
2020-08-17 andreperezmaselco.developer spirv-fuzz: Check header dominance when adding dead block (KhronosGroup#3694)

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.

2 participants