Skip to content

Even more id overflow in sroa#2806

Merged
s-perron merged 1 commit intoKhronosGroup:masterfrom
s-perron:cr995032
Aug 21, 2019
Merged

Even more id overflow in sroa#2806
s-perron merged 1 commit intoKhronosGroup:masterfrom
s-perron:cr995032

Conversation

@s-perron
Copy link
Copy Markdown
Collaborator

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

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
@s-perron s-perron requested a review from dnovillo August 21, 2019 14:29
@s-perron s-perron self-assigned this Aug 21, 2019
return true;
});

if (replaced_all_uses) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Collaborator Author

@s-perron s-perron Aug 21, 2019

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

You can see that the test that was added in #2767 still passes.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes. Given your change and my memory of #2767, I was not trusting my test. Hence the question ;)

Copy link
Copy Markdown
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. I now understand what the new logic is doing.

@s-perron s-perron merged commit aef8f92 into KhronosGroup:master Aug 21, 2019
@s-perron s-perron deleted the cr995032 branch September 10, 2021 20:10
dneto0 pushed a commit to dneto0/SPIRV-Tools that referenced this pull request Sep 14, 2024
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants