[PATCH] D88697: [WebAssembly] Rename Emscripten EH functions
Thomas Lively via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 1 17:10:31 PDT 2020
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
More information about the llvm-commits
mailing list