Skip to content
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

Bare infinite loops may trick scopes analysis #122

Open
Hugobros3 opened this issue Mar 28, 2022 · 3 comments
Open

Bare infinite loops may trick scopes analysis #122

Hugobros3 opened this issue Mar 28, 2022 · 3 comments
Labels

Comments

@Hugobros3
Copy link
Contributor

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
Copy link
Contributor

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

@madmann91 madmann91 added the bug label Mar 29, 2022
@Hugobros3
Copy link
Contributor Author

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
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants