Skip to content

Exhaustively test OptimizeInstructions on ref.cast #7542

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

Merged
merged 2 commits into from
Apr 23, 2025

Conversation

tlively
Copy link
Member

@tlively tlively commented Apr 23, 2025

Write a script to systematically generate test cases for all possible
interesting cast patterns. Update the code to correctly handle the case
where the cast is known to succeed but still requires a cast to recover
exactness.

Write a script to systematically generate test cases for all possible
interesting cast patterns. Update the code to correctly handle the case
where the cast is known to succeed but still requires a cast to recover
exactness.
@tlively tlively requested a review from kripken April 23, 2025 19:11
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.

With this automation, it is not practical to read the testcase and each of the CHECKS to see the outcome (and in particular, there are no human-written comments explaining each outcome). That makes sense here. But then what the test gives us is IIUC checking for assertions in the pass and validation errors in the output. Can we also do runtime checks, by executing the code with --fuzz-exec which would verify behavior remains the same after the optimization?

That would require adding more stuff so the code is fully runnable, but seems practical?

@tlively
Copy link
Member Author

tlively commented Apr 23, 2025

It's a good idea, but I don't think it's feasible right now because our interpreter does not yet use exact types. If you tried to interpret (ref.cast $foo (struct.new $foo)), it would currently fail. Once we have landed the changes to type allocations as exact, we would be able to revisit this.

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.

I see, sounds good.

@tlively tlively merged commit 2d97856 into main Apr 23, 2025
14 checks passed
@tlively tlively deleted the optimize-instructions-all-casts branch April 23, 2025 20:28
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