make capy tell cranelift about whether parameters are structs passed indirectly

[lenawanel]

This is a first step of making capy fully comply with the system v calling convention.

open issues

1. The sys v x64 spec states that

The classification of aggregate (structures and arrays) and union types works
as follows: 1. If the size of an object is larger than four eightbytes, or it contains unaligned
fields, it has class MEMORY ...

This is why this commit treats arrays and structs as the same, even though the cranelift docs only mention structs (is this correct).

2. alignment: this code currently artificially pretends that all structs/arrays are 8 byte aligned
3. actually split the structures into their components if they are "[smaller] than four eightbytes"

[lenawanel]

is this a problem with cranelift?
at least the last test run failed when cranelift applied relocations, which I don't see how these commits touch.

[NotAFlyingGoose]

This is a really good contribution. I think the failing tests are most likely(?) an issue in cranelift. Considering the fact that cranelift-jit is the source of the assertion failure I think that's a safe assumption.

I can go more in depth into this later

[NotAFlyingGoose]

I'm getting the bug on my M2 so I'm looking into this

[NotAFlyingGoose]

Okay so this is specifically a problem with calling a function with a struct argument on A64 machine code.

MRE:

Foo :: struct {
    bar: i32,
};

main :: () {
    hello(.{
        bar = 42,
    });
}

hello :: (some_foo: Foo) {}

note: I tried making Foo empty and I found a bug, although it's unrelated to this issue

The exact assertion failure in cranelift-jit is here.

A temporary fix might be to disable this optimization on A64 targets, but that would be giving in and so that's not gonna happen

[lenawanel]

now the assertion isn't triggered in cranelift, the problem now is wrong codegen. maybe this is what these lines in rustc are for: https://github.com/rust-lang/rust/blob/11380368dc53d0b2fc3a627408818eff1973ce9a/compiler/rustc_codegen_cranelift/src/common.rs#L401-L413

[NotAFlyingGoose]

We could do something similar to cast

create_stack_slot_addr could be in mod.rs and the definition here would just feed it a &mut self.builder and self.ptr

I think that's preferable to cloning the parameters, especially since this function might also be used by MemoryLoc and cast

Although fixing the A64 issue is more important than this comment

[NotAFlyingGoose]

now the assertion isn't triggered in cranelift, the problem now is wrong codegen.

Actually it looks like the JIT assertion failure is still there on top of the fibonacci problem ☹️

This is really disappointing, especially bc the rustc compiler has jit compilation as an optional feature.
The thing that makes me think it's a cranelift-jit internal issue is the fact that it's an assertion failure.
Usually cranelift will return an error properly if there's been a user error, but that's not what's happening here.
But at the same time, there must be something that rustc is doing that's not being done here.

Either way, I think the fact that it's an assertion failure is a very clear indicator that there should be an issue posted in the cranelift-jit repo. Although I'd still like to figure out what we're missing here. Maybe rustc does something different when "jit" is enabled?

[lenawanel]

Do you have a minimal cranelift ir example?

bytecodealliance/wasmtime#7820 is maybe the same issue we have, since they get the same panic.

Also I think it's also possible to enable is_pic when jiting to arm64, since bytecodealliance/wasmtime#2907 is closed.

[NotAFlyingGoose]

Do you have a minimal cranelift ir example?

I only have the minimum capy code example, but I'm gonna try and make a minimum ir one too.
The source of all this is mainly the aggr_pointee_size. If you set it to None the crash goes away and it get's replaced by zsh: bus error 😭

bytecodealliance/wasmtime#7820 is maybe the same issue we have, since they get the same panic.

I'm not sure, in that issue they talked about changing the linkage but that didn't do much for me. I think this is similar but different

Also I think it's also possible to enable is_pic when jiting to arm64, since bytecodealliance/wasmtime#2907 is closed.

If you set is_pic to true cranelift panics and says you're not allowed to do that bytecodealliance/wasmtime#2735
Although this is only for JIT

[NotAFlyingGoose]

Okay I made an issue in the cranelift repo bytecodealliance/wasmtime#8852

[lenawanel]

since you have to implement the whole platform-specific abis, I'll close this pr and work on a new one doing that instead