Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cranelift-filetest: Allow run directives with functions that have a vmctx #6659

Closed
fitzgen opened this issue Jun 28, 2023 · 4 comments · Fixed by #6663
Closed

cranelift-filetest: Allow run directives with functions that have a vmctx #6659

fitzgen opened this issue Jun 28, 2023 · 4 comments · Fixed by #6663
Assignees
Labels
cranelift Issues related to the Cranelift code generator

Comments

@fitzgen
Copy link
Member

fitzgen commented Jun 28, 2023

And allow passing in arbitrary values as vmctx as well, basically treating it like any other argument.

While this wouldn't allow complex global values based on the vmctx, it would allow simple ones like gv0 = vmctx.

We would also want the CLIF interpreter to support this as well, since basically all of our runtests are also interpreter tests.

@fitzgen fitzgen added the cranelift Issues related to the Cranelift code generator label Jun 28, 2023
@afonso360
Copy link
Contributor

We used to have some limited support for vmctx and global values in the interpreter (#4396). But that was essentially just used to support custom heaps, and was removed in (#5386).

We can still resolve global_value's in the interpreter, so we should still be able to use it, but we might have to manually call the function and pass the address to a stack slot or something along those lines.

Is that what you were looking to do?

We don't support something like ; run: %test(vmctx, 1, 2, 3) == 10, but I'm not sure what we would pass in instead of vmctx in the runtests.

Here's an example testcase using `vmctx` that works today
test interpret
test run
target x86_64
target s390x
target aarch64
target riscv64


function %vm_state() -> i64 {
    fn0 = %load_at_0_and_add(i64 vmctx) -> i64

    ;; This is our vmctx struct
    ss1 = explicit_slot 8

block0:
    ;; Store a 1 in the vmctx struct
    v1 = iconst.i64 1
    stack_store.i64 v1, ss1

    ;; Call %load_at_0_and_add with vmctx
    v2 = stack_addr.i64 ss1
    v3 = call fn0(v2)

    return v3
}
; run: %vm_state() == 2


function %load_at_0_and_add(i64 vmctx) -> i64 {
    gv0 = vmctx
    gv1 = load.i64 notrap aligned gv0+0

block0(v0: i64):
    v1 = global_value.i64 gv1
    v2 = iadd_imm.i64 v1, 1
    return v2
}

@fitzgen
Copy link
Member Author

fitzgen commented Jun 28, 2023

I was essentially trying to do something like this:

function %stack_limit(i64 vmctx, i64) -> i64 {
    gv0 = vmctx
    stack_limit = gv0

block0(v0: i64, v1: i64):
    return v1
}

; run(0, 42) == 42

@fitzgen
Copy link
Member Author

fitzgen commented Jun 28, 2023

I guess I could have got that working with another trampoline function to init the vmctx, that's a good trick. Still a bit of a workaround though compared to just passing in a value.

@afonso360
Copy link
Contributor

Oh, weird that it doesn't work when we call the vmctx function directly. That should be a somewhat easy fix, I don't mind working on it.

I think we don't support stack_limit in the interpreter yet though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cranelift Issues related to the Cranelift code generator
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants