Wasmtime GitHub notifications bot (Feb 07 2024 at 00:58): Wasmtime GitHub notifications bot (Feb 07 2024 at 00:58):elliottt opened PR #7882 from elliottt:trevor/fix-7874 to bytecodealliance:main:
Add a test to expose issues with unbounded recursion through
iaddduring egraph rewrites, and bound the recursion ofwill_simplify_with_ireduce.Fixes #7874
<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
Wasmtime GitHub notifications bot (Feb 07 2024 at 00:58):elliottt has marked PR #7882 as ready for review.
Wasmtime GitHub notifications bot (Feb 07 2024 at 00:58):elliottt requested wasmtime-compiler-reviewers for a review on PR #7882.
Wasmtime GitHub notifications bot (Feb 07 2024 at 00:58):elliottt requested fitzgen for a review on PR #7882.
Wasmtime GitHub notifications bot (Feb 07 2024 at 00:58):elliottt requested alexcrichton for a review on PR #7882.
Wasmtime GitHub notifications bot (Feb 07 2024 at 01:01):elliottt edited PR #7882:
Expose the underlying issue of #7874 by adding a test with a long chain of additions, and then fix the bug by bounding the recursion of
will_simplify_with_ireduce.Reading through the changes in #7719, it seemed like the recursive cases in
will_simplify_with_ireducewere missing two things:
- an additional guard of
reducible_modular_opbefore the recursion, which showed up in the entry-point insimplify- a depth bound to ensure that the recursion doesn't get out of control.
This PR makes both of these changes, and resolves the compilation time issues reported in #7874.
Fixes #7874
<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
Wasmtime GitHub notifications bot (Feb 07 2024 at 01:03):elliottt edited PR #7882:
Expose the underlying issue of #7874 by adding a test with a long chain of additions, and then fix the bug by bounding the recursion of
will_simplify_with_ireduce.Reading through the changes in #7719, it seemed like the recursive cases in
will_simplify_with_ireducewere missing two things:
- an additional guard of
reducible_modular_opbefore the recursion, which showed up in the entry-point insimplify- a depth bound to ensure that the recursion doesn't get out of control.
This PR makes both of these changes, and resolves the compilation time issues reported in #7874. The depth bound was arbitrary, and it would be fine to increase it if necessary.
Fixes #7874
<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
Wasmtime GitHub notifications bot (Feb 07 2024 at 02:44):github-actions[bot] commented on PR #7882:
Subscribe to Label Action
cc @cfallin, @fitzgen
<details>
This issue or pull request has been labeled: "cranelift", "isle"Thus the following users have been cc'd because of the following labels:
- cfallin: isle
- fitzgen: isle
To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.
Learn more.
</details>
Wasmtime GitHub notifications bot (Feb 07 2024 at 05:01):fitzgen submitted PR review:
Looks great!
Wasmtime GitHub notifications bot (Feb 07 2024 at 05:41):fitzgen merged PR #7882.
Last updated: Nov 22 2024 at 17:03 UTC