wasmtime / Issue #1918 Add config isa to c api · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / Issue #1918 Add config isa to c api

Wasmtime GitHub notifications bot (Jun 25 2020 at 08:06):

jlb6740 commented on Issue #1918:

Hi .. I wanted to expose selecting the target isa x86 or x64 for example, to the c-api. I'm looking for confirmation this makes sense to do and to see the best way to do this. My current thinking is to just go along the lines the code is going now and try to expose the selection through the cranelift_other_flags function. Currently looks like I'd have to add some flags or do something to target the target specific flags? There is currently the use_new_backend flag that looks target specific. Any comments?

Wasmtime GitHub notifications bot (Jun 25 2020 at 08:08):

jlb6740 edited a comment on Issue #1918:

Hi .. I wanted to expose selecting the target isa (x86 or x64 for example) to the c-api. I'm looking for confirmation this makes sense to do and to see the best way to do this. My current thinking is to just go along the lines the code is going now and try to expose the selection through the cranelift_other_flags function. Currently looks not sure the best way to do this sense there are no relevant flags here and maybe adding a new one would be required? There is the use_new_backend flag that looks target specific and which I need to make considerations for. Any comments?

Wasmtime GitHub notifications bot (Jun 25 2020 at 08:25):

github-actions[bot] commented on Issue #1918:

Subscribe to Label Action

cc @bnjbvr, @peterhuene

<details>
This issue or pull request has been labeled: "cranelift", "cranelift:area:x64", "wasmtime:c-api"

Thus the following users have been cc'd because of the following labels:

bnjbvr: cranelift

peterhuene: wasmtime:c-api

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

Wasmtime GitHub notifications bot (Jun 25 2020 at 09:37):

bjorn3 commented on Issue #1918:

I wanted to expose selecting the target isa (x86 or x64 for example) to the c-api.

