[PATCH] D108955: [WebAssembly] Free setjmpTable before exiting calls in EmSjLj

[Heejin Ahn via Phabricator via llvm-commits]

aheejin added inline comments.


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp:437-444
-  // If we are calling a function that is noreturn, we must remove that
-  // attribute. The code we insert here does expect it to return, after we
-  // catch the exception.
-  if (CI->doesNotReturn()) {
-    if (auto *F = CI->getCalledFunction())
-      F->removeFnAttr(Attribute::NoReturn);
-    CI->removeFnAttr(Attribute::NoReturn);
----------------
This removes `noreturn` attribute from `emscripten_longjmp`, which makes this CL incorrect. Also, I'm not sure why this code was there in the first place. We don't call those `noreturn` functions within the LLVM IR anymore; we call invokes instead, which call the `noreturn` function via indirection, so it doesn't violate the `noreturn` property.

I traced the origin and found fastcomp's Emscripten EH handling code has the same snippet. I guess then I conservatively pasted that out of concerns something might break without that. But all emscripten tests pass without this fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108955