[PATCH] D88697: [WebAssembly] Rename Emscripten EH functions

[Thomas Lively via Phabricator via llvm-commits]

tlively added inline comments.


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp:151-152
+  if (!Sig->Returns.empty())
+    for (auto VT : Sig->Returns)
+      Ret += getInvokeWrapperSig(VT);
+  else
----------------
This naming scheme does not support multivalue functions. For example functions with signatures (i32) -> (i32, i32) and (i32, i32) -> (i32) both become invoke_iii and are indistinguishable. Would it make sense to error out if `Sig->Returns` has multiple elements?


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp:213
       computeSignatureVTs(F.getFunctionType(), &F, F, TM, Params, Results);
-      auto *Sym = cast<MCSymbolWasm>(getSymbol(&F));
+      // At this point these MCSymbols may or may not have created already and
+      // thus also contain a signature, but we need to get the signature anyway
----------------
"have created" => "have been created"


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp:52
+
+  const auto *FuncTy = dyn_cast<FunctionType>(Global->getValueType());
+  const MachineFunction &MF = *MO.getParent()->getParent()->getParent();
----------------
Should this be a non-dyn cast now?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88697