Selecting a target isa doesn't make sense for a JIT. It must always match the host isa to prevent crashes. Even switching between the 32bit and 64bit version of an isa often isn't possible. (Linux has a way to switch between 64bit and 32bit x86 execution within a process, but I don't know of any other OS that supports this) If you want to switch between the old and the new x86_64 backend, then there is already the experimental_x64 feature flag. The new x86_64 backend is not complete enough to be usable at the moment though.

Wasmtime GitHub notifications bot (Jun 25 2020 at 17:43):

jlb6740 commented on Issue #1918:

Hi @bjorn3 @bnjbvr thanks for the comments. Yes, full disclosure I was experimentation in trying to use the C-API (instead of wasmtime cli) to test modules that contained newly supported wasm instructions against the new backend. At current, I did not see how to access the experimental_x64 flag and was forced to comment out code and rebuild the wasmtime lib everytime I wanted to compare results between the two backend and was thinking support could be added to the api like the second line here:

wasm_config_t *config = wasm_config_new();
wasmtime_config_arch_set(config, x64);

wasm_engine_t *engine = wasm_engine_new_with_config(config);
assert(engine != NULL);

Of course this wouldn't work if the arch wasn't supported by the platform but that was understood. So the goal wasn't anything like cross-compile. In general ... I was wondering if it made sense to add a way to access most of the same flags available to the cli.

Once enough instructions are lowered (which may be the case with @bnjbvr latest push, the experimenting that I was doing can be done with a script that calls the wasmtime cli but messing around with the C-API was helping me understand parts of the internals a little better so I went that route. I was thinking that separate engine, separate store, separate instance, etc that this was possible, but from @bjorn3 comments it sounds like it is not.

Wasmtime GitHub notifications bot (Jun 25 2020 at 17:44):

jlb6740 edited a comment on Issue #1918:

Hi @bjorn3 @bnjbvr thanks for the comments. Yes, full disclosure I was experimentation in trying to use the C-API (instead of wasmtime cli) to test modules that contained newly supported wasm instructions against the new backend. At current, I did not see how to access the experimental_x64 flag and was forced to comment out code and rebuild the wasmtime lib everytime I wanted to compare results between the two backend and was thinking support could be added to the api like the second line here:

wasm_config_t *config = wasm_config_new();
wasmtime_config_arch_set(config, x64);

wasm_engine_t *engine = wasm_engine_new_with_config(config);
assert(engine != NULL);

Of course this wouldn't work if the arch wasn't supported by the platform but that was understood. So the goal wasn't anything like cross-compile. In general ... I was wondering if it made sense to add a way to access via the capi, most of the same flags available to the cli using the cranelift_other_flags function.

Once enough instructions are lowered (which may be the case with @bnjbvr latest push, the experimenting that I was doing can be done with a script that calls the wasmtime cli but messing around with the C-API was helping me understand parts of the internals a little better so I went that route. I was thinking that separate engine, separate store, separate instance, etc that this was possible, but from @bjorn3 comments it sounds like it is not.

Wasmtime GitHub notifications bot (Jun 25 2020 at 18:44):

bjorn3 commented on Issue #1918:

You have to pass --features experimental_x64 to cargo build.

Wasmtime GitHub notifications bot (Jun 25 2020 at 18:50):

jlb6740 commented on Issue #1918:

You have to pass --features experimental_x64 to cargo build.

I do that now. It's not enough. The guard in codegen/src/isa/x86/mod.rs is what I have to comment out:
if isa_flags.use_new_backend() {
    #[cfg(not(feature = "x64"))]
    panic!("new backend x86 support not included by cargo features!");

    #[cfg(feature = "x64")]
    super::x64::isa_builder(triple).finish(shared_flags)
} else {
    Box::new(Isa {
        triple,
        isa_flags,
        shared_flags,
        cpumode: level1,
    })

Wasmtime GitHub notifications bot (Jun 25 2020 at 18:52):

jlb6740 edited a comment on Issue #1918:

Thanks @bjorn3

You have to pass --features experimental_x64 to cargo build.

I do that now. It's not enough. The guard in codegen/src/isa/x86/mod.rs is what I have to comment out:
if isa_flags.use_new_backend() {
    #[cfg(not(feature = "x64"))]
    panic!("new backend x86 support not included by cargo features!");

    #[cfg(feature = "x64")]
    super::x64::isa_builder(triple).finish(shared_flags)
} else {
    Box::new(Isa {
        triple,
        isa_flags,
        shared_flags,
        cpumode: level1,
    })

Wasmtime GitHub notifications bot (Jun 25 2020 at 21:03):

jlb6740 commented on Issue #1918:

FYI ... it's not clear to me when --features=experimental_x64 is needed because

[features]
default = ["disas", "wasm", "cranelift-codegen/all-arch"]

Is already set in cranelift/cargo.toml by default. Separately there is the set use_new_backend flag that can be passed to cranelift cli and I think this is the flag that I am trying to toggle (in a more generic way) with the wasmtime c-api.

Wasmtime GitHub notifications bot (Jun 25 2020 at 21:04):

jlb6740 edited a comment on Issue #1918:

Also ... it's not clear to me when --features=experimental_x64 is needed because

[features]
default = ["disas", "wasm", "cranelift-codegen/all-arch"]

Is already set in cranelift/cargo.toml by default. Separately there is the set use_new_backend flag that can be passed to cranelift cli and I think this is the flag that I am trying to toggle (in a more generic way) with the wasmtime c-api.

Wasmtime GitHub notifications bot (Jun 26 2020 at 09:30):

bjorn3 commented on Issue #1918:

I thought experimental_x64 caused use_new_backend to be set by default.

Wasmtime GitHub notifications bot (Jun 26 2020 at 09:41):

bnjbvr commented on Issue #1918:

Some clarification here:

clif-util compiles with support for all the architectures, and this what you see in cranelift/Cargo.toml, so this includes the support for building the x64 backend. Then, one needs to set the use_new_backend to dynamically request the new backend to be used. (This was mostly a workaround around the construction of the TargetIsas, which map 1:1 with the host target triple parsed by --target; we needed a way to distinguish between x86_64 in the old backend vs in the new backend)

wasmtime doesn't compile with support for all the architectures by default; by using --experimental_x64, you enable build support for the x64 new backend. But then the use_new_backend flag must be passed with --cranelift-other-flags.

Wasmtime GitHub notifications bot (Oct 06 2020 at 17:35):

jlb6740 commented on Issue #1918:

Closing this draft PR. @bnjbvr clarified somethings for me here but beyond that I believe API updates have made what was intended here obsolete.

Last updated: Nov 22 2024 at 16:03 UTC