Remove deopt and gc transition arguments from gc.statepoint intrinsic

[reames]

Summary

In a recent series of changes, I've introduced support for using the respective operand bundle kinds on the statepoint. At the moment, code supports either/or, but there's no need to keep the old support around. For the moment, I am simply changing the specification and verifier to require zero length argument sets in the intrinsic.

The intrinsic itself is experimental. Given that, there's no forward serialization needed. The in tree uses and generation have already been updated to use the new operand bundle based forms, the only folks broken by the change will be those with frontends generating statepoints directly and the updates should be easy.

Why not go ahead and just remove the arguments entirely? Well, I plan to. But while working on this I've found that almost all of the arguments to the statepoint can be expressed via operand bundles or attributes. Given that, I'm planning a radical simplification of the arguments and figured I'd do one update not several small ones.

While I'm posting this for review now, I do not plan to land for about 2 weeks. I want all the previous changes to have plenty of time to work through downstream test cycles before I make the transition hard to reverse. At the moment, I'd only need to revert one patch and I want to keep it that way for a bit longer.

[dantrushin]

LGTM

llvm/docs/LangRef.rst

 ``"gc-transition"`` operand bundle tag. These operand bundles mark a
 call as a transition between a function with one GC strategy to a
 function with a different GC strategy. If coordinating the transition
 between GC strategies requires additional code generation at the call
 site, these bundles may contain any values that are needed by the
 generated code.  For more details, see :ref:`GC Transitions
 <gc_transition_args>`.
 
+The bundle contain an arbitrary list of Values which need to be passed
+to GC transition code. They will be lowered and passed as operands to
+the appropriate GC_TRANSITION nodes in the selection DAG. It is assumed
+that these arguments must be available before and after (but not
+necessarily during) the execution of the callee.
+
 .. _assume_opbundles:
 
 Assume Operand Bundles
 ^^^^^^^^^^^^^^^^^^^^^^
 
 Operand bundles on an :ref:`llvm.assume <int_assume>` allows representing
 assumptions that a :ref:`parameter attribute <paramattrs>` or a
 :ref:`function attribute <fnattrs>` holds for a certain value at a certain

