[PATCH] D65246: [WebAssembly] Do not emit tail calls with return type mismatch
Heejin Ahn via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 24 19:14:47 PDT 2019
aheejin added inline comments.
================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:649
+ bool MustTail = (CallConv == CallingConv::Fast && CLI.IsTailCall &&
+ MF.getTarget().Options.GuaranteedTailCallOpt) ||
+ (CLI.CS && CLI.CS.isMustTailCall());
----------------
It's preexisting, but `GuaranteedTailCallOpt`'s description says:
> When the flag is on, participating targets will perform tail call optimization on all calls which use the fastcc calling convention and which satisfy certain target-independent criteria (being at the end of a function, having the same return type as their parent function, etc.), using an alternate ABI if necessary.
Does this mean, should we at least try to do tail calls even if `CLI.IsTailCall` is not true, i.e., there is no `tailcall` property in the instruction of .ll file? If so, it seems the check for this condition should be outside of `if (CLI.IsTailCall)`. Let me know if I'm mistaken.
================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:664
+ if (!TypesMatch) {
+ assert(!MustTail);
+ CLI.IsTailCall = false;
----------------
We don't call `fail` in this case?
================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:672
+ }
}
----------------
I'm unsure how the user interface should work. When should we error out and when should we silently switch to non-tail calls? I might be mistaken, but it looks to me that unless `CLI.CS.isMustTailCall()` is true we don't need to error out, no? But currently `MustTail` contains much broader conditions, so I'm wondering what the UI should look like.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65246/new/
https://reviews.llvm.org/D65246
More information about the llvm-commits
mailing list