[PATCH] D71496: [WebAssembly] Split and recombine multivalue calls for ISel

[Thomas Lively via Phabricator via llvm-commits]

tlively marked 2 inline comments as done.
tlively added inline comments.


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp:224
+    MachineSDNode *CallParams =
+        CurDAG->getMachineNode(WebAssembly::CALL_PARAMS, DL, MVT::i32, Ops);
+    SDValue Link(CallParams, 0);
----------------
tlively wrote:
> aheejin wrote:
> > Nit: Does this `i32` have a meaning? If not, can we use a chain (`MVT::Other`) instead?
> There's no meaning, but I have not been able avoid assertion failures using `MTV::Other`. I'll investigate further without spending too much time on it.
Ok, I've switched this to use MVT::Other. This just required an update to the previous CL to avoid a nullptr dereference.


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:433
+                                           const TargetInstrInfo &TII) {
+  MachineInstr &CallParams = *CallResults.getPrevNode();
+  assert(CallParams.getOpcode() == WebAssembly::CALL_PARAMS);
----------------
tlively wrote:
> aheejin wrote:
> > Is `CallParams` guaranteed to be the previous node? What we did in ISelDAGToDAG is to make `CallParams` an operand of `CallResult`. While they have data dependences and `CallResult` will be guaranteed to be selected after `CallParams`, I'm not sure if there can't be any other instructions in between. Can't we get the operand of `CallResults`?
> Yeah, good point. I didn't see a simple API for getting defs of operands from MachineInstrs, but making this more robust is probably worth some extra code.
Now that I switched to using MVT::Other, there isn't even a shared register to look for. I think we have no choice but to blindly hope that these instructions are emitted next to each other :/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71496