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

Small change to the aeneas script to make it more robust #299

Closed
wants to merge 2 commits into from

Conversation

eliotmoss
Copy link
Contributor

Otherwise, blocks that start with a phi do not get optimized.
An interesting question is whether there are other things in a block, besides SsaApplyOp's, that should get skipped over (or otherwise handled) in the while loop. But this change was effective in removing junk from XXGetField's expanded in normalization from accesses to unboxed variants, at least in my test case. Your CI can check to see whether this causes any regressions ...

@titzer
Copy link
Owner

titzer commented Nov 19, 2024

Otherwise, blocks that start with a phi do not get optimized.
An interesting question is whether there are other things in a block, besides SsaApplyOp's, that should get skipped over (or otherwise handled) in the while loop. But this change was effective in removing junk from XXGetField's expanded in normalization from accesses to unboxed variants, at least in my test case. Your CI can check to see whether this causes any regressions ...

Sounds good, but it looks like the PR only contains a change to the aeneas script.

@eliotmoss
Copy link
Contributor Author

Sigh -- yeah, wrong commit. I'm new to preparing PR's. I'll see about changing the title and content and then submitting the right PR!

@eliotmoss eliotmoss changed the title Fix load optimizer to skip the initial phi of a block Small change to the aeneas script to make it more robust Nov 19, 2024
to weird setting by people like me
@eliotmoss eliotmoss closed this Nov 19, 2024
@eliotmoss eliotmoss deleted the load_opt_skip_phi branch November 19, 2024 01:37
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