Skip to content

fix: handle all values for buffers in turbocall codegen #28170

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

Merged
merged 1 commit into from
Feb 18, 2025

Conversation

devsnek
Copy link
Member

@devsnek devsnek commented Feb 18, 2025

Now that ArrayBuffer/ArrayBufferView is a generic Value type, we have to handle it being passed any value. To do this, thread FastApiCallbackOptions through the function, and add error raising logic.

If we run conversion and the value is not valid, we return isize::MAX, and then in cranelift we use this value to know that we should branch to the error logic.

An example compilation looks like this:

extern "C" fn print_buffer(ptr: *const u8, len: usize);
function %print_buffer_wrapper(i64, i64, i64, i64) system_v {
    sig0 = (i64, i64) system_v
    sig1 = (i64) -> i64 system_v
    sig2 = (i64) system_v

block0(v0: i64, v1: i64, v2: i64, v3: i64):
    v4 = iconst.i64 0x6525_9198_2d00 ; turbocall_ab_contents
    v5 = call_indirect sig1, v4(v1)
    v6 = iconst.i64 0x7fff_ffff_ffff_ffff
    v7 = icmp eq v5, v6
    brif v7, block1, block2

block2:
    v8 = iconst.i64 0x7558_4c0c_0700 ; sym.ptr
    call_indirect sig0, v8(v5, v2)
    return

block1 cold:
    v9 = iconst.i64 0x6525_9198_2d70 ; turbocall_raise
    call_indirect sig2, v9(v3)
    return
}

Also cleaned up all the unwraps and added some logging.

@devsnek devsnek requested a review from littledivy February 18, 2025 15:49
Copy link
Member

@littledivy littledivy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Not for this PR but it would be nice to have codegen tests like we did before, maybe just the IR.

@devsnek devsnek enabled auto-merge (squash) February 18, 2025 15:55
@devsnek devsnek merged commit 6206343 into main Feb 18, 2025
17 checks passed
@devsnek devsnek deleted the x/fix-turbo-codegen branch February 18, 2025 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants