-
Notifications
You must be signed in to change notification settings - Fork 244
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
fix: Try to turn acir lambdas into brillig #7279
base: master
Are you sure you want to change the base?
Conversation
self.in_unconstrained_function = true; | ||
let res = self.block(block.statements); | ||
self.in_unconstrained_function = was_in_unconstrained_function; |
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.
This seems like we're treating the contents of an unsafe
block as unconstrained. This isn't the case however as it's still constrained but the only difference is that we can call into unconstrained functions.
let target_fns = | ||
signature_to_functions_as_value.get(&dispatch_signature).cloned().unwrap_or_default(); | ||
variants.insert((dispatch_signature, caller_runtime), target_fns); | ||
let key = (dispatch_signature, caller_runtime); | ||
let target_fns = signature_to_functions_as_value.get(&key).cloned().unwrap_or_default(); |
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.
With this change we end up in a situation where if the lambda is created in constrained code, and subsequently is passed to unconstrained code, then this list is empty and the compiler crashes further down.
Description
Problem*
Resolves #7247
Summary*
Draft because it's unclear how to fix the
unconstrained
flag assigned to lambdas by theMonomorphizer
.The lambda expression itself inherits the flag from where it's defined, rather than how it's going to be used. I changed it so that if it's created in an
unsafe
block then it becomesunconstrained
(which I now understand is not correct), but there are examples such asarray::sort
test where we create a lambda in a constrained function, pass it to another constrained function, which uses it in its body in anunsafe
block to pass it on to anunconstrained
function it calls. It could call it itself outside theunsafe
block as well, which means afunction
parameter would potentially need to be bothbrillig
andacir
, depending on which block it's called from. Yet, we pass a single function ID in SSA, so how could it be both?I thought about changing it to
unconstrained ordering: fn[Env](T, T) -> bool
, and then when we callsort_via
we could look at its call parameter types to divine that the lambda should be unconstrained, but if we maintain that it can be called in two ways, then I'm not sure how to do it, because the lambda argument is turned into a function before we process the function we pass it to, so the information of how it's used might not be available until later. Also, the user doesn't have to add this at the moment, in which case anacir
lambda is created, and in the SSA we are stuck with the situation we wished to avoid, withbrillig
callingacir
.I also considered flipping the flag in the lambda function when we process the function body of its caller, but there is no 1:1 correspondence between the callee and the lambda, e.g. multiple lambdas can be passed to it, so we would have to somehow remember which lambdas were passed to which functions.
Perhaps if we process the callee function first, we can gather enough information about how the arguments are used, but it sounds like it would flip everything on its head to process dependencies first, rather than queue them for later. Maybe the SSA pass like the runtime separation used to be is the more convenient way to clone lambdas with caller-specific runtimes.
Additional Context
Documentation*
Check one:
PR Checklist*
cargo fmt
on default settings.