AnyDSL / thorin

The Higher-Order Intermediate Representation
https://anydsl.github.io
GNU Lesser General Public License v3.0
150 stars 15 forks source link

Bare infinite loops may trick scopes analysis #122

Open Hugobros3 opened 2 years ago

Hugobros3 commented 2 years ago

This delightful snippet (for Artic) tricks Thorin into thinking a basic block should be the entry of a scope:

#[export]
fn zoop() {
  while true {}
}

we get the following IR after opt():

module 'loop'

extern zoop_254: fn[mem, fn[mem]] = (zoop_255, ret_256) => {
    while_head_257(zoop_254.zoop_255)
}

while_head_257: fn[mem] = (while_head_258) => {
    while_head_257(while_head_257.while_head_258)
}

According to our current Scope analysis, while_head is free within the scope of zoop, as it does not use any of zoop's parameters. consequently, while_head is later processed a scope on it's own, which breaks the codegen_prepare pass that expects a valid return parameter on all scope entries.

This is a problem for both master and app-node (though I just added an assert against this scenario in codegen_prepare instead of letting it segfault).

Any comments ?

madmann91 commented 2 years ago

Maybe Scope::for_each should check the order of a continuation, so that it only processes continuations with even order?

Hugobros3 commented 2 years ago

We could do that, but we're still facing a problem since the incriminated free continuation won't be processed/emitted (since it's not part of another scope either). So broken codegen at least still.

One way I thought this could be made to work is reversing the criteria: instead of considering everything that uses the entry continuation params, we could perhaps look at everything (odd ordered?) that doesn't use foreign parameters. But thinking about it it seems to create more problems than anything.

Another possibility is doing an analysis based on a control flow graph, and including/rejecting callees based on their order (so you avoid including fn calls). That also sounds quite brittle but maybe it would work ?

I should point out Impala doesn't suffer from this, because the loop header gets another (unused) parameter, and that "grounds" it in the scope. However the unused parameter is undesirable and should even be taken care of by an smarter parameter elimination pass (aware of loops)

madmann91 commented 2 years ago

We could do that, but we're still facing a problem since the incriminated free continuation won't be processed/emitted (since it's not part of another scope either). So broken codegen at least still.

Well that's another bug. The backends and other analyses/transformations should rely on the CFG to determine the list of basic blocks, not on the scope.