Even more id overflow in sroa#2806
Conversation
Now we need to handle id overflow when we overflow while replacing uses of the variable. While looking at this code, I noticed an error in the way we handle access chains that cannot be replaced because of overflow. Name it will make some change, and then give up by returning SuccessWithoutChange. But it was changed. This is fixed up by returning Failure if we notice the error at the time of rewriting the users. This is for both id overflow or out-of-bounds accesses. Code is added to "CheckUses" to remove variables that have out-of-bounds accesses from the candidate list, so we don't even try to rewrite its uses. Fixes https://crbug.com/995032
| return true; | ||
| }); | ||
|
|
||
| if (replaced_all_uses) { |
There was a problem hiding this comment.
Won't this undo the semantics that got implemented in #2767. In that case, we wanted to silently not do the update but also not fail.
There was a problem hiding this comment.
That is why I mentioned the changes to CheckUses. For the out-of-bounds accesses, we will gracefully bail out before we call ReplaceVariable.
The design for sroa and a lot of other optimizations, is to first check if we can do the optimization. If we can, do the changes. We are now more inline with that design.
There was a problem hiding this comment.
You can see that the test that was added in #2767 still passes.
There was a problem hiding this comment.
Yes. Given your change and my memory of #2767, I was not trusting my test. Hence the question ;)
dnovillo
left a comment
There was a problem hiding this comment.
Thanks. I now understand what the new logic is doing.
Also adds enabling HLSL on Android builds Roll third_party/effcee/ 4bef5dbed..6527fb254 (4 commits) google/effcee@4bef5db...6527fb2 $ git log 4bef5dbed..6527fb254 --date=short --no-merges --format='%ad %ae %s' 2019-08-26 dneto Fail parsing checks if var def regexp is bad 2019-08-24 dneto Fail parsing checks if the regexp is bad. 2019-08-23 dneto Add effcee-fuzz 2019-08-19 dnovillo Add Bazel build rules. Roll third_party/glslang/ 95609e6..f27bd2a (30 commits) KhronosGroup/glslang@95609e6...f27bd2a $ git log 95609e6..f27bd2a --date=short --no-merges --format='%ad %ae %s' 2019-08-27 cwallez BUILD.gn: Add missing HLSL files. 2019-08-26 cwallez GN build (for Chromium): enable HLSL in dependents. 2019-08-26 baldurk Dereference any array type before expanding root-level SSBO members 2019-08-23 dneto GN build (for Chromium): enable HLSL 2019-08-22 cepheus Bump revision. 2019-08-22 cepheus GLSL: Inherit memory qualifiers, both declaratively and in execution. 2019-08-22 jonahr Fix conformance with -Wextra-tokens 2019-08-21 cepheus Bump version. 2019-08-21 cepheus web: Fix accidental additon of refract() prototypes and update README. 2019-08-13 cepheus Web: Turn off includes, independent preprocessing path, fine tune all. 2019-08-11 cepheus Web: Make switched methods all be non-virtual, more web-dependent code, 2019-08-10 cepheus Web: Optional error management and error tightening. 2019-08-09 cepheus Web: Use isEsProfile() instead of run-time testing; remove more atomics 2019-08-08 cepheus Web: Remove unused stage functionality, SPIR-V logger, and hex_utils 2019-08-08 cepheus Web: Remove unnecessary GLSL numeric types, and some collateral. 2019-08-08 cepheus Web: Tighten up sampling code and interfaces. 2019-08-07 cepheus Web: Complete the removal of vendor-specific #ifdef's, including CMake. 2019-08-06 cepheus Web: Prune grammar and lexor down to needed subset. 2019-08-06 cepheus Web: Generalize _EXTENSIONS* in SPIR-V back-end. 2019-08-06 cepheus Web: Turn off bracket-style attributes, reflection, and IO mapping. 2019-08-01 cepheus Web: Remove/rationalize a set of *_EXTENSIONS, using GLSLANG_WEB. 2019-07-31 cepheus Web: First pass of tabling the built-in function declarations. 2019-07-28 cepheus Web: Selectively remove a few key features, using #ifndef GLSLANG_WEB 2019-07-27 cepheus Web: Change a bunch of HLSL methods from dynamic to compile-time known. 2019-07-27 cepheus Web: Remove a few additional HLSL constructs with ENABLE_HLSL. 2019-07-26 cepheus Web: Add sanity check test suite for smaller-footprint builds. 2019-08-20 cepheus Bump revision. 2019-08-14 kainino convert_glsl_to_spirv: fail early, reduce copies, remove input buffer allocation 2019-07-25 kainino make glslang.js easy to use 2019-08-14 kainino enable build for node Roll third_party/googletest/ c9ccac7cb..6a3d632f4 (8 commits) google/googletest@c9ccac7...6a3d632 $ git log c9ccac7cb..6a3d632f4 --date=short --no-merges --format='%ad %ae %s' 2019-08-26 misterg Googletest export 2019-08-23 absl-team Googletest export 2019-08-23 krystian.kuzniarek Googletest export 2019-08-20 absl-team Googletest export 2019-08-14 krystian.kuzniarek reuse IndexSequence from googletest 2019-08-13 krystian.kuzniarek remove a custom implementation of std::remove_const 2019-08-13 krystian.kuzniarek remove a custom implementation of std::enable_if 2019-08-13 krystian.kuzniarek remove a custom implementation of std::add_lvalue_reference Roll third_party/re2/ be0e1305d..5bd613749 (4 commits) google/re2@be0e130...5bd6137 $ git log be0e1305d..5bd613749 --date=short --no-merges --format='%ad %ae %s' 2019-08-26 junyer Partial revert of commit 7a10064. 2019-08-26 junyer Adjust a thread annotation. 2019-08-13 milkovic.marek Improvements in install target generated by CMake 2019-08-26 junyer Simplify the plumbing for re2.pc. Roll third_party/spirv-cross/ 4ce04480e..563e99448 (7 commits) KhronosGroup/SPIRV-Cross@4ce0448...563e994 $ git log 4ce04480e..563e99448 --date=short --no-merges --format='%ad %ae %s' 2019-08-27 post MSL: Deal with array copies from and to threadgroup. 2019-08-27 post Do not allow base expressions for non-native row-major matrices. 2019-08-27 post Deal with ldexp taking uint input. 2019-08-26 post Move branchless analysis to CFG. 2019-08-26 post Elide branches to continue block when continue block is also a merge. 2019-08-26 post Deal correctly with sign on bitfield operations. 2019-08-26 post Fix variable scope when switch block exits multiple times. Roll third_party/spirv-headers/ e4322e3..059a495 (1 commit) KhronosGroup/SPIRV-Headers@e4322e3...059a495 $ git log e4322e3..059a495 --date=short --no-merges --format='%ad %ae %s' 2019-06-12 cepheus Grammar: Add instruction-printing classes. Roll third_party/spirv-tools/ bc62722..15fc19d (7 commits) KhronosGroup/SPIRV-Tools@bc62722...15fc19d $ git log bc62722..15fc19d --date=short --no-merges --format='%ad %ae %s' 2019-08-26 stevenperron Refactor instruction folders (KhronosGroup#2815) 2019-08-23 8729214+jonahryandavis Add missing files to BUILD.gn (KhronosGroup#2809) 2019-08-22 afdx Extend reducer to remove relaxed precision decorations (KhronosGroup#2797) 2019-08-22 stevenperron Handle Id overflow in private-to-local (KhronosGroup#2807) 2019-08-21 stevenperron Even more id overflow in sroa (KhronosGroup#2806) 2019-08-21 stevenperron Add name for variables in desc sroa (KhronosGroup#2805) 2019-08-20 dneto Remove unimplemented method declaration (KhronosGroup#2804) Created with: roll-dep third_party/effcee third_party/glslang third_party/googletest third_party/re2 third_party/spirv-cross third_party/spirv-headers third_party/spirv-tools
Now we need to handle id overflow when we overflow while replacing uses of the variable. While looking at this code, I noticed an error in the way we handle access chains that cannot be replaced because of overflow. Name it will make some change, and then give up by returning SuccessWithoutChange. But it was changed.
This is fixed up by returning Failure if we notice the error at the time of rewriting the users. This is for both id overflow or out-of-bounds accesses.
Code is added to "CheckUses" to remove variables that have out-of-bounds accesses from the candidate list, so we don't even try to rewrite its uses.
Fixes https://crbug.com/995032