Skip to content

[WebAssembly] Rename CATCH/CATCH_ALL to *_LEGACY #107187

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Sep 4, 2024
Merged

Conversation

aheejin
Copy link
Member

@aheejin aheejin commented Sep 4, 2024

This renames MIR instruction CATCH and CATCH_ALL to CATCH_LEGACY and CATCH_ALL_LEGACY respectively.

Follow-up PRs for the new EH (exnref) implementation will use CATCH, CATCH_REF, CATCH_ALL, and CATCH_ALL_REF as pseudo-instructions that return extracted values or exnref or both, because we don't currently support block return values in LLVM. So to give the old (real) CATCHes and the new (pseudo) CATCHes different names, this attaches _LEGACY prefix to the old names.

This also rearranges WebAssemblyInstrControl.td so that the old legacy instructions are listed all together at the end.

This renames MIR instruction `CATCH` and `CATCH_ALL` to `CATCH_P3` and
`CATCH_ALL_P3` respectively. `P3` means Phase 3.

Follow-up PRs for the new EH (exnref) implementation will use `CATCH`,
`CATCH_REF`, `CATCH_ALL`, and `CATCH_ALL_REF` as pseudo-instructions
that return extracted values or `exnref` or both, because we don't
currently support block return values in LLVM. So to give the old
(real) `CATCH`es and the new (pseudo) `CATCH`es different names, this
attaches `_P3` prefix to the names. This is what we did in Binaryen, but
in this case they were `enum` values:
https://github.com/WebAssembly/binaryen/blob/952286421a19fb8358d645f49d455b75bfbd1d19/src/wasm-binary.h#L1111-L1112

This also rearranges `WebAssemblyInstrControl.td` so that the old Phase
3 instructions are listed all together at the end.
@dschuff
Copy link
Member

dschuff commented Sep 4, 2024

I wonder if "CATCH_LEGACY" is a better name, since we settled on "legacy exception handling" as the name in the spec?

@aheejin aheejin changed the title [WebAssembly] Rename CATCH/CATCH_ALL to *_P3 [WebAssembly] Rename CATCH/CATCH_ALL to *_LEGACY Sep 4, 2024
@aheejin
Copy link
Member Author

aheejin commented Sep 4, 2024

Yeah that sounds better. Changed the code to _LEGACY.

aheejin added a commit to aheejin/binaryen that referenced this pull request Sep 4, 2024
This renames `Catch(All)_P3` enum to denote the old Phase 3
`catch(_all)` instructions to `Catch(All)_Legacy`, which sounds clearer.

This is also to be consistent with
llvm/llvm-project#107187.
aheejin added a commit to WebAssembly/binaryen that referenced this pull request Sep 4, 2024
This renames `Catch(All)_P3` enum to denote the old Phase 3
`catch(_all)` instructions to `Catch(All)_Legacy`, which sounds clearer.

This is also to be consistent with
llvm/llvm-project#107187.
@aheejin aheejin merged commit aecbc92 into llvm:main Sep 4, 2024
8 checks passed
@aheejin aheejin deleted the catch_p3 branch September 4, 2024 23:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants