Skip to content

Conversation

vouillon
Copy link
Contributor

@vouillon vouillon commented Jul 5, 2024

No description provided.

(func $foo (export "foo") (param $p i32)
;; The locals $x and $y can be coalesced into a single local, but as we do not
;; run that pass, they will not be. Other minor optimizations will occur here,
;; such as using a tee.
Copy link
Member

Choose a reason for hiding this comment

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

This comment says a tee will happen, which contradicts the previous one if I am not mistaken?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I copied the test from O1_skip.wast but forgot to update all the comments. This is fixed.

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 with those fixes.

;; CHECK-NEXT: )
(func $foo (export "foo") (param $p i32)
;; The locals $x and $y can be coalesced into a single local, but as
;; we do not run that pass, they will not be. The could be
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 run that pass, they will not be. The could be
;; we do not run that pass, they will not be. They could be
Comment on lines 7 to 8
;; There should also be no warning in the output.

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
;; There should also be no warning in the output.

May as well delete this as the test is not verifying that atm. (It could in theory if we added additional stuff here, but I don't think it's worth it.)

@kripken kripken merged commit 994d8d3 into WebAssembly:main Jul 17, 2024
@vouillon vouillon deleted the skip-passes branch July 17, 2024 16:10
@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

2 participants