[PATCH] D65246: [WebAssembly] Do not emit tail calls with return type mismatch

[Heejin Ahn via Phabricator via llvm-commits]

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