[PATCH] D94050: [WebAssembly] Handle EH terminate pads for cleanup

Heejin Ahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 29 09:00:48 PST 2021


aheejin added inline comments.


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyHandleEHTerminatePads.cpp:125
+    // a single BB.
+    MachineBasicBlock *CatchBB = Call->getParent();
+    assert(CatchBB->isEHPad());
----------------
tlively wrote:
> Is it possible for one BB to contain multiple calls to `__clang_call_terminate`? If so, that BB would end up with multiple `catch_all`s in the current code. Would it be worth asserting that this doesn't happen?
We [[ https://github.com/llvm/llvm-project/blob/5cf6412a27892a7a48c83e26d79f8c3ae1cfa944/llvm/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp#L319-L323 | ensure ]] that doesn't happen in LateEHPrepare, [[ https://reviews.llvm.org/D94045#2484875 | thanks to ]] you.

If we want to put some `assert` here it wouldn't be a single line assert, but we need to keep a set of something to see if there is no `__clang_call_terminate` calls with an EH pad that has already been added, which we are doing in LateEHPrepare anyway. I guess we don't need to duplicate that logic just for assertion..?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94050



More information about the llvm-commits mailing list