Ivan Chinenov (Nov 23 2021 at 10:09): Ivan Chinenov (Nov 23 2021 at 10:09):I am encountering a lot of this obscure error:
thread 'main' panicked at 'function must be compiled before it can be finalized', /home/johnd/.cargo/registry/src/github.com-1ecc6299db9ec823/cranelift-jit-0.78.0/src/backend.rs:356:14
It sure would be nice why exactly the function doesn't compile. As far as I explored, one thing that causes this is type mismatch between signature and actual params, but there's a bunch of more subtle things that cause this that i am yet to figure out
Ivan Chinenov (Nov 23 2021 at 10:41):Here is the CLIF for a function that fails to compile:
function u0:0(r64, r64) system_v {
ss0 = explicit_slot 31
ss1 = explicit_slot 16
ss2 = explicit_slot 16
ss3 = explicit_slot 16
sig0 = (r64, r64, r64) system_v
fn0 = u0:0 sig0
block0(v0: r64, v1: r64):
v19 -> v0
v2 = iconst.i64 0
v3 = stack_addr.r64 ss0
v4 = stack_addr.r64 ss1
v5 = iconst.i64 10
v6 = iconst.i64 1
store aligned v6, v4
store aligned v5, v4+8
call fn0(v3, v1, v4)
v8 = load.i64 aligned v3+8
brz v8, block2
jump block1
block1:
v9 = iconst.i64 0
v10 = stack_addr.r64 ss2
v11 = iconst.i64 1
v12 = iconst.i64 1
store aligned v12, v10
store aligned v11, v10+8
jump block3(v10)
block2:
v13 = iconst.i64 0
v14 = stack_addr.r64 ss3
v15 = iconst.i64 2
v16 = iconst.i64 1
store aligned v16, v14
store aligned v15, v14+8
jump block3(v14)
block3(v7: f64):
v17 = load.i64 aligned v7
v18 = load.i64 aligned v7+8
store aligned v17, v0
store aligned v18, v0+8
return
}
Did you define all functions you declared with a linkage other than Linkage::Import before you called .finalize_definitions()?
Ivan Chinenov (Nov 23 2021 at 10:46):Yes:
let callee = self
.module
.declare_function(self.env.lookup_symbol(name).unwrap(), Linkage::Import, &sig)
.expect("problem declaring function");
let local_callee = self.module.declare_func_in_func(callee, self.builder.func);
Note that in this case the function is recursive. I suspect that there is something wrong with how I translate if condition, since non-recursive calls without if condition work, but break when wrapped in an if
Ivan Chinenov (Nov 23 2021 at 10:48):For reference, here is the translation function for if: https://github.com/JohnDowson/CraneLisp/blob/struct-values/src/jit.rs#L367-L397
bjorn3 (Nov 23 2021 at 10:59):One unrelated nit. You can unconditionally call prepare_for_function_redefine at https://github.com/JohnDowson/CraneLisp/blob/aec9b9710b31f32e53634c3f4b81416cbd412b75/src/jit.rs#L119 I believe. Even if it hasn't been defined previously.
bjorn3 (Nov 23 2021 at 11:04):I don't see anything wrong with your code.
Ivan Chinenov (Nov 23 2021 at 11:05):I guess I should file an issue then.
Ivan Chinenov (Nov 23 2021 at 12:32):Linking the issue just in case https://github.com/bytecodealliance/wasmtime/issues/3559
bjorn3 (Nov 23 2021 at 13:13):@Ivan Chinenov ifjit.crl is missing in the struct-values branch.
Ivan Chinenov (Nov 23 2021 at 15:13):bjorn3 said:
Ivan Chinenov
ifjit.crlis missing in the struct-values branch.
Thank you for pointing this out, I forgot to push the file.
bjorn3 (Nov 23 2021 at 15:41):I can reproduce the issue now.
bjorn3 (Nov 23 2021 at 15:52):The problem was that module.declare_function returned an error other than DuplicateDefinition. This is completely ignored by your code. When adding an unwrap() I get:
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Verifier(VerifierErrors([VerifierError { location: inst17, context: Some("jump block3(v10)"), message: "arg 0 (v10) has type r64, expected f64" }, VerifierError { location: inst24, context: Some("jump block3(v14)"), message: "arg 0 (v14) has type r64, expected f64" }, VerifierError { location: inst25, context: Some("v17 = load.i64 aligned v7"), message: "arg 0 (v7) with type f64 failed to satisfy type set ValueTypeSet { lanes: BitSet(1), ints: BitSet(96), floats: BitSet(0), bools: BitSet(0), refs: BitSet(96) }" }, VerifierError { location: inst26, context: Some("v18 = load.i64 aligned v7+8"), message: "arg 0 (v7) with type f64 failed to satisfy type set ValueTypeSet { lanes: BitSet(1), ints: BitSet(96), floats: BitSet(0), bools: BitSet(0), refs: BitSet(96) }" }]))', /home/bjorn/Projects/wasmtime/cranelift/jit/src/backend.rs:659:37
@Ivan Chinenov
bjorn3 (Nov 23 2021 at 15:53):You are confusing r64 and f64.
Ivan Chinenov (Nov 23 2021 at 16:46):Oh god, this is embarrassing, I've stared at my code for many hours and did not notice that.
Notification Bot (Nov 27 2021 at 20:16):Ivan Chinenov has marked this topic as resolved.
Notification Bot (Nov 27 2021 at 20:37):Ivan Chinenov has marked this topic as unresolved.
Ivan Chinenov (Nov 27 2021 at 20:40):Okay, this time I didn't ignore any errors:
thread 'main' panicked at 'assertion failed: vlr_env[cand_vlrix].is_ref == is_ref', /home/johnd/.cargo/registry/src/github.com-1ecc6299db9ec823/regalloc-0.0.32/src/bt_spillslot_allocator.rs:295:13
clif:
function u0:0(r64) -> r64 system_v {
ss0 = explicit_slot 16
ss1 = explicit_slot 16
ss2 = explicit_slot 16
ss3 = explicit_slot 16
ss4 = explicit_slot 16
sig0 = () -> r64 system_v
sig1 = (i64) -> r64 system_v
sig2 = (i64, r64) -> r64 system_v
fn0 = u0:0 sig0
fn1 = u0:1 sig1
block0(v0: r64):
v1 = call fn0()
v2 = iconst.i64 0x55a4_6e04_70f0
v3 = raw_bitcast.r64 v2
v4 = stack_addr.r64 ss0
v5 = iconst.i64 1
v6 = iconst.i64 1
store aligned v6, v4
store aligned v5, v4+8
v7 = stack_addr.r64 ss1
v8 = iconst.i64 2
v9 = iconst.i64 1
store aligned v9, v7
store aligned v8, v7+8
v10 = stack_addr.r64 ss2
v11 = iconst.i64 3
v12 = iconst.i64 1
store aligned v12, v10
store aligned v11, v10+8
v13 = stack_addr.r64 ss3
v14 = iconst.i64 4
v15 = iconst.i64 1
store aligned v15, v13
store aligned v14, v13+8
v16 = stack_addr.r64 ss4
v17 = iconst.i64 5
v18 = iconst.i64 1
store aligned v18, v16
store aligned v17, v16+8
v19 = iconst.i64 5
v20 = iconst.i64 40
v21 = call fn1(v20)
store aligned v4, v21
store aligned v7, v21+1
store aligned v10, v21+2
store aligned v13, v21+3
store aligned v16, v21+4
v22 = call_indirect sig2, v3(v19, v21)
return v22
}
What I am trying to do: call malloc and pass the resulting pointer to a callee:
// malloc
let mut m_sig = self.module.make_signature();
m_sig.params.push(AbiParam::new(types::I64));
m_sig.returns.push(AbiParam::new(types::R64));
let malloc = self
.module
.declare_function("malloc", Linkage::Import, &m_sig)?;
let local_malloc = self.module.declare_func_in_func(malloc, self.builder.func);
let mut sig = self.module.make_signature();
sig.returns.push(AbiParam::new(types::R64));
sig.params.push(AbiParam::new(types::I64));
sig.params.push(AbiParam::new(types::R64));
#[allow(clippy::fn_to_numeric_cast)]
if func.body as u64 != 0 {
let sig = self.builder.func.import_signature(sig);
let callee = self.builder.ins().iconst(types::I64, func.body as i64);
let callee = self.builder.ins().raw_bitcast(types::R64, callee);
let mut arg_values = Vec::new();
let args = exprs
.into_iter()
.map(|a| self.translate_expr(a))
.collect::<Result<Vec<_>>>()?;
let count = self.builder.ins().iconst(types::I64, args.len() as i64);
// call malloc
let count_bytes = self
.builder
.ins()
.iconst(types::I64, (args.len() * 8) as i64);
let call = self.builder.ins().call(local_malloc, &[count_bytes]);
let heap_addr = self.builder.inst_results(call)[0];
let mut memflags = MemFlags::new();
memflags.set_aligned();
for (i, arg) in args.into_iter().enumerate() {
self.builder.ins().store(memflags, arg, heap_addr, i as i32);
}
arg_values.push(count);
arg_values.push(heap_addr);
let call = self.builder.ins().call_indirect(sig, callee, &arg_values);
self.builder.inst_results(call)[0].okay()
The raw_bitcast.r64 may be the problem. The register allocator has some problems with dealing with this kind of construct. @Chris Fallin regalloc2 fixes this once it gets merged, right?
bjorn3 (Nov 27 2021 at 22:02):@Ivan Chinenov Please @ mention me if you respond so I get a notification.
Ivan Chinenov (Nov 27 2021 at 22:09):@bjorn3 so if it is invalid to use R64 with iconst, and it is invalid to bitcast I64 to R64, how am I supposed to get a R64 value from an external pointer?
bjorn3 (Nov 27 2021 at 22:13):I guess storing on the stack as i64 and loading as r64 may work around this bug.
Ivan Chinenov (Nov 27 2021 at 22:26):@bjorn3 oh, yeah, that works.
Ivan Chinenov (Nov 28 2021 at 12:34):Oh, I think I could've just used const_addr.
Ivan Chinenov (Nov 28 2021 at 18:02):@bjorn3 is there some way to insert comments into generated function clif? I see that there is something like that in rustc_cranelift_codegen but it seems to be fairly intertwined with abstractions you've built over the cranelift.
bjorn3 (Nov 28 2021 at 18:09):CommentWriter from rustc_codegen_cranelift doesn't depend on any abstractions at all. All functions that depend on abstractions in cg_clif are convenience functions only.
bjorn3 (Nov 28 2021 at 18:10):@Ivan Chinenov Maybe I should upstream CommentWriter one day.
Ivan Chinenov (Nov 28 2021 at 19:34):@bjorn3 that would be supremely useful!
On a side note, do you know if call_indirect supposed to use half-width register for callq and then jump into non-executable memory?
bjorn3 (Nov 28 2021 at 20:07):No, call_indirect should directly call into executable memory. For cranelift-jit it will jump to a stub that calls the actual function. This is how redefining functions can work without having to make all executable memory writable again and relocating everything.
bjorn3 (Nov 28 2021 at 20:07):@Ivan Chinenov Can you show a disassembly?
Ivan Chinenov (Nov 28 2021 at 20:22):@bjorn3 I did some rubber duck debugging on people in the rust community discord and found out that i'm either processing or passing arguments incorrectly. I'm basically doing variadics via a heap array.
Ivan Chinenov (Nov 28 2021 at 20:24):Either here https://github.com/JohnDowson/CraneLisp/blob/main/src/jit.rs#L571-L625
or here https://github.com/JohnDowson/CraneLisp/blob/main/src/function.rs#L26-L30
respectively
Ivan Chinenov (Nov 28 2021 at 20:25):I hope you don't mind being poked by my footshooting questions.
bjorn3 (Nov 28 2021 at 20:31):@Ivan Chinenov The jitted code expects an array of *mut Atom, right? You are passing a reference to an array of Atoms.
Ivan Chinenov (Nov 28 2021 at 20:32):Also one thing i'm wondering, is to how to do pointer arithmetic without the load/store trick, cause iadd_imm doesn't work on R types
bjorn3 (Nov 28 2021 at 20:34):Cranelift reference types are currently pretty much only used for wasm reference types, which are opaque.
bjorn3 (Nov 28 2021 at 20:34):I think adding support for pointer arithmetic to reference types makes sense though.
Ivan Chinenov (Nov 28 2021 at 20:38):That's super counter-intuitive. And also wouldn't you have to cast either way to use I64 with store/load?
Ivan Chinenov (Nov 28 2021 at 20:39):Also yes, that one mistake in call cost me at 4-5 hours today, seems to work fine now.
Ivan Chinenov (Nov 28 2021 at 20:41):@bjorn3 although for some reason only first two arguments work, trying to use a third one still causes a segfault.
bjorn3 (Nov 28 2021 at 20:52):store and load work just fine with i64. cranelift ir uses i64 as pointer type on 64bit systems. r64 exists for when you have a garbage collector that needs to know where on the stack it can find roots. Every r64 value is considered a root.
bjorn3 (Nov 28 2021 at 20:52):bjorn3 although for some reason only first two arguments work, trying to use a third one still causes a segfault.
did you push your changes?
Ivan Chinenov (Nov 28 2021 at 20:58):bjorn3 said:
store and load work just fine with i64. cranelift ir uses i64 as pointer type on 64bit systems. r64 exists for when you have a garbage collector that needs to know where on the stack it can find roots. Every r64 value is considered a root.
That's certainly nice to know.
Ivan Chinenov (Nov 28 2021 at 20:59):@bjorn3 said:
bjorn3 although for some reason only first two arguments work, trying to use a third one still causes a segfault.
did you push your changes?
I have now, although that shouldn't affect anything.
bjorn3 (Nov 28 2021 at 21:05):The code to call into the jitted code looks correct now. I can't find anything obviously wrong with the argument handling in the jitted code either right now.
Ivan Chinenov (Nov 28 2021 at 21:07):@bjorn3 Another thing, am I using memflags right?
Ivan Chinenov (Nov 28 2021 at 21:11):As in, everything should be aligned, right?
bjorn3 (Nov 28 2021 at 21:25):Yeah, the stackslot should be 16 bytes aligned and the array 8 bytes aligned. All loads only need 8 bytes alignment and load at a multiple of 8 offset from the start of the array, so everything should be aligned just fine.
bjorn3 (Nov 28 2021 at 21:27):If it isn't aligned, you won't notice anything going wrong on x86 by the way unless you try to load or store floats or vectors. Cranelift itself doesn't have any optimization that will cause unaligned accesses to misbehave if the underlying CPU allows them.
Chris Fallin (Nov 29 2021 at 04:10):bjorn3 said:
The
raw_bitcast.r64may be the problem. The register allocator has some problems with dealing with this kind of construct. Chris Fallin regalloc2 fixes this once it gets merged, right?
Not directly but at least puts us in a better place to fix this. raw_bitcast in general is a hack and I'd prefer to have real ref-to-int and int-to-ref operators; but right now, you're right (and apologies @Ivan Chinenov ) the way that reference-typed values work is mostly filled out just enough to support Wasm reference types, which are completely opaque and never cast to/from ints or pointers within the Cranelift-compiled code, only carried through and passed back out.
Last updated: Dec 23 2024 at 12:05 UTC