[PATCH] D62877: [WebAssembly] Implement tail calls and unify tablegen call classes

Heejin Ahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 7 11:05:34 PDT 2019


aheejin added inline comments.


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:790
+    InTys.clear();
+    InTys.push_back(MVT::Other);
+    InTys.push_back(MVT::Glue);
----------------
tlively wrote:
> aheejin wrote:
> > - Sorry, but why should we clear `InTys` here? I understand that we don't need output types, but should input types different from normal calls?
> > - `InTys.push_back(MVT::Other);` is common for both `if` and `else`, so we can hoist it (If we don't need to clear `InTys`)
> The terminology here is super confusing, but everything named `In*` refers to values coming back into the current frame once the call finishes, i.e. the return values. `return_call` has no return values except for the chain and glue, so `InTys` gets cleared.
> 
> We would need to duplicate the condition for the clear and the glue push if we hoisted the other push, so I don't think it's worth it.
Oh I see.... I thought `InTys` is the input parameters, and that was one of the reasons I suggested merging into the existing workflow. Yeah, the benefits of merging look a bit less clear then. If you think the previous code is more clear I think it's good to revert to it. Sorry for my confusion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62877





More information about the llvm-commits mailing list