I like the idea of a consistent naming scheme with double-underscores for all of those special symbols. I also don't see any reason emscripten shouldn't use those names too. If it's easier, we could just prepare both an emscripten and an LLVM change and land them more-or-less at the same time and avoid a 3-stage process.

Is this enough to add SIMD to one of our existing binary-format tests?

Are there tests that trigger these? It might make sense to add a case at least to the explicit locals and reg stackify. Probably fast-isel support should be a separate change (you can take the existing SIMD tests and run the same test, forcing fast-isel as some of the other tests do)

Yeah, rL341580 looks almost identical to this.

Passing it as a parameter does sound better.

• Pass the binary name as a parameter instead

LGTM to ensure we are using the stack instructions.
It's unfortunate about the type index requirement. That will make it quite a pain to hand-write assembly files. It would be nice if we could eventually figure out a way to symbolize those like the wat format.

I took another look at the diff, and it does seem like we are fairly close already. I agree that some of the changes to the switch statements aren't great, but on balance it's probably worthwhile to be able to just run clang-format and accept whatever it generates. I think we can go ahead and land this.

If we want to make concerted progress toward full clang-formattedness (I like the idea in general, but opinions sometimes differ on the value of that) we could do it one file at a time, to be landed before making some change to that file. That would be a sort of compromise; it would keep the changes small and manageable, but it would be a larger granularity and scope than what you'd strictly need if you were going to make a small change or just work on one component (which would mean faster progress).

otherwise LGTM

@aheejin the main thing is that the instructions are now converted from register to stack in MCInstLower. @aardappel were you also planning on moving ExplicitLocals to the end as before?

Otherwise LGTM, although I'm not an expert in DebugInfo either.

This should probably have some tests. It looks like there are some existing x32 tests in test/Driver/linux-header-search.cpp and test/Driver/cross-linux.c that might be relevant for updating.

This looks OK to me; I can't think of any good alternative to adding a new predicate. Fixing PEI directly would require having it check for particular instructions (wasm catchret), which we can't really do.

Oops, retroactive LGTM

also please rebase against master.

Was the problem just that the PEI was inserting an epilog for every instruction that isReturn?

Keeping MUL is fine for now. IIRC there was some discussion in the last CG in-person meeting about removing or changing it (e.g. probably not super-useful especially for small types due to the likelihood of overflow). Probably in cases where the spec is still in flux it makes sense for LLVM to match what V8 implements now, so we can test things end-to-end. Then when we settle on things we can just change LLVM (or do LLVM first to see whether a proposal is feasible).

Great that the atomicexpandpass just works for this.

LGTM With those changes.

Still looks good 👍

I would imagine that's already on Ben's plate to do sometime, but yes it should be changed to match JS.

I like this approach too, assuming it's sufficient to handle the CMake (and autoconf?) ugliness. I am a bit concerned that this pass will become a large and difficult-to-maintain pile of heuristics with different purposes, so I think we should be as clear as possible about what use cases we are trying to handle.
Other than the comment nit, the code looks fine too.

Note that in JS, atomics.wake is now atomics.notify (https://github.com/tc39/ecma262/pull/1220). So we should use the new terminology for new and internal stuff. Since there's no legacy yet (except maybe in Binaryen's and WABT's text parser) hopefully LLVM can just use "notify" everywhere.

\o/

I think I actually prefer the approach of linking this code in from a library rather than generating N copies of it. It's more consistent with how system code is usually included into programs (i.e. linking it in rather than generating it magically from the compiler). It's also easier to read and understand for non-compiler developers.

@jfb just as a sanity check, it should always be valid to just "upgrade" LLVM's orderings and select them all as the sequentially consistent wasm ops, right?

SGTM

There should also be a few tests that orderings other than seq_cst get selected.

This change is really quite nice and minimal other than the renaming.

Otherwise LGTM

A nit about the commit message: it's not just LTO passes that make assumptions about known library functions, lots of passes do. Also I might be just a little bit more explicit than "when statically linking libc" and make it "when including libc in LTO"
Otherwise LGTM

OK, the pass looks good; don't know if @majnemer has any more feedback on this or D44134?

Looks pretty reasonable otherwise. I like the MIR unittest 👌

LGTM with the tweak to the comment.