```suggestion ; Wasm, to generate valid code, always internally sets `--trap-unreachable` to 1 ; and `--no-trap-after-noreturn` to 0, and these command lines options, if ; explicitly given, are ignored. Various combinations of these options should ; have no effect and should not generate invalid code. ```

```suggestion ; "end_function" lines to prevent the tests from passing when there are ; superfluous instructions at the end of a function. You can run ; update_llc_test_checks.py again, but please keep the "end_function" lines ; intact when you commit. ``` Nit: 80 col wrapping

```suggestion ; LLVM IR's 'unreachable' is translated to Wasm 'unreachable'. ```

```suggestion ; This is similar to the above test, but the callee has a 'noreturn' attribute. ; There is an optimization that removes an 'unreachable' after a noreturn call, ; but Wasm backend doesn't use it and ignore `--no-trap-after-noreturn`, if ; given, to generate valid code. ```

I don't think tests below this line are really necessary. They are not related to the bug you are fixing.

We can be more direct by saying that there is a LLVM optimization that omits `unreachable` if that comes after a `noreturn` function, but because we always set `--no-trap-after-noreturn` internally to fix the validation problem, we don't use that optimization and end up emitting the `unreachable`.

As I said before, I think this comment is confusing. We don't emit `unreachable` to "fill in for the missing i32 return value". We emit `unreachable` because there is LLVM `unreachable`.

I think we should be more direct and simple by saying, whatever value you specify for `--trap-unreachable` and `--no-trap-after-noreturn`, they will be ignored and we will do `--trap-unreachable=1` and `--no-trap-after-noreturn=0`. (Doesn't have to be exactly this sentence but something to the same effect.) The current comment sounds like various combinations might produce different results. They don't.

```suggestion define void @void_sig_match_noreturn_unreachable() { ; CHECK-LABEL: void_sig_match_noreturn_unreachable: ; CHECK: .functype void_sig_match_noreturn_unreachable () -> () ```

```suggestion define i32 @i32_sig_match_unreachable() { ; CHECK-LABEL: i32_sig_match_unreachable: ; CHECK: .functype i32_sig_match_unreachable () -> (i32) ```

```suggestion define void @void_sig_match_unreachable() { ; CHECK-LABEL: void_sig_match_unreachable: ; CHECK: .functype void_sig_match_uanreachable () -> () ```

```suggestion define i32 @missing_ret_noreturn_unreachable() { ; CHECK-LABEL: missing_ret_noreturn_unreachable: ; CHECK: .functype missing_ret_noreturn_unreachable () -> (i32) ```

```suggestion define i32 @missing_ret_unreachable() { ; CHECK-LABEL: missing_ret_unreachable: ; CHECK: .functype missing_ret_unreachable () -> (i32) ``` As I said in https://github.com/llvm/llvm-project/pull/65876#discussion_r1339094353, there are remaining uses of `unreach`.. Will put suggestions.

That's because... there's no unreachable in the IR in the first place. Why do we need this test?

This confuses me. Wasm `unreachable` is not emitted to fill in for the missing `i32`. It is emitted because there's LLVM `unreachable` in the bitcode.

I think this file needs some more comments on what this file tries to test. It is hard to figure out what this file is trying to test. It looks all tests just produce Wasm `unreachable` for LLVM `unreachable` and nothing else, and it is hard to figure out why we need all these different tests and all these combinations of this options for, unless they want to go through all the discussions in this PR.

```suggestion define void @trap_unreacheable() { ; CHECK-LABEL: trap_unreacheable: ; CHECK: .functype trap_unreacheable () -> () ``` Nit: I think not shortening this is clearer. The same for other uses for `unreach` in this file.

```suggestion define void @debugtrap_ret_void() { ; CHECK-LABEL: debugtrap_ret_void: ; CHECK: .functype debugtrap_ret_void () -> () ``` Nit: Not shortening it sounds clearer to me

```suggestion ; LLVM trap followed by LLVM unreachable could become exactly one wasm ; unreachable, but two are emitted currently. ```

```suggestion ; Test that the LLVM trap and debug trap intrinsics are lowered to wasm ; unreachable. ``` Nit: It was this way before (in the original line 4-5) and did not exceed 80 cols. I think we don't need to change that.

I'm not sure if I understand what all the new tests in this file are testing. Currently this file emits the same results for all combination of `--trap-unreachable` and `--no-trap-after-noreturn`. Also without this PR the results are the same. Not sure what this PR is trying to achieve...

Why is this a bugfix? (You said this is an actual bugfix in https://github.com/llvm/llvm-project/pull/65876/files#r1322760404) This option is false by default and since we haven't overridden it, it has been false so far. What effect does this line have then?

I guess this comment is what you intended for the next PR. This test still contains two `unreachable`s

Please wrap the comments in the tests to 80 cols

Why do we need this flag? And why do we need to test wasm code with this flag, given that we set it to false?

Maybe this dead code removal change seems substantial enough as to warrant landing (and testing) separately?

Why is the lambda here necessary?