TomBebbington / llvm-rs

LLVM wrappers for Rust
BSD 3-Clause "New" or "Revised" License
68 stars 27 forks source link

How to call `JitEngine.with_function` with more than 1 argument #12

Closed Hywan closed 8 years ago

Hywan commented 8 years ago

Hello :-),

In your example, you have things like this:

let func = module.add_function("fib", Type::get::<fn(u64) -> u64>(&ctx));
…
let ee = JitEngine::new(&module, JitOptions {opt_level: 0}).unwrap();
ee.with_function(func, |fib: extern fn(u64) -> u64| {
    for i in 0..10 {
        println!("fib {} = {}", i, fib(i))
    }
});

This is fine. func has only one argument. But how to declare 2 arguments? If I do:

let func = module.add_function("fib", Type::get::<fn(u64, u64) -> u64>(&ctx));

it compiles and the produced IR is correct if I do not do ee.with_function. If I “uncomment” it:

ee.with_function(func, |fib: extern fn(u64, u64) -> u64| { … })

does not compile. We have the following error:

error: type mismatch: the type `[closure@source/bin/tvm.rs:200:28: 201:6]` implements the trait `core::ops::FnOnce<(extern "C" fn(u64, u64) -> u64,)>`, but the trait `core::ops::FnOnce<(extern "C" fn(_) -> _,)>` is required (incorrect number of function parameters) [E0281]

I really don't know how to fix this.

cc @pnkfelix, @HeroesGrave, @bryal.

bryal commented 8 years ago

I'm not at a pc atm, but try having the extern fn take a tuple as an argument, i.e.

ee.with_function(func, |fib: extern fn((u64, u64)) -> u64| { … })

Hywan commented 8 years ago

It compiles but I can't call fib(2, 3) for instance (inside the closure body) because: error: this function takes 1 parameter but 2 parameters were supplied [E0061].

Hywan commented 8 years ago

My complete (not working) example is the following:

fn main() {
    let ctx = Context::new();
    let module = Module::new("simple", &ctx);
    let func = module.add_function("add", Type::get::<fn(u64, u64) -> u64>(&ctx));
    func.add_attributes(&[NoUnwind, ReadNone]);
    let a = &func[0];
    let b = &func[1];
    a.set_name("a");
    b.set_name("b");

    let entry = func.append("entry");

    let builder = Builder::new(&ctx);
    builder.position_at_end(entry);
    builder.build_ret(builder.build_add(a, b));

    module.verify().unwrap();
    println!("{:?}", module);

    let ee = JitEngine::new(&module, JitOptions {opt_level: 0}).unwrap();
    ee.with_function(func, |add: extern fn((u64, u64)) -> u64| {
        for i in 0..25 {
            let j = i + 1;
            println!("add {} + {} = {}", i, j, add(i, j))
        }
    });
}
HeroesGrave commented 8 years ago

This should theoretically work:

--            println!("add {} + {} = {}", i, j, add(i, j))
++            println!("add {} + {} = {}", i, j, add((i, j)))

But it would be nice if there was a way to do this without such a noisy workaround.

Hywan commented 8 years ago

I tried but no, it does not work :-/. Because now, it does not match the LLVM function protoype: let func = module.add_function("add", Type::get::<fn(u64, u64) -> u64>(&ctx));.

pnkfelix commented 8 years ago

@Hywan I think the work around being suggested requires that you replace fn (u64, u64) -> u64 with fn ((u64, u64)) -> u64 everywhere in your code, at both the with_function call and the add_function call

of course this is entirely side stepping your question, since we are no longer using a multi-are function at that point

Hywan commented 8 years ago

Yup, if I use module.add_functionType::get::<fn((u64, u64) -> u64>(&ctx)), then the semantics is different. I really need 2 arguments, but the API does not allow that.

TomBebbington commented 8 years ago

Yeah, sorry, there's not really any workaround for this without making the entire with_function function unstable, since this would bypass the checking it has in place.

To use the function with the correct number of args (but sacrificing the sanity checking in place) use get_global like this:

unsafe {
    let add: &fn(u64, u64) = ee.get_global(func);
    for i in 0..25 {
        let j = i + 1;
        println!("add {} + {} = {}", i, j, add(i, j))
    }
}
Hywan commented 8 years ago

Thanks for the reply.