otherwise LGTM!

Otherwise LGTM

LGTM too. I realized I don't really like (or perhaps I just don't understand) the term "Attribute" where it's used in the spec, but it's fine for this code to match that.

Conceptually, COMPILER_RT_ABI is the calling convention used by all compiler-rt functions. It is explicitly used everywhere because it may be different from the default calling convention on some targets. In practice I believe it only affects ARM, where the calling convention uses ARM's soft-float ABI (where IIRC floats are passed in integer registers) even on targets that use the hard-float ABI by default. The compiler knows the calling conventions and emits the right code when it emits the libcalls, and the ABI declaration on the implementations causes the right calling convention to be used there. The unusual thing about the tests is that they locally declare the compiler-rt functions explicitly. So this declaration has to match the implementation. It may not matter for PPC but my recommendation would be "just use it everywhere in compiler-rt" for simplicity.

otherwise lgtm

Object files currently import their memories, right? We should probably make that import shared when we use thread-model=posix.

I like the idea of trying to make the assembler less stateful if possible, but that can be another CL

By "Multiple catches for a try" you mean catches with different tags?

At some point I could imagine a frontend possibly wanting to opt out of this pass entirely, but we can always add that option in the future.

I think assertions here are OK as long as the plan of record is is to put error handling into the parsers. If we have assertions, we can't have a test case (until we can test the parser errors) because it would always fail on release builds without asserts.

I think catching block mismatch errors at parse time makes sense; although the third way that MCInst can be generated (aside from the compiler and asmparser) is the object file parser. So those checks would have to go both in the asm parser and object file parser. But if that means that we can hook into some existing error handling mechanism instead of using report_fatal_error, then it's probably worth it.
I agree that we can't use an assert string in a CHECK. The test wouldn't pass in a release non-asserts build (our release binaries have assertions, but it's not the default, and the official release binaries don't).

This kind of code should never be produced by the wasm backend, right? In which case this could be an assert/unreachable rather than a fatal error. But I guess this code also prints insts that are parsed from an obj or asm file, right? In that case the input could just be bogus and we shouldn't assert.
So I'm not sure if it's possible here to tell which one to ideally use.
But also:

1. can this test be an llvm-mc test (which has assembly input and prints assembly)? I think the answer is "yes" (at least eventually) but I'm not sure if the asm parser is far enough along yet for that.
2. I wonder if there's some kind of infrastructure in MC for printing errors (e.g. for malformed asm) other than just the big hammer of report_fatal_error? If so it probably makes sense to use that (even if we can't tell the difference between bogus code from the compiler and bogus input from an asm user).

This was landed in rL343733. not sure why it didn't close already.

Is that information what's used/needed to allow the linker to create a separate wasm segment per global?

Test looks much nicer now, LGTM

Right but the point is that we don't currently have a way to ensure that the code is doing its job; if the SLP vectorizer stops doing its thing and calling the code that was asserting, then this test won't be testing anything.

W is int64_t (which is long long for wasm32 and would probably be long for wasm64 since that would probably LP64 (like Linux) rather than LLP64 (Like Win64). So if we want it to be the same for both wasm32 and wasm64, I guess we want LL.

Otherwise LGTM, and I verified that it fixes the expensive checks failure.

Yes, I can reproduce. I'll take a look.

@sbc100 or @sunfish, any more comments?

I think it could be clearer than it is. The problem is that there are 2 overlapping naming conventions in use simultaneously:

• SizeFoo referring to the size of a foo object or objects, (SizeAction, SizeActions)
• BarAction referring to the index of a particular action (FirstAction, PrevAction).

Both of those are unsigned but mean very different things. So it's good that there's a convention but SizeAction fits into both and is ambiguous.
I think renaming SizeAction to SizeActionEntry would fix that. SizeActions is probably ok. I think a brief comment explaining the usage is fine but putting it inline with the declaration minimizes risk of it getting outdated.

• clang-format

PTAL

• Merge branch 'master' into signatures
• add assert
• Don't compute signature in EmitEndOfAsmFile if one is already there
• Handle SRet in ComputeSignatureVTs
• Fix -Wcovered-switch-default
• Use ComputeSignatureVTs in more places

• Comments

I think it makes the most sense to try to remove colons from instruction names in the spec. Only a few shipped instructions have them. Slashes is probably harder, there are a lot more of those and they are all from the original MVP and not from nontrappingFP. So we can probably just remove colon support from this CL and land it.

• Make argument order consistent

• cleanup
• Use WasmSignature directly in DenseMap

Yeah sorry this still needs some cleanup before it's ready for review (also I need to post the lld side) . arc's --plan-changes flag claims to upload without requesting review (and it does) but unfortunately it doesn't suppress the herald rules :/

Thanks!

Not exactly sure what you mean; do you mean that hasOneUse should return true if there is only one non-debug use? That makes sense, maybe it should be hasOneNonDbgUse or something. But please either commit this fix and then follow up, or revert this patch and re-apply with hasOneUse fixed.

otherwise LGTM

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?