Relax constprop recursion detection #39918
Conversation
At the moment, we restrict const prop whenever we see a cycle in methods being called. However, I think this condition can be relaxed slightly: In particular, if the type complexity limiting did not decide to limit the growth of the type in question, I think it should be fine to constant prop as long as there is no cycle in *method instances* (rather than just methods). Fixes #39915
| @@ -456,7 +458,7 @@ function abstract_call_method_with_const_args(interp::AbstractInterpreter, @nosp | |||
| infstate = sv | |||
| cyclei = 0 | |||
| while !(infstate === nothing) | |||
| if method === infstate.linfo.def && any(infstate.result.overridden_by_const) | |||
| if (edgelimited ? method === infstate.linfo.def : mi === infstate.linfo) && any(infstate.result.overridden_by_const) | |||
There was a problem hiding this comment.
This seems to make sense in general, but I'd need to think hard through the specifics here to make sure it is good.
So, here we are checking with mi, which isn't normally a limited object, and thus would normally not be valid here for cycle detection (thus also why we don't use it originally in cycle detection). But I guess the concept is that abstract_call_gf_by_type is expected to deal with that possibility (via setting edgelimited when required), so we won't get here if this would be unrestricted in complexity, and making this === equivalent to <=?
There was a problem hiding this comment.
Correct. Basically, the intuition is that the same logic that prevents infinite recursion over the type lattice should also restrict infinite recursion of the lattice that adjoins const elements, except that we can get additional recursion by calling the same method instance with different const elements (which is a cycle, but not infinite over the type lattice). There's an example that shows that case added in the tests (I verified the example stack overflows if you replace edgecycle by edgelimited above).
|
Huh. And here I was thinking this was a total impossibility. This opens up a lot of doors leading to doors leading to doors leading to doors. |
|
|
||
| # issue #39915 | ||
| function f33915(a_tuple, which_ones) | ||
| rest = my_getindex(tail(a_tuple), tail(which_ones)) |
There was a problem hiding this comment.
| rest = my_getindex(tail(a_tuple), tail(which_ones)) | |
| rest = f33915(tail(a_tuple), tail(which_ones)) |
| @@ -542,7 +545,6 @@ function abstract_call_method(interp::AbstractInterpreter, method::Method, @nosp | |||
| parent_method2 isa Method || (parent_method2 = nothing) # Union{Method, Nothing} | |||
| if parent.linfo.def === sv.linfo.def && sv_method2 === parent_method2 | |||
| topmost = infstate | |||
| edgecycle = true | |||
There was a problem hiding this comment.
What's the reason to delete these edgecycle = true ?
In these cases there is a cycle, and we still need to try recursion detection in abstract_call_method_with_const_args ?
There was a problem hiding this comment.
Yes, you're right - these were deleted by accident (leftover from an earlier experiment) - thanks.
|
Needs an update after #39305 |
|
I will try it. |
At the moment, we restrict const prop whenever we see a cycle in methods being called. However, I think this condition can be relaxed slightly: In particular, if the type complexity limiting did not decide to limit the growth of the type in question, I think it should be fine to constant prop as long as there is no cycle in *method instances* (rather than just methods). Fixes JuliaLang#39915, replaces JuliaLang#39918 Co-Authored-By: Keno Fisher <[email protected]>
|
Continued in #40561 |
At the moment, we restrict const prop whenever we see a cycle in methods being called. However, I think this condition can be relaxed slightly: In particular, if the type complexity limiting did not decide to limit the growth of the type in question, I think it should be fine to constant prop as long as there is no cycle in *method instances* (rather than just methods). Fixes JuliaLang#39915, replaces JuliaLang#39918 Co-Authored-By: Keno Fisher <[email protected]>
At the moment, we restrict const prop whenever we see a cycle in methods being called. However, I think this condition can be relaxed slightly: In particular, if the type complexity limiting did not decide to limit the growth of the type in question, I think it should be fine to constant prop as long as there is no cycle in *method instances* (rather than just methods). Fixes JuliaLang#39915, replaces JuliaLang#39918 Co-Authored-By: Keno Fisher <[email protected]>
At the moment, we restrict const prop whenever we see a cycle in methods being called. However, I think this condition can be relaxed slightly: In particular, if the type complexity limiting did not decide to limit the growth of the type in question, I think it should be fine to constant prop as long as there is no cycle in *method instances* (rather than just methods). Fixes JuliaLang#39915, replaces JuliaLang#39918 Co-Authored-By: Keno Fisher <[email protected]>
At the moment, we restrict const prop whenever we see a cycle in methods being called. However, I think this condition can be relaxed slightly: In particular, if the type complexity limiting did not decide to limit the growth of the type in question, I think it should be fine to constant prop as long as there is no cycle in *method instances* (rather than just methods). Fixes #39915, replaces #39918 Co-authored-by: Keno Fisher <[email protected]>
At the moment, we restrict const prop whenever we see a cycle in methods being called. However, I think this condition can be relaxed slightly: In particular, if the type complexity limiting did not decide to limit the growth of the type in question, I think it should be fine to constant prop as long as there is no cycle in *method instances* (rather than just methods). Fixes JuliaLang#39915, replaces JuliaLang#39918 Co-authored-by: Keno Fisher <[email protected]>
At the moment, we restrict const prop whenever we see a cycle in methods being called. However, I think this condition can be relaxed slightly: In particular, if the type complexity limiting did not decide to limit the growth of the type in question, I think it should be fine to constant prop as long as there is no cycle in *method instances* (rather than just methods). Fixes JuliaLang#39915, replaces JuliaLang#39918 Co-authored-by: Keno Fisher <[email protected]>
At the moment, we restrict const prop whenever we see a cycle in
methods being called. However, I think this condition can be relaxed
slightly: In particular, if the type complexity limiting did not
decide to limit the growth of the type in question, I think it
should be fine to constant prop as long as there is no cycle in
method instances (rather than just methods).
Fixes #39915