-
Notifications
You must be signed in to change notification settings - Fork 18
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
We need to stop relying on lower2cff #137
Comments
See #95. What you need here is to lift |
To expand a little bit on that, you don't need to closure-convert |
Yes correct. There is one annoying edge case though: What happens, if the loop body contains again a continuation as free variable? This may occur, e.g., if you capture the |
@madmann91 Thanks for the input! I have started a https://github.com/AnyDSL/thorin/commits/lift2cff branch where I implement this, it works on simple examples, but it's not actually very powerful on its own because of two reasons:
Closure conversion seems very unhappy with all the new work it has to do, and swiftly proceeds to produce broken IR. Me and @m-kurtenacker have been plotting to nuke and rewrite it so that's not a big deal. Also this new @leissa Exactly, I have actually decided to use jmp_buf to implement the |
I wrote this sample to reduce a problem with my substitution_rework branch, where by accidentally making eta conversion weaker I made the compiler more or less explode. I think it's a good example of some larger conceptual issues at play though:
Full IR dump follows later in the document. Here are detailed explanations about what's going on:
The return continuation for the inner loop call (
cont_75
) jumps to the break point of the loop (cont_79
)Both of them are dead code but this is irrelevant because eliminate_params cannot see it: the loop header return parameter is 'used' by
cont_75
, andcont_75
itself looks live becauseloop_65
passes it to itself every further iteration. This is a separate defect though.If eta reduction is working correctly,
cont_75
reduces toloop_65.ret_67
(the return param ofloop_65
).When lower2cff runs,
loop_65
's return parameter is eliminated cleanly (sinceloop_65
is not top-level, it may not have a ret param of order 2). this is the case because there is a substitution fromret_67
(the param) tocont_79
(the argument in the outer call), and so both calls into loop can be changed to the new mangled CFF-form loop.If however, eta reduction is disabled,
lower2cff
will find the inner call toloop_65
uses a different def:cont_75
. It thus proceeds to manglecont_75
as well, effectively duplicating it - but that means the inner call is still calling the old version ofloop_65
with the old signature, we've effectively only managed to peel one iteration!This process goes on until either the entire loop is fully peeled, or the compiler blows up.
A valid follow-up question is: how come we need closure conversion at all ? Well,
lower2cff
can get stuck and recognize that fact (that it did not manage to change anything at all in the last iteration), and it quits then. There are also scenarios where a function can be stored to memory, which escapes any order-based type signature analyses. At least that's my best understanding of those scenarios.It seems clear to me that this approach to lowering everything higher-order is not viable. It unpredictably blows up the code size and makes it possible for the compiler to diverge, even though staring at the PE filters suggests it would not.
I propose instead making higher-order functions a heuristic for inlining, giving them a higher budget (also scaling with opt level?) but stopping after a while. Instead we'd rely on closure conversion more often (possibly bringing back the need for a GC on the table).
The text was updated successfully, but these errors were encountered: