[PATCH] D96595: [WebAssemblly] Fix rethrow's argument computation

[Heejin Ahn via Phabricator via llvm-commits]

aheejin marked an inline comment as done.
aheejin added a comment.

In D96595#2560362 <https://reviews.llvm.org/D96595#2560362>, @dschuff wrote:

> In D96595#2559644 <https://reviews.llvm.org/D96595#2559644>, @aheejin wrote:
>
>> This replaces D96532 <https://reviews.llvm.org/D96532>. D96532 <https://reviews.llvm.org/D96532> didn't correctly handle this case:
>>
>>   try
>>     ...
>>   catch
>>     try
>>       rethrow 1 ;; (1)
>>     catch
>>       rethrow 0 ;; (2)
>>     end
>>   end
>
> OK, I think I get the indexing scheme now... I didn't think about 'try' having a label, so if (1) were 'rethrow 0' it would refer to the 'try' (and therefore be invalid)

Yes, if (1) were `rethrow 0`, it will refer to the inner `try`, so it is a validation failure.

I originally hadn't thought about `try` having a label as well, but as I wrote in https://github.com/WebAssembly/exception-handling/issues/146, it seems convenient and consistent to allow `delegate` have a label argument that is computed in the same way as branches, and `rethrow` seems no different from `delegate` if we look at the `rethrow` label rule written in https://github.com/WebAssembly/exception-handling/pull/137. I just copied that rule from the very first version of the spec and I myself was misunderstanding that rule, which I realized after reading Thibaud's comment in https://bugs.chromium.org/p/v8/issues/detail?id=11402.



================
Comment at: llvm/test/CodeGen/WebAssembly/exception.mir:108
+    %0:i32 = CATCH &__cpp_exception, implicit-def dead $arguments
+    RETHROW 0, implicit-def dead $arguments
+
----------------
dschuff wrote:
> this is "RETHROW 0" in the test input because it's just the placeholder from ISel, right?
Yes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96595