-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Find cycles in rewrite constraints without performing the full exponential expansion of the RHS #5673
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
base: trunk
Are you sure you want to change the base?
Conversation
I couldn't come up with a counter example yet that would still be exponential with a power of the size of the cycle, before finding said cycle, under the solution proposed here. |
T:! Z where | ||
.T1 = (.T2, .T2, .T2) and | ||
.T2 = (.T3, .T3, .T3) and | ||
.T3 = (.T4, .T4, .T4) and | ||
.T4 = (.T5, .T5, .T5) and | ||
.T5 = (.T6, .T6, .T6) and | ||
.T6 = (.T7, .T7, .T7) and | ||
.T7 = (.T1, .T1, .T1) | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't we also get exponential runtime if we tweaked this example to not have a cycle? Eg
T:! Z where | |
.T1 = (.T2, .T2, .T2) and | |
.T2 = (.T3, .T3, .T3) and | |
.T3 = (.T4, .T4, .T4) and | |
.T4 = (.T5, .T5, .T5) and | |
.T5 = (.T6, .T6, .T6) and | |
.T6 = (.T7, .T7, .T7) and | |
.T7 = (.T1, .T1, .T1) | |
); | |
T:! Z where | |
.T1 = (.T2, .T2, .T2) and | |
.T2 = (.T3, .T3, .T3) and | |
.T3 = (.T4, .T4, .T4) and | |
.T4 = (.T5, .T5, .T5) and | |
.T5 = (.T6, .T6, .T6) and | |
.T6 = (.T7, .T7, .T7) and | |
.T7 = (i32, i32, i32) | |
); |
If we want that case to also be sub-exponential, I think we'd want a different strategy. A couple of options come to mind:
- Cache the type that each rewrite substitutes into and reuse it rather than rebuilding it.
- Precompute the dependencies between rewrites and substitute bottom-up. (But we'd need to handle the
.A = .B.A and .B = .Self
case somehow.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noting here that with #5692 and #5693 this particular example becomes much faster (17ms). But it's still exponential, so we just need to add more to make it slow.
This version of it, which doesn't have a cycle, and is thus not impacted by those changes, takes minutes (at least) to complete:
fn F(
T:! Z where
.T0 = (.T1, .T1, .T1, .T1, .T1, .T1, .T1, .T1, .T1, .T1, .T1, .T1) and
.T1 = (.T2, .T2, .T2, .T2, .T2, .T2, .T2, .T2, .T2, .T2, .T2, .T2) and
.T2 = (.T3, .T3, .T3, .T3, .T3, .T3, .T3, .T3, .T3, .T3, .T3, .T3) and
.T3 = (.T4, .T4, .T4, .T4, .T4, .T4, .T4, .T4, .T4, .T4, .T4, .T4) and
.T4 = (.T5, .T5, .T5, .T5, .T5, .T5, .T5, .T5, .T5, .T5, .T5, .T5) and
.T5 = (.T6, .T6, .T6, .T6, .T6, .T6, .T6, .T6, .T6, .T6, .T6, .T6) and
.T6 = (.T7, .T7, .T7, .T7, .T7, .T7, .T7, .T7, .T7, .T7, .T7, .T7) and
.T7 = (.T8, .T8, .T8, .T8, .T8, .T8, .T8, .T8, .T8, .T8, .T8, .T8) and
.T8 = (.T9, .T9, .T9, .T9, .T9, .T9, .T9, .T9, .T9, .T9, .T9, .T9) and
.T9 = ()
);```
Substitute one associated constant reference at a time on the RHS. If there's a cycle in the rewrite constraints, this allows it to be found without expanding the entire RHS, which can involve exponential type expansion, and repeating this growth a number of times equal to the number of steps in the cycle.
Fixes #5672