Fix ADCE pass bug for mulitple entries#3470
Merged
s-perron merged 1 commit intoKhronosGroup:masterfrom Jun 29, 2020
Merged
Conversation
When there are multiple entries and the shader has a variable with WorkGroup storage class, those multiple entry functions store values to the variable. Since ADCE pass uses def-use chains to propagate the work list, some of instructions in the work list are not actually a part of the currently processed function. As a result, it adds instructions in other functions and put them in |live_insts_|. However, it does not have the control flow information for those instructions in other functions i.e., |block2headerBranch_| and |header2nextHeaderBranch_|. When it processes those instructions (they are added when it processes a different function), it skips handling them because they are already in |live_insts_| and does not check |block2headerBranch_| and |header2nextHeaderBranch_|, which results in skipping some branches. Even though those branches are live branches, it considers they are dead branches.
Contributor
Author
|
This issue is initially reported in DXC github repo microsoft/DirectXShaderCompiler#2828 |
alan-baker
approved these changes
Jun 29, 2020
s-perron
requested changes
Jun 29, 2020
Collaborator
s-perron
left a comment
There was a problem hiding this comment.
I need more time to look at this. I did a quick look before the weekend, and I think this might be missing something if we have not inlined everything. I need to think about it more.
Contributor
Author
|
Yeah. Please doublecheck it. I am not 100% sure I understand all the relations between global variables. |
s-perron
approved these changes
Jun 29, 2020
Collaborator
s-perron
left a comment
There was a problem hiding this comment.
Looks good. The private_like_local_ flags handles the case I was worried about.
dneto0
pushed a commit
to dneto0/SPIRV-Tools
that referenced
this pull request
Sep 14, 2024
Roll third_party/glslang/ 8397044..f5ed7a6 (30 commits) KhronosGroup/glslang@8397044...f5ed7a6 $ git log 8397044..f5ed7a6 --date=short --no-merges --format='%ad %ae %s' 2020-07-03 marcin.slusarz Add --quiet option. 2020-07-05 ShabbyX gn: Fix dawn tests in Chromium 2020-07-05 ShabbyX gn: Fix `gn gen --check` by adding missing dependency 2020-07-03 bclayton Add GLSLANG_BUILD_PIC CMake flag 2020-07-03 ShabbyX gn: Optionally disable optimizations and HLSL 2020-07-03 bclayton Don't use add_link_options() on old CMake versions 2020-07-03 bclayton License headers: s/Google/The Khronos Group 2020-07-03 bclayton Kokoro: Correct the `build_file' path to build.sh 2020-07-02 bclayton Add config for license-checker and Kokoro scripts. 2020-07-02 bclayton Fix GLSLANG_IS_SHARED_LIBRARY define 2020-07-01 bclayton Add missing copyright headers 2020-07-02 cepheus Bump revision. 2020-07-01 cepheus SPIRV-Tools and tests: Update to location-validation in SPIRV-Tools. 2020-07-01 cepheus Tests: More broadly use automapping binding/location. 2020-07-01 bclayton Add additional licenses in use to LICENSE.txt 2020-07-01 cepheus HLSL: Catch error cases earlier, preventing a later assert. 2020-06-29 bclayton glslang: Only export public interface for SOs 2020-06-29 bclayton CMake: break up glslang into smaller static libs 2020-06-30 cepheus SPV: RelaxedPrecision: use the result precision for texture sampling. 2020-06-30 cepheus SPV: RelaxedPrecision: Generalize fix KhronosGroup#2293 to cover more operations. 2020-06-24 e.proydakov Fixed GCC -Wunused-parameter in hlslParseables.cpp. 2020-06-29 bclayton CMake: Compile with -fPIC when building SOs 2020-06-29 bclayton CMake: Error on unresolved symbols 2020-06-29 bclayton Remove root kokoro/linux-*-cmake configs 2020-06-26 cepheus SPV: Fix KhronosGroup#2293: keep relaxed precision on arg passed to relaxed param 2020-06-26 cepheus SPV: Partially address KhronosGroup#2293: correct "const in" precision matching. 2020-06-25 lriki.net Add pack_matrix test 2020-06-12 lriki.net HLSL: Fix #pragma pack_matrix(row_major) not work on global uniforms 2020-06-24 bclayton Kokoro: Split linux cmake cfgs into static/shared 2020-06-23 e.proydakov Fixed msvc 2019 nmake compiler warnings with RTTI. By default cmake generates cxx_flags with `/GR` parameter. I updated CMAKE_CXX_FLAGS string and replaced `/GR` -> `/GR-` Created with: roll-dep third_party/glslang Roll third_party/googletest/ c6e309b26..356f2d264 (10 commits) google/googletest@c6e309b...356f2d2 $ git log c6e309b26..356f2d264 --date=short --no-merges --format='%ad %ae %s' 2020-07-01 absl-team Googletest export 2020-06-26 absl-team Googletest export 2020-06-25 absl-team Googletest export 2020-06-24 absl-team Googletest export 2020-06-24 absl-team Googletest export 2020-06-19 mayur.shingote Updated googletest issue tracker url. 2020-06-10 rharrison Fix build issue for MinGW 2020-02-21 nini16041988-gitbucket Add missing call for gtest_list_output_unittest_ unitTest. Add unitTest for fixed TEST_P line number. Use CodeLocation TestInfo struct. 2020-02-18 nini16041988-gitbucket Fix: shadow member 2020-02-18 nini16041988-gitbucket Add correct line number to TEST_P test cases for gtest_output. Created with: roll-dep third_party/googletest Roll third_party/re2/ 14d319322..fe8a81adc (3 commits) google/re2@14d3193...fe8a81a $ git log 14d319322..fe8a81adc --date=short --no-merges --format='%ad %ae %s' 2020-07-05 junyer Bump SONAME, which missing ')' versus unexpected ')' needed. 2020-07-03 courbet Make the compiler inline the hot RE2::DFA loop. 2020-06-25 Shikugawa change bazel cpu symbol from wasm to wasm32 Created with: roll-dep third_party/re2 Roll third_party/spirv-cross/ f9ae06512..559b21c6c (14 commits) KhronosGroup/SPIRV-Cross@f9ae065...559b21c $ git log f9ae06512..559b21c6c --date=short --no-merges --format='%ad %ae %s' 2020-07-06 dsinclair Roll deps. 2020-07-01 post MSL: Do not emit swizzled writes in packing fixups. 2020-07-01 post MSL: Workaround broken vector -> scalar access chain in MSL. 2020-07-06 post MSL: Use input attachment index directly for resource index fallback. 2020-07-06 post GLSL: Support I/O flattening with arrays as final type. 2020-07-03 post GLSL: Support multi-level struct flattening for I/O. 2020-07-01 post Run format_all.sh. 2020-07-01 post test: Use --hlsl-dx9-compatible when attempting to compile SM 3.0 shaders. 2020-06-30 post GLSL: Fix nested legacy switch workarounds. 2020-06-29 post GLSL: Implement switch on ESSL 1.0. 2020-06-29 post GLSL: Use for-loop fallback instead of do/while for legacy ESSL. 2020-06-29 post Implement context-sensitive expression read tracking. 2020-06-29 post Fix bug with control dependent expression tracking. 2020-06-23 post HLSL: Workaround FXC bugs with degenerate switch blocks. Created with: roll-dep third_party/spirv-cross Roll third_party/spirv-headers/ 11d7637..308bd07 (1 commit) KhronosGroup/SPIRV-Headers@11d7637...308bd07 $ git log 11d7637..308bd07 --date=short --no-merges --format='%ad %ae %s' 2020-06-26 dj2 Register the Tint compiler Created with: roll-dep third_party/spirv-headers Roll third_party/spirv-tools/ d4b9f57..6a4da9d (16 commits) KhronosGroup/SPIRV-Tools@d4b9f57...6a4da9d $ git log d4b9f57..6a4da9d --date=short --no-merges --format='%ad %ae %s' 2020-07-06 jaebaek Debug info preservation in copy-prop-array pass (KhronosGroup#3444) 2020-07-03 vasniktel spirv-fuzz: TransformationInvertComparisonOperator (KhronosGroup#3475) 2020-07-02 vasniktel Fix regression (KhronosGroup#3481) 2020-07-02 vasniktel spirv-fuzz: Add fuzzerutil::FindOrCreate* (KhronosGroup#3479) 2020-06-30 vasniktel spirv-fuzz: Add FuzzerPassAddCopyMemoryInstructions (KhronosGroup#3391) 2020-06-30 vasniktel spirv-fuzz: Add one parameter at a time (KhronosGroup#3469) 2020-06-29 jaebaek Fix ADCE pass bug for mulitple entries (KhronosGroup#3470) 2020-06-26 ehsannas Add gl_BaseInstance to the name mapper. (KhronosGroup#3462) 2020-06-26 andreperezmaselco.developer Implement the OpMatrixTimesScalar linear algebra case (KhronosGroup#3450) 2020-06-25 jaebaek Clear debug information for kill and replacement (KhronosGroup#3459) 2020-06-25 alanbaker Validate location assignments (KhronosGroup#3308) 2020-06-23 ehsannas Support OpCompositeExtract pattern in desc_sroa (KhronosGroup#3456) 2020-06-23 vasniktel spirv-fuzz: Implement FuzzerPassAddParameters (KhronosGroup#3399) 2020-06-23 vasniktel spirv-fuzz: Add GetParameters (KhronosGroup#3454) 2020-06-23 vasniktel spirv-fuzz: Permute OpPhi instruction operands (KhronosGroup#3421) 2020-06-22 rharrison Add support for different default/trunks in roll-deps (KhronosGroup#3442) Created with: roll-dep third_party/spirv-tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When there are multiple entries and the shader has a variable with
WorkGroup storage class, those multiple entry functions store values to
the variable. Since ADCE pass uses def-use chains to propagate the work
list, some of instructions in the work list are not actually a part of
the currently processed function. As a result, it adds instructions in
other functions and put them in
live_insts_. However, it does nothave the control flow information for those instructions in other
functions i.e.,
block2headerBranch_andheader2nextHeaderBranch_.It skips handling those instructions because they are already in
live_insts_and does not check their control flow information e.g.,block2headerBranch_andheader2nextHeaderBranch_, which resultsin skipping some branches. Even though those branches are live
branches, it considers they are dead branches.
It happens because multiple functions can store values to a variable with
WorkGroup storage class and the code handling store instructions add
them into
live_insts_without considering they are in a function currentlyprocessed. This commit lets it add an instruction to
live_insts_onlywhen the function is currently processed one.