[PATCH] D81648: MIR Statepoint refactoring. Part 4: ISEL changes.

[Philip Reames via Phabricator via llvm-commits]

reames requested changes to this revision.
reames added a comment.
This revision now requires changes to proceed.

I took some time today to apply this patch with the attention of addressing some of the unaddressed comments and landing it to unblock progress here.  However, I hit two problems.

First, a trivial test case crashes with this flag enabled.

define void @in_register(i8 addrspace(1)* %a) #1 gc "statepoint-example" {

  %sp = call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* undef, i32 0, i32 0, i32 0, i32 0) ["gc-live" (i8 addrspace(1)* %a)]
  %a1 = call i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token %sp, i32 0, i32 0)
  call void @use(i8 addrspace(1)* %a1)
  ret void

}

Second, there appears to be a semantic problem around the handling of base vs derived slots unless we *always* spill the base.  We can't tie both uses to a single def.  This may warrant some offline discussion.

Denis, please address all of the previous applied comments, including writing targetting tests.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp:225
   unsigned NumVRegs = HasVRegVariadicDefs ? NumResults : II.getNumDefs();
+  if (Node->getMachineOpcode() == TargetOpcode::STATEPOINT)
+    NumVRegs = NumResults;
----------------
dantrushin wrote:
> reames wrote:
> > This line looks unnecessary provided the defs list is marked variadic.  If you want to leave it for the moment, that's fine, but I think we can clean this up in a follow up change.
> > 
> > (To be specific, OutOperandList in Target.td for STATEPOINT can be variable_ops, and this line disappears.)
> `II` here is `MCInstrDesc`.
> Statepoint has 0 defs in it MCIntrDesc. (It is `MachineInstr::getNumDefs` which work correctly for variadic outs).
> So I really need it here
> 
Please read the line above your change with the explicit handling for variadic defs.

(Not a blocking item.  I'll fix after submit if needed)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81648