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

Improve handling of guards while flattening. #28392

Merged
merged 1 commit into from
Oct 18, 2024
Merged

Improve handling of guards while flattening. #28392

merged 1 commit into from
Oct 18, 2024

Conversation

mikebenfield
Copy link
Collaborator

@mikebenfield mikebenfield commented Oct 10, 2024

Specifically two changes are made:

  1. Cache Anded together chains of conditionals for early returns. This prevents quadratic code being generated in the face of a chain of ifs like
if a { return 0u32; } else if b { return 1u32 } else if ...
  1. Take into account early returns for asserts.

Fixes #28386, #28387

@mikebenfield
Copy link
Collaborator Author

Couple things to note:

This doesn't currently use the cached conditional idea for guarding against early returns for asserts... so in principle a lot of unnecessary code could be generated if there are multiple asserts following a bunch of early returns.

Also, the ntzseals.leo test now passes because it doesn't generate too much code.

tests/tests/execution/assert_early_return.leo Show resolved Hide resolved
compiler/passes/src/flattening/flattener.rs Outdated Show resolved Hide resolved
compiler/passes/src/flattening/flattener.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@d0cd d0cd left a comment

Choose a reason for hiding this comment

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

Minor nits, but looks good overall!

compiler/passes/src/flattening/flattener.rs Outdated Show resolved Hide resolved
compiler/passes/src/flattening/flattener.rs Outdated Show resolved Hide resolved
Specifically three changes are made:

1. Cache Anded together chains of conditionals for early returns.
This prevents quadratic code being generated in the face of a chain
of ifs like

```
if a { return 0u32; } else if b { return 1u32 } else if ...
```

2. Take into account early returns for asserts, and cache the
when reconstructing the assert, as with caching them for early returns.

Fixes #28386, #28387
@mikebenfield
Copy link
Collaborator Author

Alright; I've made a few changes and I think I've addressed everything including the SSA issue. Note that I rebased and changed the commit message a bit too.

Copy link
Collaborator

@d0cd d0cd left a comment

Choose a reason for hiding this comment

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

LGTM! Great work on this PR @mikebenfield!

@d0cd d0cd merged commit 86783da into mainnet Oct 18, 2024
12 of 13 checks passed
@d0cd d0cd deleted the ifelse branch October 18, 2024 21:09
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.

[Bug] if ... else if chain yields code of size quadratic in the number of conditions
2 participants