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

Fri Jan 17 15:30:30 PST 2020

tlively marked an inline comment as done.
tlively added inline comments.

================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:433
+                                           const TargetInstrInfo &TII) {
+  MachineInstr &CallParams = *CallResults.getPrevNode();
+  assert(CallParams.getOpcode() == WebAssembly::CALL_PARAMS);
----------------
aheejin wrote:
> tlively wrote:
> > 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 :/
> Oh, if changing to `MTV::Other` makes things harder, just don't do it ;( Sorry I was just suggesting. 
> Oh the other hand, can we use the `MTV::Glue` thing? I haven't used it myself and I'm not super familiar with it, but the definition of that is to glue nodes together, right? If we can actually guarantee they are glued together that's gonna be good.
`MVT::Glue` works! There must have been something else wrong when I experimented with it before.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71496