AnyDSL / thorin

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

LLVM rewrite #109

Closed madmann91 closed 3 years ago

madmann91 commented 3 years ago

This PR tracks the current status of the llvm_rewrite branch.

leissa commented 3 years ago

First examples are working again. Thorin output is currently broken. Since I need this to track down the remaining issues, I started the porting branch which incorporates a number of improvements from t2. As soon as porting is on a par with llvm_rewrite, I'll merge them.

madmann91 commented 3 years ago

Let us know if there's something we can do (testing? bug fixing?).

leissa commented 3 years ago

Okay, so I merged (and removed) the porting branch into llvm_rewrite. As a next step, I will implement a textual output that is at least usable to some extent for debugging purposes. Then, I will continue looking into the remaining issues in the llvm backend.

The biggest changes (besides tons of small things here and there) are:

Regarding the C-Backend: As promised, this needs some major overhaul ;) We would need to do the same adjustments as in the LLVM backend here. What is more, since the C backend is currently broken anyway, I changed the streaming API (from the t2 branch) into a more std::fmt kind of style. Actually resorting to std::fmt (or the fmt library) would definitely reduce some boilerplate on our side while gaining more features, but so far I didn't have the patience to do that.

madmann91 commented 3 years ago

Alright, I'll have a look at the things that need porting right now. I think I've already mostly ported the NVVM stuff anyway.

madmann91 commented 3 years ago

I think the remaining three backends rely on the C backend to work, so I'll leave them commented out until the C backend is done.

leissa commented 3 years ago

Another issue you might look into is the split_slots phase. It also relied on the instruction list - which is non-existent anymore. We need to re-implement this to simply work recursively.

madmann91 commented 3 years ago

We can simply leave it for the time being. It's only relevant for the C backends anyway, since LLVM has the "scalar replication of aggregates" phase. I've created a branch in artic named llvm_rewrite that contains fixes for this branch of thorin. There are still bugs before even getting into the LLVM backend. The following piece of code produces an assertion with artic on llvm_rewrite:

fn test2(x: i32, _: i32) -> i32 {
    test1(x, 2)
}
fn test1(x: i32, _: i32) -> i32 {
    test2(x, 1)
}

The result is the following:

> bin/artic test.art --emit-thorin -O1
artic: /home/madmann/Projects/anydsl/thorin/src/thorin/def.cpp:38: void thorin::Def::set_op(size_t, const thorin::Def*): Assertion `def && "setting null pointer"' failed.
fish: “bin/artic test.art --emit-thori…” terminated by signal SIGABRT (Abort)
leissa commented 3 years ago

I'm looking into it.

leissa commented 3 years ago

Your last example isn't a regression but is in fact also broken on master. I fixed it in 120349df6fff9fb889dc412a1239dc49de546e49 and ported to llvm_rewrite.

leissa commented 3 years ago

Most impala examples work again save 2-3 test cases. However, there are still some messy thing's I'd like to improve.

leissa commented 3 years ago

All except one example from impala work now. I will investigate the remaining example tomorrow. Then, I'll nuke Param::peek as mentioned above. Then, we can discuss any remaining issues and what to do with the C backend.

leissa commented 3 years ago

Alright, I think I'm done. I will polish the Thorin output a bit. But this is orthogonal to the other stuff. So the elephant in the room is: What will happen with the C backend?

madmann91 commented 3 years ago

Well we should port it I suppose?

leissa commented 3 years ago

:D The question was more, who is going to do that. The problem I have with the C backend, that there was never an easy way to test this thing.

madmann91 commented 3 years ago

You could just use Stincilla in OpenCL mode, with pocl

leissa commented 3 years ago

I will do that.

leissa commented 3 years ago

Okay, I looked into libfmt as an alternative to what we currently have. But there are a couple of serious problems that don't make libfmt a good fit for us. So, I'll stick with the streaming API we currently have in llvm_rewrite:

The downside is that we don't have this super fancy format mini language. If we really need that for whatever reason, we could still use libfmt for these cases.

madmann91 commented 3 years ago

Another downside is that std::fmt is part of the standard in C++20, which means we would have this functionality without having to rewrite it once it's "old enough" to be implemented by all compilers.

leissa commented 3 years ago

As I argued above, std::fmt doesn't help us. It's basically the same as libfmt and there are a couple of serious problems that don't make libfmt/std::fmt a good fit for us. So std::fmt/libfmt is at best a nice-to-have addition.

leissa commented 3 years ago

My last commit fixes the real problem of "bug fix: only emit phi-args for valid args". We must be careful to not forget the else-branch for cases like this:

if (auto sth = emit_unsafe(asdf); !sth.empty()) {
    // ...
} else {
    return "";
}
leissa commented 3 years ago

I added to impala's test script that the C backend will be tested as well. So far we are passing 71/100 tests (8 of which are inline asm examples). One probl is that we have to either run a topo sort to emit all functions in dependency order, or we have to declare them beforehand (and remove static). Example:

static void foo() {
    bar();
}

static void bar() {}
madmann91 commented 3 years ago

This should not be a problem. We should be emitting them as prototypes. I am working for a fix that emits every function prototype, but there are a couple of things that need refactoring first.