clang-tidy fix

Typo fixes

Update comment

Oh, forgot to fix comments... will fix soon

In D91722#2774622, @sbc100 wrote:

In D91722#2774612, @jmorse wrote:

In D91722#2774557, @sbc100 wrote:

After the third re-land of this patch we are seeing a crash in the emscripten compiler: https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket.appspot.com/8846604336499720256/+/steps/Emscripten_testsuite__upstream__other_/0/stdout

Thanks for the report -- we're out of working hours in the UK timezone and I can't revert myself, if you could revert this for us, please do.

We are investigating now. I looks like this assert only fires in combination with WebAssemlby change that was landed between the 2nd and 3rd re-landing: https://reviews.llvm.org/D102589. I think we will try to fix forward if we can and if that fails revert either or the other.

Remove unnecessary llvm::

Use make_early_inc_range()

Comment fix

Address comments

Address comments

This happened to uncover a long-existing bug in WebAssembly::getCalleeOp. For indirect calls it returns MI.getOperand(MI.getNumOperands() - 1), but in the presence of implicit operands, most of the time it returns an incorrect operand. For example:

%37:i32 = CALL_INDIRECT 0, 0, %55:i32, %38:i32, %54:i32, implicit-def dead $arguments, implicit $sp32, implicit $sp64, implicit-def dead $arguments, implicit $sp32, implicit $sp64, implicit-def $value_stack, implicit $value_stack, debug-location !39; system/lib/libc/musl/src/stdlib/bsearch.c:9:10

The callee operand is %54 here, but this returns implicit $value_stack. The should have used MI.getNumExplicitOperand() instead. This has not been a problem so far because this utility function has been used only from a few places and all of them only cared about direct calls.

Fix WebAssembly::getCalleeOp for indirect calls

Comment fix

New standalone test

In D102978#2777793, @dschuff wrote:

I think this makes sense.

my extra comment was that it seems likely that there are other places that currently use getOperand(0) that should now use this, and I was wondering if there were an easy way to find them since we can't just grep (especially since LLVM IR also has a getOperand). Thinking about that some more: Presumably uses in other backends would be safe to leave in place, and anything that affects codegen (at least in code that is covered at all) would be pretty broken, so maybe there actually aren't that many places outside of debug info (in other words broken debug info would be much more likely to go unnoticed than broken codegen).

More

Better comment

Add comment for DIArgList

Add BEFORE and AFTER tests

This also contains a revert of D102589.

In D102791#2769892, @dschuff wrote:

BTW Is there a way to disable this warning?

@jmorse Thanks for your help!

Improve comments and CHECK line

Comment

Use DPTR in tests

Fix incorrect comment pointed out here: https://github.com/emscripten-core/emscripten/pull/14108/files#r627479849

This is a second attempt after D101736, which was a hacky take to make MachineVerifier pass. It looks that was not necessary after all; thanks @jmorse.

@jmorse Thank you very much for the info! I didn't know DbgEntityHistoryCalculator terminates debug value ranges at the end of a BB. That solves everything without this hacky stuff. I uploaded a new patch in D102309.

@dblaikie I can remove this one. This is not an important test anyway. But where are we supposed to test the arguments clang driver invokes the backend LLVM compilation with? This was mainly to test if -exception-model=wasm reaches the backend compilation.

To improve hygiene, consistency, and usability, it would be good to replace all
the macro intrinsics in wasm_simd128.h with functions. The reason for using
macros in the first place was to enforce the use of constants for some arguments
using _Static_assert with __builtin_constant_p. This commit switches to
using functions and uses the __diagnose_if__ attribute rather than
_Static_assert to enforce constantness.

In D101812#2740599, @sbc100 wrote:

emitEndOfAsmFile

Ping

I see, thanks. Then other load instructions are OK not to be fixed?

• v128.load
• v128.loadN_splat
• v128.loadNxM_s/u

Why do only these instructions have const? It doesn't apply to other read-only arguments?

Thanks for the explanation!

Fix more comments

Sorry I'm not sure if I understand. The "expected alignment" for all those memory instructions is 1, regardless of the size of the element of the vectors? Can you elaborate on why the previous code is incorrect?

Revert another accidental change

Revert accidental test changes

In D101736#2733694, @aardappel wrote:

Yes, this makes sense to me. Another way to put it is that our br_if has the side effect of forcing termination of a "register" lifetime, which is uncommon.

Does LLVM not emit code for any other stack machines where this could happen? x87? :)