[WebAssembly] promote from experimental to normal target

[andrewrk]

Summary

what do we think? is it time?

[sunfish]

I think it's getting closer. IMO we should at least land https://reviews.llvm.org/D43147 and https://reviews.llvm.org/D40526 first, and address any follow-up issues.

There's also the question of testing and buildbots, though the way those requirements are worded in the DeveloperPolicy could be interpreted as not applying to wasm.

[hans]

Whenever it lands, please include an update to docs/ReleaseNotes.rst :-)

[sunfish]

We should probably also wait for the WebAssembly committee to settle on final instruction names.

[andrewrk]

Would llvm interact with grow_memory or memory_size instructions? Seems like that would be the frontend or standard library's job. It's not in LLVM's business to interact with the "OS" by allocating memory.

[sunfish]

Indeed; I don't anticipate LLVM will be using mem.grow or mem.size itself. But, clang has __builtin_wasm_mem_grow and __builtin_wasm_mem_size, to allow standard libraries to use them. It's desirable that the names of these builtins, and the corresponding LLVM IR intrinsics, match the names in the official WebAssembly documentation.

[andrewrk]

Ping. Is it time yet? Looks like the stated blockers are resolved now.

[andrewrk]

Ping

[sunfish]

Indeed; after discussing this with a few other folks, I think this is getting close.

I'm going to refresh https://reviews.llvm.org/D40526 in hopes of landing it.

[sunfish]

https://reviews.llvm.org/D40526 has now landed. Is there anything else we should do before leaving "experimental" status?

Is it important to resolve https://reviews.llvm.org/D43675 (Rename imported/exported memory symbol to __linear_memory) first?

[aardappel]

Would be nice to get this CL in before we switch out of experimental: https://reviews.llvm.org/D50568
Not only does this change the text syntax for assembler and disassembler, it changes the instruction format used in MC.
As such it is a larger, messy change that would be good to have behind us, such that future changes will likely change any of these less.
I had to revert it to fix (hopefully) one last thing, hopefully re-land it soonish.

[sunfish]

https://reviews.llvm.org/D50568 is now fixed!

This just leaves https://reviews.llvm.org/D43675. However, @echristo pointed out on llvm-dev that since LLVM just had a release, we have 6 months until they next one, which is likely enough time to resolve D43675 and get some general stabilizing in. And lots of people are eager for us to flip this switch. So let's do it!

I'll land the patch here as soon as I run a few more tests.

[sbc100]

Woah! This a big step people! Congratulations.

On a practical note, presumably these mean that a lot more LLVM bots will be building and test wasm, right? Are there many bots that build all default targets?

[sunfish]

In D43211#1241361, @sbc100 wrote:

Woah! This a big step people! Congratulations.

Indeed, we've come a long way!

On a practical note, presumably these mean that a lot more LLVM bots will be building and test wasm, right? Are there many bots that build all default targets?

There are a bunch, and notably, some of them run with a bunch of sanitizer configurations, and unfortunately, one of them found a memory leak. So for now I submitted r342707 to revert the change until we have a chance to fix it.

llvm/trunk/CMakeLists.txt

set(LLVM_ALL_TARGETS
   Hexagon
   Lanai
   Mips
   MSP430
   NVPTX
   PowerPC
   Sparc
   SystemZ
+  WebAssembly
   X86
   XCore
   )
 
 # List of targets with JIT support:
 set(LLVM_TARGETS_WITH_JIT X86 PowerPC AArch64 ARM Mips SystemZ)
 
 set(LLVM_TARGETS_TO_BUILD "all"