Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Push struct.new down to make it more likely that heap stores can be optimized. #6584

Merged
merged 7 commits into from
May 28, 2024

Conversation

rluble
Copy link
Contributor

@rluble rluble commented May 12, 2024

Heap stores (struct.set) are optimized into the struct.new when they are adjacent in a statement list.

Pushing struct.new down when they are not adjacent increases the likelihood for heap stores to be optimized. Optimizing heap stores is a beneficial step in making struct fields immutable which in turn helps other various optimziations.

@rluble
Copy link
Contributor Author

rluble commented May 13, 2024

@gkdn Reproed the case with the string constructor that had some inline code that prevented the value field to be made immutable and verified that this pr optimized that case.

src/passes/OptimizeInstructions.cpp Outdated Show resolved Hide resolved
src/passes/OptimizeInstructions.cpp Outdated Show resolved Hide resolved
src/passes/OptimizeInstructions.cpp Outdated Show resolved Hide resolved
src/passes/OptimizeInstructions.cpp Show resolved Hide resolved
src/passes/OptimizeInstructions.cpp Show resolved Hide resolved
src/passes/OptimizeInstructions.cpp Outdated Show resolved Hide resolved
src/passes/OptimizeInstructions.cpp Show resolved Hide resolved
src/passes/OptimizeInstructions.cpp Outdated Show resolved Hide resolved
src/passes/OptimizeInstructions.cpp Show resolved Hide resolved
Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

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

lgtm % last comments

Comment on lines 1942 to 1944
auto* tmp = list[i];
list[i] = list[j];
list[j] = tmp;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
auto* tmp = list[i];
list[i] = list[j];
list[j] = tmp;
std::swap(list[i], list[j]);

src/passes/OptimizeInstructions.cpp Show resolved Hide resolved
@@ -1,5 +1,5 @@
;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited.
;; RUN: wasm-opt %s --remove-unused-names --optimize-instructions -all -S -o - \
;; RUN: wasm-opt %s --remove-unused-names --optimize-instructions -all -S -o - \
Copy link
Member

Choose a reason for hiding this comment

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

unneeded space?

Suggested change
;; RUN: wasm-opt %s --remove-unused-names --optimize-instructions -all -S -o - \
;; RUN: wasm-opt %s --remove-unused-names --optimize-instructions -all -S -o - \

(nop)
;;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
;;
;; We do not swap with another local.set of struct.new.

@kripken kripken merged commit fc48efe into WebAssembly:main May 28, 2024
13 checks passed
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.

None yet

2 participants