Skip to content

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.

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]);

@@ -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 - \

)
;; Anything that we don't recognize breaks the pattern.
(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
@gkdn gkdn mentioned this pull request Aug 31, 2024
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