AnyDSL / thorin

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

Partial evaluator bug. #8

Closed madmann91 closed 9 years ago

madmann91 commented 9 years ago

Here is a simple code that generates an assertion `pred->direct_succs().size() == 1 && "critical edge"' failure. The strange thing is that if you remove the @ on the if branch, or if you leave it but replace the push_node calls by the equivalent inlined code, it works fine. It even works when you add @ signs on each push_node call and remove the @ on the branch.

struct Node {
    child_first: int,
    prim_count: int,
}

extern fn test_recursion(mut nodes: ~[Node]) -> () {
    let mut stack: [int * 32];
    let mut stack_idx = 1;

    fn push_node(node: int) -> () {
        stack(stack_idx) = node;
        stack_idx += 1;
    }

    fn pop_node() -> int {
        stack_idx -= 1;
        stack(stack_idx)
    }

    stack(0) = 0;
    while stack_idx > 0 @{
        let node_id = pop_node();
        let cur_node = &nodes(node_id);

        if cur_node.prim_count == 0 @{ // This is the @ sign that causes the assertion failure 
            // The following manually inlined code works fine :
            /*stack(stack_idx) = cur_node.child_first;
            stack_idx += 1;
            stack(stack_idx) = cur_node.child_first + 1;
            stack_idx += 1*/

            // Adding @ signs each call here and removing the one on the branch works too
            push_node(cur_node.child_first);
            push_node(cur_node.child_first + 1)
        }
    }
}
leissa commented 9 years ago

I tried on current new_master (thorin and impala branches). It seems to work there. Once I've fixed the remaining issues in new_master I will merge new_master back to master. Can you plz try if on new_master you get the expected result?

madmann91 commented 9 years ago

Just checked this on new-master branch (I re-compiled impala and thorin), and I still have the issue. Did you run impala with -emit-llvm ? The assertion is triggered only when you pass this option (or -emit-thorin). Otherwise there might be something I'm missing.

madmann91 commented 9 years ago

Oh turns out I was using a local branch 'new-master' instead of new_master. Still, it does not compile. It says 'merge_lambdas.cpp' is missing.

madmann91 commented 9 years ago

The file "il.cpp" is also missing.

richardmembarth commented 9 years ago

Reconfigure cmake, those files have been removed in the new_master branch.

madmann91 commented 9 years ago

You are right. This bug seems to have disappeared. You can mark this issue as resolved then. Note that my benchmark suite that compiled in under 20 seconds with the previous version now takes more than 20 minutes to compile (I didn't wait for the end so it might not even terminate). But this is another issue.

leissa commented 9 years ago

This is one of the major reasons I haven't merged back to master.