llvm/docs/Statepoints.rst

 
 ::
 
       declare token
         @llvm.experimental.gc.statepoint(i64 <id>, i32 <num patch bytes>,
                        func_type <target>, 
                        i64 <#call args>, i64 <flags>,
                        ... (call parameters),
-                       i64 <# transition args>, ... (transition parameters),
-                       i64 <# deopt args>, ... (deopt parameters),
+                       i64 0, i64 0,
                        ... (gc parameters))
 
 Overview:
 """""""""
 
 The statepoint intrinsic represents a call which is parse-able by the
 runtime.
 

 
 The 'call parameters' arguments are simply the arguments which need to
 be passed to the call target.  They will be lowered according to the
 specified calling convention and otherwise handled like a normal call
 instruction.  The number of arguments must exactly match what is
 specified in '# call args'.  The types must match the signature of
 'target'.
 
-The 'transition parameters' arguments contain an arbitrary list of
-Values which need to be passed to GC transition code. They will be
-lowered and passed as operands to the appropriate GC_TRANSITION nodes
-in the selection DAG. It is assumed that these arguments must be
-available before and after (but not necessarily during) the execution
-of the callee. The '# transition args' field indicates how many operands
-are to be interpreted as 'transition parameters'.
-
-The 'deopt parameters' arguments contain an arbitrary list of Values
-which is meaningful to the runtime.  The '# deopt args' field
-indicates how many operands are to be interpreted as 'deopt parameters'.
+The 'call parameter' attributes must be followed by two 'i64 0' constants.
+These were originally the length prefixes for 'gc transition parameter' and
+'deopt parameter' arguments, but the role of these parameter sets have been
+entirely replaced with the corresponding operand bundles.  In a future
+revision, these now redundant arguments will be removed.
 
 The 'gc parameters' arguments contain every pointer to a garbage
 collector object which potentially needs to be updated by the garbage
 collector.  Note that the argument list must explicitly contain a base
 pointer for every derived pointer listed.  The order of arguments is
 unimportant.  Unlike the other variable length parameter sets, this
 list is not length prefixed.  Use of the 'gc parameters' list is
 deprecated and will eventually be replaced entirely with the

 
 * Constant which describes the calling convention of the call target. This
   constant is a valid :ref:`calling convention identifier <callingconv>` for
   the version of LLVM used to generate the stackmap. No additional compatibility
   guarantees are made for this constant over what LLVM provides elsewhere w.r.t.
   these identifiers.
 * Constant which describes the flags passed to the statepoint intrinsic
 * Constant which describes number of following deopt *Locations* (not
-  operands)
-* Variable number of Locations, one for each deopt parameter listed in
-  the IR statepoint (same number as described by previous Constant).  At 
-  the moment, only deopt parameters with a bitwidth of 64 bits or less 
-  are supported.  Values of a type larger than 64 bits can be specified 
-  and reported only if a) the value is constant at the call site, and b) 
-  the constant can be represented with less than 64 bits (assuming zero 
+  operands).  Will be 0 if no "deopt" bundle is provided.
+* Variable number of Locations, one for each deopt parameter listed in the
+  "deopt" operand bundle.  At the moment, only deopt parameters with a bitwidth
+  of 64 bits or less are supported.  Values of a type larger than 64 bits can be
+  specified and reported only if a) the value is constant at the call site, and
+  b) the constant can be represented with less than 64 bits (assuming zero 
   extension to the original bitwidth).
 * Variable number of relocation records, each of which consists of 
   exactly two Locations.  Relocation records are described in detail
   below.
 
 Each relocation record provides sufficient information for a collector to 
 relocate one or more derived pointers.  Each record consists of a pair of 
 Locations.  The second element in the record represents the pointer (or 

llvm/lib/CodeGen/TargetInstrInfo.cpp

default:
     llvm_unreachable("unexpected stackmap opcode");
   }
 
   unsigned DefToFoldIdx = MI.getNumOperands();
 
   // Return false if any operands requested for folding are not foldable (not
   // part of the stackmap's live values).
   for (unsigned Op : Ops) {
+    // Caller is expected to avoid passing in tied operands
+    assert(!MI.getOperand(Op).isTied());
     if (Op < NumDefs) {
       assert(DefToFoldIdx == MI.getNumOperands() && "Folding multiple defs");
       DefToFoldIdx = Op;
     } else if (Op < StartIdx) {
       return nullptr;
     }
-    // When called from regalloc (InlineSpiller), operands must be untied,
-    // and regalloc will take care of (re)loading operand from memory.
-    // But when called from other places (e.g. peephole pass),
-    // we cannot fold operand which are tied - callers are unaware they
-    // need to reload destination register.
-    if (MI.getOperand(Op).isTied())
-      return nullptr;
   }
 
   MachineInstr *NewMI =
       MF.CreateMachineInstr(TII.get(MI.getOpcode()), MI.getDebugLoc(), true);
   MachineInstrBuilder MIB(MF, NewMI);
 
   // No need to fold return, the meta data, and function arguments
   for (unsigned i = 0; i < StartIdx; ++i)

llvm/lib/IR/Verifier.cpp

void Verifier::verifyStatepoint(const CallBase &Call) {
 
   const Value *NumTransitionArgsV = Call.getArgOperand(EndCallArgsInx + 1);
   Assert(isa<ConstantInt>(NumTransitionArgsV),
          "gc.statepoint number of transition arguments "
          "must be constant integer",
          Call);
   const int NumTransitionArgs =
       cast<ConstantInt>(NumTransitionArgsV)->getZExtValue();
-  Assert(NumTransitionArgs >= 0,
-         "gc.statepoint number of transition arguments must be positive", Call);
-  const int EndTransitionArgsInx = EndCallArgsInx + 1 + NumTransitionArgs;
-
-  // We're migrating away from inline operands to operand bundles, enforce
-  // the either/or property during transition.
-  if (Call.getOperandBundle(LLVMContext::OB_gc_transition)) {
-    Assert(NumTransitionArgs == 0,
-           "can't use both deopt operands and deopt bundle on a statepoint");
-  }
+         "gc.statepoint w/inline transition bundle is deprecated", Call);
+  const int EndTransitionArgsInx = EndCallArgsInx + 1 + NumTransitionArgs;
 
   const Value *NumDeoptArgsV = Call.getArgOperand(EndTransitionArgsInx + 1);
   Assert(isa<ConstantInt>(NumDeoptArgsV),
          "gc.statepoint number of deoptimization arguments "
          "must be constant integer",
          Call);
   const int NumDeoptArgs = cast<ConstantInt>(NumDeoptArgsV)->getZExtValue();
-  Assert(NumDeoptArgs >= 0,
-         "gc.statepoint number of deoptimization arguments "
-         "must be positive",
-         Call);
-
-  // We're migrating away from inline operands to operand bundles, enforce
-  // the either/or property during transition.
-  if (Call.getOperandBundle(LLVMContext::OB_deopt)) {
-    Assert(NumDeoptArgs == 0,
-           "can't use both deopt operands and deopt bundle on a statepoint");
-  }
+         "gc.statepoint w/inline deopt operands is deprecated", Call);
 
   const int ExpectedNumArgs =
       7 + NumCallArgs + NumTransitionArgs + NumDeoptArgs;
   Assert(ExpectedNumArgs <= (int)Call.arg_size(),
          "gc.statepoint too few arguments according to length fields", Call);
 
   // Check that the only uses of this gc.statepoint are gc.result or
   // gc.relocate calls which are tied to this statepoint and thus part

llvm/test/Verifier/statepoint.ll

 ; RUN: opt -S %s -verify | FileCheck %s
 
 declare void @use(...)
 declare i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token, i32, i32)
 declare i64 addrspace(1)* @llvm.experimental.gc.relocate.p1i64(token, i32, i32)
 declare token @llvm.experimental.gc.statepoint.p0f_isVoidf(i64, i32, void ()*, i32, i32, ...)
 declare token @llvm.experimental.gc.statepoint.p0f_isVoidp0s_structsf(i64, i32, void (%struct*)*, i32, i32, ...)
 declare i32 @"personality_function"()
 
 ;; Basic usage
 define i64 addrspace(1)* @test1(i8 addrspace(1)* %arg) gc "statepoint-example" {
 entry:
   %cast = bitcast i8 addrspace(1)* %arg to i64 addrspace(1)*
-  %safepoint_token = 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 5, i32 0, i32 0, i32 0, i32 10, i32 0, i8 addrspace(1)* %arg, i64 addrspace(1)* %cast, i8 addrspace(1)* %arg, i8 addrspace(1)* %arg)
-  %reloc = call i64 addrspace(1)* @llvm.experimental.gc.relocate.p1i64(token %safepoint_token, i32 12, i32 13)
+  %safepoint_token = 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, i8 addrspace(1)* %arg, i64 addrspace(1)* %cast, i8 addrspace(1)* %arg, i8 addrspace(1)* %arg) ["deopt" (i32 0, i32 0, i32 0, i32 10, i32 0)]
+  %reloc = call i64 addrspace(1)* @llvm.experimental.gc.relocate.p1i64(token %safepoint_token, i32 7, i32 8)
   ;; It is perfectly legal to relocate the same value multiple times...
-  %reloc2 = call i64 addrspace(1)* @llvm.experimental.gc.relocate.p1i64(token %safepoint_token, i32 12, i32 13)
-  %reloc3 = call i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token %safepoint_token, i32 13, i32 12)
+  %reloc2 = call i64 addrspace(1)* @llvm.experimental.gc.relocate.p1i64(token %safepoint_token, i32 7, i32 8)
+  %reloc3 = call i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token %safepoint_token, i32 8, i32 7)
   ret i64 addrspace(1)* %reloc
 ; CHECK-LABEL: test1
 ; CHECK: statepoint
 ; CHECK: gc.relocate
 ; CHECK: gc.relocate
 ; CHECK: gc.relocate
 ; CHECK: ret i64 addrspace(1)* %reloc
 }
 
 ; This test catches two cases where the verifier was too strict:
 ; 1) A base doesn't need to be relocated if it's never used again
 ; 2) A value can be replaced by one which is known equal.  This
 ; means a potentially derived pointer can be known base and that
 ; we can't check that derived pointer are never bases.
 define void @test2(i8 addrspace(1)* %arg, i64 addrspace(1)* %arg2) gc "statepoint-example" {
 entry:
   %cast = bitcast i8 addrspace(1)* %arg to i64 addrspace(1)*
   %c = icmp eq i64 addrspace(1)* %cast,  %arg2
   br i1 %c, label %equal, label %notequal
 
 notequal:
   ret void
 
 equal:
-  %safepoint_token = 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 5, i32 0, i32 0, i32 0, i32 10, i32 0, i8 addrspace(1)* %arg, i64 addrspace(1)* %cast, i8 addrspace(1)* %arg, i8 addrspace(1)* %arg)
-  %reloc = call i64 addrspace(1)* @llvm.experimental.gc.relocate.p1i64(token %safepoint_token, i32 12, i32 13)
+  %safepoint_token = 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, i8 addrspace(1)* %arg, i64 addrspace(1)* %cast, i8 addrspace(1)* %arg, i8 addrspace(1)* %arg) ["deopt" (i32 0, i32 0, i32 0, i32 10, i32 0)]
+  %reloc = call i64 addrspace(1)* @llvm.experimental.gc.relocate.p1i64(token %safepoint_token, i32 7, i32 7)
   call void undef(i64 addrspace(1)* %reloc)
   ret void
 ; CHECK-LABEL: test2
 ; CHECK-LABEL: equal
 ; CHECK: statepoint
 ; CHECK-NEXT: %reloc = call 
 ; CHECK-NEXT: call
 ; CHECK-NEXT: ret voi
 }
 
 ; Basic test for invoke statepoints
 define i8 addrspace(1)* @test3(i8 addrspace(1)* %obj, i8 addrspace(1)* %obj1) gc "statepoint-example" personality i32 ()* @"personality_function" {
 ; CHECK-LABEL: test3
 entry:
   ; CHECK-LABEL: entry
   ; CHECK: statepoint
-  %0 = invoke 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 5, i32 0, i32 -1, i32 0, i32 0, i32 0, i8 addrspace(1)* %obj, i8 addrspace(1)* %obj1)
+  %0 = invoke 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, i8 addrspace(1)* %obj, i8 addrspace(1)* %obj1) ["deopt" (i32 0, i32 -1, i32 0, i32 0, i32 0)]
           to label %normal_dest unwind label %exceptional_return
 
 normal_dest:
   ; CHECK-LABEL: normal_dest:
   ; CHECK: gc.relocate
   ; CHECK: gc.relocate
   ; CHECK: ret
-  %obj.relocated = call coldcc i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token %0, i32 12, i32 12)
-  %obj1.relocated = call coldcc i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token %0, i32 12, i32 12)
+  %obj.relocated = call coldcc i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token %0, i32 7, i32 7)
+  %obj1.relocated = call coldcc i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token %0, i32 8, i32 8)
   ret i8 addrspace(1)* %obj.relocated
 
 exceptional_return:
   ; CHECK-LABEL: exceptional_return
   ; CHECK: gc.relocate
   ; CHECK: gc.relocate
   %landing_pad = landingpad token
           cleanup
-  %obj.relocated1 = call coldcc i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token %landing_pad, i32 12, i32 12)
-  %obj1.relocated1 = call coldcc i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token %landing_pad, i32 12, i32 12)
+  %obj.relocated1 = call coldcc i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token %landing_pad, i32 7, i32 7)
+  %obj1.relocated1 = call coldcc i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token %landing_pad, i32 8, i32 8)
   ret i8 addrspace(1)* %obj1.relocated1
 }
 
 ; Test for statepoint with sret attribute.
 ; This should be allowed as long as the wrapped function is not vararg.
 %struct = type { i64, i64, i64 }
 
 declare void @fn_sret(%struct* sret)