Skip to content

Conversation

@tlively
Copy link
Member

@tlively tlively commented Jul 31, 2025

HeapStoreOptimization moves subsequent sets back into initial
allocations, so it must ensure the set values can be reordered before
all of the original set values that it will precede. If the allocation
has a descriptor operand, then the set value must be reordered before
that as well, but we were not checking that operand for effects. Fix it.

HeapStoreOptimization moves subsequent sets back into initial
allocations, so it must ensure the set values can be reordered before
all of the original set values that it will precede. If the allocation
has a descriptor operand, then the set value must be reordered before
that as well, but we were not checking that operand for effects. Fix it.
@tlively tlively requested a review from kripken July 31, 2025 17:25
(func $no-reorder-nested
(local $struct (ref $struct))
;; As above, but now it is not the top-level allocation that traps, but
;; rather its descriptor operand. We still cannot optimized.
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
;; rather its descriptor operand. We still cannot optimized.
;; rather its descriptor operand. We still cannot optimize.

;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: )
;; CHECK-NEXT: (nop)
Copy link
Member

Choose a reason for hiding this comment

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

This looks optimized..?

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed, the test was out of date. Fixed.

@tlively tlively enabled auto-merge (squash) July 31, 2025 18:02
@tlively tlively merged commit fc6a797 into main Jul 31, 2025
16 checks passed
@tlively tlively deleted the heap-store-desc-effects branch July 31, 2025 18:31
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.

3 participants