wasmtime / PR #1517 Add ability to call CLIF functions wi... · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / PR #1517 Add ability to call CLIF functions wi...

Wasmtime GitHub notifications bot (Apr 15 2020 at 21:06):

abrown opened PR #1517 from run-clif to master:

This resolves the work started in https://github.com/bytecodealliance/cranelift/pull/1231 and https://github.com/bytecodealliance/wasmtime/pull/1436. Cranelift filetests currently have the ability to run CLIF functions with a signature like () -> b* and check that the result is true under the test run directive. This PR adds the ability to call functions with arbitrary arguments and non-boolean returns and either print the result or check against a list of expected results:

run commands look like ; run: %add(2, 2) == 4 or ; run: %add(2, 2) != 5 and verify that the executed CLIF function returns the expected value

print commands look like ; print: %add(2, 2) and print the result of the function to stdout

To make this work, this PR compiles a single Cranelift Function into a CompiledFunction using a SingleFunctionCompiler. Because we will not know the signature of the function until runtime, we use a Trampoline to place the values in the appropriate location for the calling convention; this should look a lot like what @alexcrichton is doing with VMTrampoline in wasmtime (see https://github.com/bytecodealliance/wasmtime/blob/3b7cb6ee64469470fcdd68e185abca8eb2a1b20a/crates/api/src/func.rs#L510-L526, https://github.com/bytecodealliance/wasmtime/blob/3b7cb6ee64469470fcdd68e185abca8eb2a1b20a/crates/jit/src/compiler.rs#L260). To avoid re-compiling Trampolines for the same function signatures, Trampolines are cached in the SingleFunctionCompiler.

Wasmtime GitHub notifications bot (Apr 15 2020 at 21:26):

abrown updated PR #1517 from run-clif to master:

This resolves the work started in https://github.com/bytecodealliance/cranelift/pull/1231 and https://github.com/bytecodealliance/wasmtime/pull/1436. Cranelift filetests currently have the ability to run CLIF functions with a signature like () -> b* and check that the result is true under the test run directive. This PR adds the ability to call functions with arbitrary arguments and non-boolean returns and either print the result or check against a list of expected results:

run commands look like ; run: %add(2, 2) == 4 or ; run: %add(2, 2) != 5 and verify that the executed CLIF function returns the expected value

print commands look like ; print: %add(2, 2) and print the result of the function to stdout

To make this work, this PR compiles a single Cranelift Function into a CompiledFunction using a SingleFunctionCompiler. Because we will not know the signature of the function until runtime, we use a Trampoline to place the values in the appropriate location for the calling convention; this should look a lot like what @alexcrichton is doing with VMTrampoline in wasmtime (see https://github.com/bytecodealliance/wasmtime/blob/3b7cb6ee64469470fcdd68e185abca8eb2a1b20a/crates/api/src/func.rs#L510-L526, https://github.com/bytecodealliance/wasmtime/blob/3b7cb6ee64469470fcdd68e185abca8eb2a1b20a/crates/jit/src/compiler.rs#L260). To avoid re-compiling Trampolines for the same function signatures, Trampolines are cached in the SingleFunctionCompiler.

Wasmtime GitHub notifications bot (Apr 15 2020 at 21:34):

abrown updated PR #1517 from run-clif to master:

This resolves the work started in https://github.com/bytecodealliance/cranelift/pull/1231 and https://github.com/bytecodealliance/wasmtime/pull/1436. Cranelift filetests currently have the ability to run CLIF functions with a signature like () -> b* and check that the result is true under the test run directive. This PR adds the ability to call functions with arbitrary arguments and non-boolean returns and either print the result or check against a list of expected results:

run commands look like ; run: %add(2, 2) == 4 or ; run: %add(2, 2) != 5 and verify that the executed CLIF function returns the expected value

print commands look like ; print: %add(2, 2) and print the result of the function to stdout

To make this work, this PR compiles a single Cranelift Function into a CompiledFunction using a SingleFunctionCompiler. Because we will not know the signature of the function until runtime, we use a Trampoline to place the values in the appropriate location for the calling convention; this should look a lot like what @alexcrichton is doing with VMTrampoline in wasmtime (see https://github.com/bytecodealliance/wasmtime/blob/3b7cb6ee64469470fcdd68e185abca8eb2a1b20a/crates/api/src/func.rs#L510-L526, https://github.com/bytecodealliance/wasmtime/blob/3b7cb6ee64469470fcdd68e185abca8eb2a1b20a/crates/jit/src/compiler.rs#L260). To avoid re-compiling Trampolines for the same function signatures, Trampolines are cached in the SingleFunctionCompiler.

Wasmtime GitHub notifications bot (Apr 18 2020 at 17:59):

abrown updated PR #1517 from run-clif to master:

This resolves the work started in https://github.com/bytecodealliance/cranelift/pull/1231 and https://github.com/bytecodealliance/wasmtime/pull/1436. Cranelift filetests currently have the ability to run CLIF functions with a signature like () -> b* and check that the result is true under the test run directive. This PR adds the ability to call functions with arbitrary arguments and non-boolean returns and either print the result or check against a list of expected results:

run commands look like ; run: %add(2, 2) == 4 or ; run: %add(2, 2) != 5 and verify that the executed CLIF function returns the expected value

print commands look like ; print: %add(2, 2) and print the result of the function to stdout

To make this work, this PR compiles a single Cranelift Function into a CompiledFunction using a SingleFunctionCompiler. Because we will not know the signature of the function until runtime, we use a Trampoline to place the values in the appropriate location for the calling convention; this should look a lot like what @alexcrichton is doing with VMTrampoline in wasmtime (see https://github.com/bytecodealliance/wasmtime/blob/3b7cb6ee64469470fcdd68e185abca8eb2a1b20a/crates/api/src/func.rs#L510-L526, https://github.com/bytecodealliance/wasmtime/blob/3b7cb6ee64469470fcdd68e185abca8eb2a1b20a/crates/jit/src/compiler.rs#L260). To avoid re-compiling Trampolines for the same function signatures, Trampolines are cached in the SingleFunctionCompiler.

Wasmtime GitHub notifications bot (Apr 18 2020 at 22:15):

abrown updated PR #1517 from run-clif to master:

This resolves the work started in https://github.com/bytecodealliance/cranelift/pull/1231 and https://github.com/bytecodealliance/wasmtime/pull/1436. Cranelift filetests currently have the ability to run CLIF functions with a signature like () -> b* and check that the result is true under the test run directive. This PR adds the ability to call functions with arbitrary arguments and non-boolean returns and either print the result or check against a list of expected results:

run commands look like ; run: %add(2, 2) == 4 or ; run: %add(2, 2) != 5 and verify that the executed CLIF function returns the expected value

print commands look like ; print: %add(2, 2) and print the result of the function to stdout

To make this work, this PR compiles a single Cranelift Function into a CompiledFunction using a SingleFunctionCompiler. Because we will not know the signature of the function until runtime, we use a Trampoline to place the values in the appropriate location for the calling convention; this should look a lot like what @alexcrichton is doing with VMTrampoline in wasmtime (see https://github.com/bytecodealliance/wasmtime/blob/3b7cb6ee64469470fcdd68e185abca8eb2a1b20a/crates/api/src/func.rs#L510-L526, https://github.com/bytecodealliance/wasmtime/blob/3b7cb6ee64469470fcdd68e185abca8eb2a1b20a/crates/jit/src/compiler.rs#L260). To avoid re-compiling Trampolines for the same function signatures, Trampolines are cached in the SingleFunctionCompiler.

Wasmtime GitHub notifications bot (Apr 18 2020 at 22:16):

abrown requested bnjbvr for a review on PR #1517.

Wasmtime GitHub notifications bot (Apr 18 2020 at 22:33):

abrown updated PR #1517 from run-clif to master:

This resolves the work started in https://github.com/bytecodealliance/cranelift/pull/1231 and https://github.com/bytecodealliance/wasmtime/pull/1436. Cranelift filetests currently have the ability to run CLIF functions with a signature like () -> b* and check that the result is true under the test run directive. This PR adds the ability to call functions with arbitrary arguments and non-boolean returns and either print the result or check against a list of expected results:

run commands look like ; run: %add(2, 2) == 4 or ; run: %add(2, 2) != 5 and verify that the executed CLIF function returns the expected value

print commands look like ; print: %add(2, 2) and print the result of the function to stdout

To make this work, this PR compiles a single Cranelift Function into a CompiledFunction using a SingleFunctionCompiler. Because we will not know the signature of the function until runtime, we use a Trampoline to place the values in the appropriate location for the calling convention; this should look a lot like what @alexcrichton is doing with VMTrampoline in wasmtime (see https://github.com/bytecodealliance/wasmtime/blob/3b7cb6ee64469470fcdd68e185abca8eb2a1b20a/crates/api/src/func.rs#L510-L526, https://github.com/bytecodealliance/wasmtime/blob/3b7cb6ee64469470fcdd68e185abca8eb2a1b20a/crates/jit/src/compiler.rs#L260). To avoid re-compiling Trampolines for the same function signatures, Trampolines are cached in the SingleFunctionCompiler.

Wasmtime GitHub notifications bot (Apr 18 2020 at 22:35):

abrown updated PR #1517 from run-clif to master:

This resolves the work started in https://github.com/bytecodealliance/cranelift/pull/1231 and https://github.com/bytecodealliance/wasmtime/pull/1436. Cranelift filetests currently have the ability to run CLIF functions with a signature like () -> b* and check that the result is true under the test run directive. This PR adds the ability to call functions with arbitrary arguments and non-boolean returns and either print the result or check against a list of expected results:

run commands look like ; run: %add(2, 2) == 4 or ; run: %add(2, 2) != 5 and verify that the executed CLIF function returns the expected value

print commands look like ; print: %add(2, 2) and print the result of the function to stdout

To make this work, this PR compiles a single Cranelift Function into a CompiledFunction using a SingleFunctionCompiler. Because we will not know the signature of the function until runtime, we use a Trampoline to place the values in the appropriate location for the calling convention; this should look a lot like what @alexcrichton is doing with VMTrampoline in wasmtime (see https://github.com/bytecodealliance/wasmtime/blob/3b7cb6ee64469470fcdd68e185abca8eb2a1b20a/crates/api/src/func.rs#L510-L526, https://github.com/bytecodealliance/wasmtime/blob/3b7cb6ee64469470fcdd68e185abca8eb2a1b20a/crates/jit/src/compiler.rs#L260). To avoid re-compiling Trampolines for the same function signatures, Trampolines are cached in the SingleFunctionCompiler.

Wasmtime GitHub notifications bot (Apr 22 2020 at 17:20):

abrown updated PR #1517 from run-clif to master:

This resolves the work started in https://github.com/bytecodealliance/cranelift/pull/1231 and https://github.com/bytecodealliance/wasmtime/pull/1436. Cranelift filetests currently have the ability to run CLIF functions with a signature like () -> b* and check that the result is true under the test run directive. This PR adds the ability to call functions with arbitrary arguments and non-boolean returns and either print the result or check against a list of expected results:

run commands look like ; run: %add(2, 2) == 4 or ; run: %add(2, 2) != 5 and verify that the executed CLIF function returns the expected value

print commands look like ; print: %add(2, 2) and print the result of the function to stdout

To make this work, this PR compiles a single Cranelift Function into a CompiledFunction using a SingleFunctionCompiler. Because we will not know the signature of the function until runtime, we use a Trampoline to place the values in the appropriate location for the calling convention; this should look a lot like what @alexcrichton is doing with VMTrampoline in wasmtime (see https://github.com/bytecodealliance/wasmtime/blob/3b7cb6ee64469470fcdd68e185abca8eb2a1b20a/crates/api/src/func.rs#L510-L526, https://github.com/bytecodealliance/wasmtime/blob/3b7cb6ee64469470fcdd68e185abca8eb2a1b20a/crates/jit/src/compiler.rs#L260). To avoid re-compiling Trampolines for the same function signatures, Trampolines are cached in the SingleFunctionCompiler.

Wasmtime GitHub notifications bot (Apr 22 2020 at 18:04):

abrown updated PR #1517 from run-clif to master:

This resolves the work started in https://github.com/bytecodealliance/cranelift/pull/1231 and https://github.com/bytecodealliance/wasmtime/pull/1436. Cranelift filetests currently have the ability to run CLIF functions with a signature like () -> b* and check that the result is true under the test run directive. This PR adds the ability to call functions with arbitrary arguments and non-boolean returns and either print the result or check against a list of expected results:

run commands look like ; run: %add(2, 2) == 4 or ; run: %add(2, 2) != 5 and verify that the executed CLIF function returns the expected value

print commands look like ; print: %add(2, 2) and print the result of the function to stdout

To make this work, this PR compiles a single Cranelift Function into a CompiledFunction using a SingleFunctionCompiler. Because we will not know the signature of the function until runtime, we use a Trampoline to place the values in the appropriate location for the calling convention; this should look a lot like what @alexcrichton is doing with VMTrampoline in wasmtime (see https://github.com/bytecodealliance/wasmtime/blob/3b7cb6ee64469470fcdd68e185abca8eb2a1b20a/crates/api/src/func.rs#L510-L526, https://github.com/bytecodealliance/wasmtime/blob/3b7cb6ee64469470fcdd68e185abca8eb2a1b20a/crates/jit/src/compiler.rs#L260). To avoid re-compiling Trampolines for the same function signatures, Trampolines are cached in the SingleFunctionCompiler.

Wasmtime GitHub notifications bot (Apr 24 2020 at 17:23):

bnjbvr submitted PR Review.

Wasmtime GitHub notifications bot (Apr 24 2020 at 17:23):

bnjbvr submitted PR Review.

Wasmtime GitHub notifications bot (Apr 24 2020 at 17:23):

bnjbvr created PR Review Comment:

I think we don't need to worry about the clone, this is an error path, so not perf sensitive. Could it be e.to_string() to avoid the macro use?

Wasmtime GitHub notifications bot (Apr 24 2020 at 17:23):

bnjbvr created PR Review Comment:

Instead, could this look if trimmed is exactly run or print? I don't see the value in the ambiguity. If you agree with this, then we could rename this function is_run_or_print_command (since otherwise, there's a "run_command" called "run" and another called "print", which is a bit confusing).

In addition to this, could this return the command it found (so an Option, with None equivalent to current's false), instead of letting the caller clean up comments again and analyze the value?

Wasmtime GitHub notifications bot (Apr 24 2020 at 17:23):

bnjbvr created PR Review Comment:

Do we need to trim comments again here? There can't be a comment within a comment, so this would only removed spaces at the start of comment.text, right? If so, and if we want this, could we do it explicitly with trim_start_matches(" ") instead?

Wasmtime GitHub notifications bot (Apr 24 2020 at 17:23):

bnjbvr created PR Review Comment:

No need to worry about cloning here, it's not a perf-sensitive path. Can you use e.to_string() though?

Wasmtime GitHub notifications bot (Apr 24 2020 at 17:23):

bnjbvr created PR Review Comment:

Could we just do a write here, using a transmute? It avoids the indirection through Trampoline::write_value_to, which is a bit unsettling.

If we did the same for reads, then the need for the Trampoline data structure is much lower, and could probably be avoided. Or we could hide the writing and reading into a different new UnboxedValues struct type (that'd be a vec<u128>); then the slot_size could be an attribute of this struct, a const func returning the size of the values it's storing (and then we can remove a bunch of comments, and have more confidence in the size being sync'd with the storage type). What do you think?

Wasmtime GitHub notifications bot (Apr 24 2020 at 17:23):

bnjbvr created PR Review Comment:

Could it also check that the host architecture is equal to the requested ISA's architecture?

Wasmtime GitHub notifications bot (Apr 24 2020 at 17:23):

bnjbvr created PR Review Comment:

Could we clone the function's signature before, since it's the only field that's used after (and it's cheap), and then pass the ownership to compile?

Wasmtime GitHub notifications bot (Apr 27 2020 at 18:46):

abrown submitted PR Review.

Wasmtime GitHub notifications bot (Apr 27 2020 at 18:46):

abrown created PR Review Comment:

Those checks should already exist at a higher level, e.g. test_run.rs, and I didn't want to limit this code too much because there are cases (x86 32-bit vs 64-bit) where multiple Cranelift ISAs could run on the same machine.

Wasmtime GitHub notifications bot (Apr 27 2020 at 18:47):

abrown submitted PR Review.

Wasmtime GitHub notifications bot (Apr 27 2020 at 18:47):

abrown created PR Review Comment:

Good idea... done.

Wasmtime GitHub notifications bot (Apr 27 2020 at 18:51):

abrown updated PR #1517 from run-clif to master:

This resolves the work started in https://github.com/bytecodealliance/cranelift/pull/1231 and https://github.com/bytecodealliance/wasmtime/pull/1436. Cranelift filetests currently have the ability to run CLIF functions with a signature like () -> b* and check that the result is true under the test run directive. This PR adds the ability to call functions with arbitrary arguments and non-boolean returns and either print the result or check against a list of expected results:

run commands look like ; run: %add(2, 2) == 4 or ; run: %add(2, 2) != 5 and verify that the executed CLIF function returns the expected value

print commands look like ; print: %add(2, 2) and print the result of the function to stdout

To make this work, this PR compiles a single Cranelift Function into a CompiledFunction using a SingleFunctionCompiler. Because we will not know the signature of the function until runtime, we use a Trampoline to place the values in the appropriate location for the calling convention; this should look a lot like what @alexcrichton is doing with VMTrampoline in wasmtime (see https://github.com/bytecodealliance/wasmtime/blob/3b7cb6ee64469470fcdd68e185abca8eb2a1b20a/crates/api/src/func.rs#L510-L526, https://github.com/bytecodealliance/wasmtime/blob/3b7cb6ee64469470fcdd68e185abca8eb2a1b20a/crates/jit/src/compiler.rs#L260). To avoid re-compiling Trampolines for the same function signatures, Trampolines are cached in the SingleFunctionCompiler.

Wasmtime GitHub notifications bot (Apr 27 2020 at 18:55):

abrown submitted PR Review.

Wasmtime GitHub notifications bot (Apr 27 2020 at 18:55):

abrown created PR Review Comment:

Yeah, this is a good direction. These weird trimmed_comment_chars and is_potential_run_command functions were trying to do some lookahead to see if we actually needed to really parse the comment. I moved that logic into parse_run_command (which is in the parser module, where this functionality should be) and made it return ParseResult<Option<RunCommand>> instead of ParseResult<RunCommand>. The signature is a bit more complex but I think it is headed more in the direction you are suggesting.

Wasmtime GitHub notifications bot (Apr 27 2020 at 19:07):

bjorn3 submitted PR Review.

Wasmtime GitHub notifications bot (Apr 27 2020 at 19:07):

bjorn3 created PR Review Comment:

32bit x86 doesn't work in 64bit processes.

Wasmtime GitHub notifications bot (Apr 27 2020 at 19:22):

abrown submitted PR Review.

Wasmtime GitHub notifications bot (Apr 27 2020 at 19:22):

abrown created PR Review Comment:

I like having the Trampoline structure around to know what we are talking about; otherwise we have an Mmap floating around that is implicitly a trampoline. But that could be a type alias, I guess. I think your UnboxedValues makes sense and I started down this road but I'm not too confident about the transmute stuff--I will ping you on Zulip to see what you think.

Wasmtime GitHub notifications bot (Apr 29 2020 at 00:29):

abrown updated PR #1517 from run-clif to master:

This resolves the work started in https://github.com/bytecodealliance/cranelift/pull/1231 and https://github.com/bytecodealliance/wasmtime/pull/1436. Cranelift filetests currently have the ability to run CLIF functions with a signature like () -> b* and check that the result is true under the test run directive. This PR adds the ability to call functions with arbitrary arguments and non-boolean returns and either print the result or check against a list of expected results:

run commands look like ; run: %add(2, 2) == 4 or ; run: %add(2, 2) != 5 and verify that the executed CLIF function returns the expected value

print commands look like ; print: %add(2, 2) and print the result of the function to stdout

To make this work, this PR compiles a single Cranelift Function into a CompiledFunction using a SingleFunctionCompiler. Because we will not know the signature of the function until runtime, we use a Trampoline to place the values in the appropriate location for the calling convention; this should look a lot like what @alexcrichton is doing with VMTrampoline in wasmtime (see https://github.com/bytecodealliance/wasmtime/blob/3b7cb6ee64469470fcdd68e185abca8eb2a1b20a/crates/api/src/func.rs#L510-L526, https://github.com/bytecodealliance/wasmtime/blob/3b7cb6ee64469470fcdd68e185abca8eb2a1b20a/crates/jit/src/compiler.rs#L260). To avoid re-compiling Trampolines for the same function signatures, Trampolines are cached in the SingleFunctionCompiler.

Wasmtime GitHub notifications bot (Apr 29 2020 at 14:52):

bnjbvr requested bnjbvr for a review on PR #1517.

Wasmtime GitHub notifications bot (Apr 30 2020 at 14:58):

bnjbvr submitted PR Review.

Wasmtime GitHub notifications bot (Apr 30 2020 at 14:58):

bnjbvr submitted PR Review.

Wasmtime GitHub notifications bot (Apr 30 2020 at 14:58):

bnjbvr created PR Review Comment:

nit: can we put the unsafe within the push, to make it clear that the push itself isn't an issue?

Wasmtime GitHub notifications bot (Apr 30 2020 at 14:58):

bnjbvr created PR Review Comment:

This will manifest as a panic when running a test, right? Is there a way to propagate this as an error to the test runner, with the right file location, instead?

Wasmtime GitHub notifications bot (Apr 30 2020 at 17:48):

abrown created PR Review Comment:

Parser::parse_data_value actually does that type checking before we ever get here; I'm going to change this to an assert! since that more clearly indicates the intent of this check.

Wasmtime GitHub notifications bot (Apr 30 2020 at 17:48):

abrown submitted PR Review.

Wasmtime GitHub notifications bot (Apr 30 2020 at 17:51):

abrown updated PR #1517 from run-clif to master:

This resolves the work started in https://github.com/bytecodealliance/cranelift/pull/1231 and https://github.com/bytecodealliance/wasmtime/pull/1436. Cranelift filetests currently have the ability to run CLIF functions with a signature like () -> b* and check that the result is true under the test run directive. This PR adds the ability to call functions with arbitrary arguments and non-boolean returns and either print the result or check against a list of expected results:

run commands look like ; run: %add(2, 2) == 4 or ; run: %add(2, 2) != 5 and verify that the executed CLIF function returns the expected value

print commands look like ; print: %add(2, 2) and print the result of the function to stdout

To make this work, this PR compiles a single Cranelift Function into a CompiledFunction using a SingleFunctionCompiler. Because we will not know the signature of the function until runtime, we use a Trampoline to place the values in the appropriate location for the calling convention; this should look a lot like what @alexcrichton is doing with VMTrampoline in wasmtime (see https://github.com/bytecodealliance/wasmtime/blob/3b7cb6ee64469470fcdd68e185abca8eb2a1b20a/crates/api/src/func.rs#L510-L526, https://github.com/bytecodealliance/wasmtime/blob/3b7cb6ee64469470fcdd68e185abca8eb2a1b20a/crates/jit/src/compiler.rs#L260). To avoid re-compiling Trampolines for the same function signatures, Trampolines are cached in the SingleFunctionCompiler.

Wasmtime GitHub notifications bot (Apr 30 2020 at 18:21):

abrown merged PR #1517.

Last updated: Nov 22 2024 at 16:03 UTC