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

replaceSymbolicSizes #3346

Merged
merged 2 commits into from
Nov 6, 2024
Merged

replaceSymbolicSizes #3346

merged 2 commits into from
Nov 6, 2024

Conversation

naoyam
Copy link
Collaborator

@naoyam naoyam commented Nov 5, 2024

can't repro but this seems to be the right thing to do

Update: added a standalone repro

@naoyam
Copy link
Collaborator Author

naoyam commented Nov 5, 2024

!test --diff

@naoyam
Copy link
Collaborator Author

naoyam commented Nov 5, 2024

!test --diff-bench

@naoyam naoyam marked this pull request as ready for review November 5, 2024 21:35
@naoyam
Copy link
Collaborator Author

naoyam commented Nov 5, 2024

@jacobhinkle As I mentioned offline, there's one test that had register usage jump, but I still think this is the right thing to do. I started the diff check with the benchmarks as well.

@naoyam naoyam requested a review from jacobhinkle November 5, 2024 21:52
@naoyam naoyam changed the title [WIP] replaceSymbolicSizes replaceSymbolicSizes Nov 5, 2024
Copy link
Collaborator

@jacobhinkle jacobhinkle left a comment

Choose a reason for hiding this comment

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

LGTM

@naoyam
Copy link
Collaborator Author

naoyam commented Nov 6, 2024

@jacobhinkle No codegen change with the benchmarks.

@naoyam naoyam merged commit 86a6a80 into main Nov 6, 2024
27 of 28 checks passed
@naoyam naoyam deleted the fix_replace_sizes branch November 6, 2024 00:19
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