Rename wasm32-wasi-threads target to wasm32-wasi-preview1-threads

[loganek]

Threading support is based on the wasi-threads proposal. It was agreed that the proposal will not be included in any of the WASI releases; instead, there's another proposal which adds spawning threads functionality to the WASM core. wasm32-wasi-threads is quite a nice name for the long-term solution, therefore I think it should be reserved for the thread-spawn proposal and use it once the proposal is ready. Given we'd like to support both wasi-threads and thread-spawn (at least for some time), I'm proposing adding another target specifically for wasi threads.

I used the wasm32-wasi-preview1-threads target as the proposal kind of builds on top of wsp1. It's also the same as the Rust target. I don't have strong opininion about the name and I'm open for suggestions; if there's a better name for that target, I'd be happy to update the PR.

I keep both wasm32-wasi-threads and wasm32-wasi-preview1-threads for backward/forward compatibility. I know that wasm32-wasi-threads was considered experimental and would probably be safe to delete, but at the same time I think keeping it around is very little effort and can be re-used for thread-spawn in the future. If anybody thinks it's confusing and not worth it, happy to remove it.

I also updated the thread-spawn import to be consistent with other wsp1 imports.

[jeffparsons]

Apologies in advance if this is the wrong venue for this question...

Would it be worth rolling the wasm32-wasi -> wasm32-wasi-preview1 rename into this, or better to wait until after this PR lands? Is the goal to preserve the old name for that as an alias for now — maybe that complicates it?

[loganek]

Would it be worth rolling the wasm32-wasi -> wasm32-wasi-preview1 rename into this, or better to wait until after this PR lands? Is the goal to preserve the old name for that as an alias for now — maybe that complicates it?

I know this is being planned for Rust, and given the ABI for preview 2 will be different than preview 1, I think it makes sense to apply the same for WASI libc. IRC @sunfishcode has already started some work towards supporting preview 2, so he might have different opinion on that though.

Having said that, I think this can be a follow-up PR if we choose to make that change.

[loganek]

@sbc100 / @abrown / @sunfishcode would you be able to review the PR? Many thanks

[sbc100]

I wasn't party the target renaming discussions but this seems like a change to me.

[sbc100]

I'll leave it @abrown to approve

[abrown]

@loganek, I am in favor for doing something like this. I've been at WasmCon (and preparing for WasmCon) and so has @sunfishcode so I haven't been able to look at this closely. I will take a closer look early next week!

[yamt]

it's better to avoid confusing triple parsing code further.
wasm32-wasi and wasm32-wasi-threads are already confusing.

[abrown]

LGTM, pending my question about the import names. Naturally I think we are all concerned with an explosion in the number of targets (@yamt) but I talked to @sunfishcode and he is OK with this, noting that it is easier to add and remove new targets than change existing ones.

[yamt]

i'm against the change.

• it's incompatible with the spec and implementations as @abrown pointed out
• the rename doesn't solve anything. it just introduces confusions.
• after all, it's unclear if wasm32-wasi-threads will be an appropriate triplet for the new thread proposal.

[loganek]

it's incompatible with the spec and implementations as @abrown pointed out

This is already addressed

the rename doesn't solve anything. it just introduces confusions.

We introduce the new target so people can use it as a stable target for preview1 with threading. I think it'd be reasonable to do the same for non-threaded target as well, but that's outside of the scope for this PR - we can discuss this separately.

after all, it's unclear if wasm32-wasi-threads will be an appropriate triplet for the new thread proposal.

That's why I considered deleting the wasm32-wasi-threads target, but eventually decided to keep it for backward compatibility - eventually, we probably should remove it, and re-introduce it (if needed) when core wasm threads proposal is at the implementation stage.

[yamt]

it's incompatible with the spec and implementations as @abrown pointed out

This is already addressed

the rename doesn't solve anything. it just introduces confusions.

We introduce the new target so people can use it as a stable target for preview1 with threading. I think it'd be reasonable to do the same for non-threaded target as well, but that's outside of the scope for this PR - we can discuss this separately.

there is no problem with using the current "wasm-wasi-threads" as a stable target. for preview1 with threading.

after all, it's unclear if wasm32-wasi-threads will be an appropriate triplet for the new thread proposal.

That's why I considered deleting the wasm32-wasi-threads target, but eventually decided to keep it for backward compatibility - eventually, we probably should remove it, and re-introduce it (if needed) when core wasm threads proposal is at the implementation stage.

sorry, i don't understand your logic.

as the new one will be an extension to core wasm, it might be more appropriate to change the "wasm32" part as memory64 does.

[loganek]

Based on the offline discussion we've had with @abrown and @yamt I'm resolving the ticket. We'll reopen the discussion once we have more info on preview2 support in wasi libc and there are any ABI incompatible changes.

[adamnovak]

Rust seems to have settled on wasm32-wasi-preview1-threads as the target name. rustup can't (can no longer?) install a wasm32-wasi-threads toolchain.

But when I build Rust code that depends on C code for the wasm32-wasi-preview1-threads Rust target, it passes along --target=wasm32-wasi-preview1-threads to my WASI SDK v20 Clang, which then I think builds for wasm32 but without any of the threading features, and at link time I get:

  = note: rust-lld: error: --shared-memory is disallowed by sqlite3.o because it was not compiled with 'atomics' or 'bulk-memory' features.

I'm not sure if Rust is able to pass anything other than its own name for the target to Clang. So the rust names might need to be followed for the libc/SDK to work in Rust builds, or some other workaround would need to be found.