[PATCH] D59352: [WebAssembly] Make rethrow take an except_ref type argument

[Heejin Ahn via Phabricator via llvm-commits]

aheejin added inline comments.


================
Comment at: lib/CodeGen/WasmEHPrepare.cpp:180
   // Insert an unreachable instruction after a call to @llvm.wasm.throw /
   // @llvm.wasm.rethrow and delete all following instructions within the BB, and
   // delete all the dead children of the BB as well.
----------------
dschuff wrote:
> Update the comment too.
Done.

> You've removed this test; should we also remove the code that it's testing?
This is the part where I removed the code for that. Note I deleted the line
```
for (auto L : {ThrowF->users(), RethrowF->users()}) {
```
and now the `User *U` only refers to `ThrowF->users()`.


================
Comment at: test/CodeGen/WebAssembly/cfg-stackify-eh.ll:36
 ; CHECK:   end_block                                   # label3:
-; CHECK:   call      __cxa_rethrow
+; CHECK:   rethrow   ${{.}}                            # to caller
 ; CHECK: end_try                                       # label0:
----------------
dschuff wrote:
> Here you are trying to match the `$` but below you are not. I guess the reason you are not checking the value (in catch or rethrow) is because these tests are only intended to verify the control flow?
Yes, I didn't try to check if the argument is correct in this file because this is for CFG ctackify pass checking. But I wasn't gonna make this different from other occurrences in this file by checking `$` here only; I will change this to `{{.*}} like others.


================
Comment at: test/CodeGen/WebAssembly/wasmehprepare.ll:399
-; CHECK-LABEL: @test6
-define i32 @test6(i1 %b, i8* %p) {
-entry:
----------------
dschuff wrote:
> You've removed this test; should we also remove the code that it's testing?
I've already done that; please see the comment above in WasmEHPrepare.cpp.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59352/new/

https://reviews.llvm.org/D59352