Introduce a "gc-live" bundle for the gc arguments of a statepoint

[reames]

Summary

Currently, gc.relocates are defined in terms of indices into the statepoint's operand list. Given the gc args are at the end of a variable length list of operands, this makes interpreting their indices by hand a tad challenging. We can simplify the statepoint sequence and improve readability quite a bit by pulling these new operands into their own named operand bundle.

This patch defines a new operand bundle tag "gc-live". The semantics of the bundle are the same as the existing gc arguments of a statepoint. This patch simply introduces the definition and codegen for the bundle, future patches will migrate RS4GC to emitting the new form.

I'll note that I'm not particularly attached to the name "gc-live". If anyone has a better suggestion, please make it.

Interestingly, with this done and the recent migration to using deopt and gc-transition bundles, we really don't have much left in the statepoint itself. It really looks like the existing ID and flags fields are redundant; we have (existing!) attributes for all of them. I think we'll be able to reduce the gc.statepoint signature to simply a wrapped call (e.g. actual target and actual arguments).

[dantrushin]

LGTM with one note (you can ignore it if you have reasons).

llvm/lib/IR/Verifier.cpp

4760

BTW, I find this inconsistent with definition of GCRelocateInst.
Its base/derived ptr indices are unsigned and one can successfully create gc.relocate with index in [0, 2^32-1] range.
Then verifier suddenly shrinks allowed range into [0, 2^31-1].
Do you really want enforce index be positive signed int? Why not just require it is smaller (unsigned) than Inputs.size()?

[anna]

LGTM w/ comments inline.

llvm/docs/Statepoints.rst

620

Nit: then *both arguments* are indices into the operand bundle's operand.

636

or within the operand list in gc-live bundle.

llvm/test/CodeGen/X86/statepoint-gc-live.ll

39

Perhaps add a testcase for a relocated derived pointer?

[reames]

commit 0e7c77053f560d391fecbec5d5e0e42082865c8a

[reames]

llvm/lib/IR/Verifier.cpp

4760

Addressed in a separate commit.

commit ff529e0f2792e1383a602e4b8a466337fd0c2926

llvm/docs/LangRef.rst

.. code-block:: llvm
       ...
 
       %t = call token @llvm.call.preallocated.setup(i32 1)
       %a = call i8* @llvm.call.preallocated.arg(token %t, i32 0) preallocated(%foo)
       %b = bitcast i8* %a to %foo*
       ; initialize %b
       call void @bar(i32 42, %foo* preallocated(%foo) %b) ["preallocated"(token %t)]
 
+.. _ob_gc_live
+
+GC Live Operand Bundles
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+A "gc-live" operand bundle is only valid on a :ref:`gc.statepoint <gc_statepoint>`
+intrinsic. The operand bundle must contain every pointer to a garbage collected
+object which potentially needs to be updated by the garbage collector.
+
+When lowered, any relocated value will be recorded in the corresponding
+:ref:`stackmap entry <statepoint-stackmap-format>`.  See the intrinsic description
+for further details.
+
 .. _moduleasm:
 
 Module-Level Inline Assembly
 ----------------------------
 
 Modules may contain "module-level inline asm" blocks, which corresponds
 to the GCC "file scope inline asm" blocks. These blocks are internally
 concatenated by LLVM and treated as a single unit, but may be separated

llvm/docs/Statepoints.rst

 strategy-specific lowering is not present, and all GC transitions are emitted as
 as single no-op before and after the call instruction. These no-ops are often
 removed by the backend during dead machine instruction elimination.
 
 
 Intrinsics
 ===========
 
+.. _gc_statepoint:
+
 'llvm.experimental.gc.statepoint' Intrinsic
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Syntax:
 """""""
 
 ::
 

 which is meaningful to the runtime.  The '# deopt args' field
 indicates how many operands are to be interpreted as 'deopt parameters'.
 
 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.
+list is not length prefixed.  Use of the 'gc parameters' list is
+deprecated and will eventually be replaced entirely with the
+:ref:`gc-live <ob_gc_live>` operand bundle.
 
 Semantics:
 """"""""""
 
 A statepoint is assumed to read and write all memory.  As a result,
 memory operations can not be reordered past a statepoint.  It is
 illegal to mark a statepoint as being either 'readonly' or 'readnone'.
 

 Operands:
 """""""""
 
 The first argument is the ``gc.statepoint`` which starts the
 safepoint sequence of which this ``gc.relocation`` is a part.
 Despite the typing of this as a generic token, *only* the value defined 
 by a ``gc.statepoint`` is legal here.
 
-The second argument is an index into the statepoints list of arguments
-which specifies the allocation for the pointer being relocated.
-This index must land within the 'gc parameter' section of the
-statepoint's argument list.  The associated value must be within the
-object with which the pointer being relocated is associated. The optimizer
-is free to change *which* interior derived pointer is reported, provided that
-it does not replace an actual base pointer with another interior derived 
-pointer.  Collectors are allowed to rely on the base pointer operand 
-remaining an actual base pointer if so constructed.
-
-The third argument is an index into the statepoint's list of arguments
-which specify the (potentially) derived pointer being relocated.  It
-is legal for this index to be the same as the second argument
-if-and-only-if a base pointer is being relocated. This index must land
-within the 'gc parameter' section of the statepoint's argument list.
+The second and third arguments are both indices into operands of their
+corresponding statepoint.  If the statepoint has a :ref:`gc-live <ob_gc_live>`
+operand bundle, then the value is an index into the operand bundle's operands.
+If there is no "gc-live" bundle, then the index is into the statepoint's list
+of arguments.  This index must land within the 'gc parameter' section of the
+statepoint's argument list.  Use of the "gc-live" form is recommended.
+
+The second argument is an index which specifies the allocation for the pointer
+being relocated. The associated value must be within the object with which the
+pointer being relocated is associated. The optimizer is free to change *which*
+interior derived pointer is reported, provided that it does not replace an
+actual base pointer with another interior derived pointer. Collectors are
+allowed to rely on the base pointer operand remaining an actual base pointer if
+so constructed.
+
+The third argument is an index which specify the (potentially) derived pointer
+being relocated.  It is legal for this index to be the same as the second
+argument if-and-only-if a base pointer is being relocated. This index must
+land within the 'gc parameter' section of the statepoint's argument list.
 
 Semantics:
 """"""""""
 
 The return value of ``gc.relocate`` is the potentially relocated value
 of the pointer specified by its arguments.  It is unspecified how the
 value of the returned pointer relates to the argument to the
 ``gc.statepoint`` other than that a) it points to the same source

llvm/include/llvm/IR/LLVMContext.h

#undef LLVM_FIXED_MD_KIND
   /// operand bundle tags without comparing strings. Keep this in sync with
   /// LLVMContext::LLVMContext().
   enum : unsigned {
     OB_deopt = 0,         // "deopt"
     OB_funclet = 1,       // "funclet"
     OB_gc_transition = 2, // "gc-transition"
     OB_cfguardtarget = 3, // "cfguardtarget"
     OB_preallocated = 4,  // "preallocated"
+    OB_gc_live = 5,       // "gc-live"
   };
 
   /// getMDKindID - Return a unique non-zero ID for the specified metadata kind.
   /// This ID is uniqued across modules in the current LLVMContext.
   unsigned getMDKindID(StringRef Name) const;
 
   /// getMDKindNames - Populate client supplied SmallVector with the name for
   /// custom metadata IDs registered in this LLVMContext.

llvm/include/llvm/IR/Statepoint.h

public:
   /// range adapter for actual call arguments
   iterator_range<const_op_iterator> actual_args() const {
     return make_range(actual_arg_begin(), actual_arg_end());
   }
 
   /// Returns an iterator to the begining of the argument range describing gc
   /// values for the statepoint.
   const_op_iterator gc_args_begin() const {
+    if (auto Opt = getOperandBundle(LLVMContext::OB_gc_live))
+      return Opt->Inputs.begin();
+
     // The current format has two length prefix bundles between call args and
     // start of gc args.  This will be removed in the near future.
     const Value *NumGCTransitionArgs = *actual_arg_end();
     uint64_t NumTrans = cast<ConstantInt>(NumGCTransitionArgs)->getZExtValue();
     const_op_iterator trans_end = actual_arg_end() + 1 + NumTrans;
     const Value *NumDeoptArgs = *trans_end;
     uint64_t NumDeopt = cast<ConstantInt>(NumDeoptArgs)->getZExtValue();
     auto I = trans_end + 1 + NumDeopt;
     assert((arg_end() - I) >= 0);
     return I;
   }
 
   /// Return an end iterator for the gc argument range
-  const_op_iterator gc_args_end() const { return arg_end(); }
+  const_op_iterator gc_args_end() const {
+    if (auto Opt = getOperandBundle(LLVMContext::OB_gc_live))
+      return Opt->Inputs.end();
+
+    return arg_end();
+  }
 
   /// Return the operand index at which the gc args begin
   unsigned gcArgsStartIdx() const {
+    assert(!getOperandBundle(LLVMContext::OB_gc_live));
     return gc_args_begin() - op_begin();
   }
 
   /// range adapter for gc arguments
   iterator_range<const_op_iterator> gc_args() const {
     return make_range(gc_args_begin(), gc_args_end());
   }
 
public:
 
   /// The index into the associate statepoint's argument list which
   /// contains the pointer whose relocation this gc.relocate describes.
   unsigned getDerivedPtrIndex() const {
     return cast<ConstantInt>(getArgOperand(2))->getZExtValue();
   }
 
   Value *getBasePtr() const {
+    if (auto Opt = getStatepoint()->getOperandBundle(LLVMContext::OB_gc_live))
+      return *(Opt->Inputs.begin() + getBasePtrIndex());
     return *(getStatepoint()->arg_begin() + getBasePtrIndex());
   }
 
   Value *getDerivedPtr() const {
+    if (auto Opt = getStatepoint()->getOperandBundle(LLVMContext::OB_gc_live))
+      return *(Opt->Inputs.begin() + getDerivedPtrIndex());
     return *(getStatepoint()->arg_begin() + getDerivedPtrIndex());
   }
 };
 
 /// Represents calls to the gc.result intrinsic.
 class GCResultInst : public GCProjectionInst {
 public:
   static bool classof(const IntrinsicInst *I) {

llvm/lib/IR/LLVMContext.cpp

assert(CFGuardTargetEntry->second == LLVMContext::OB_cfguardtarget &&
          "cfguardtarget operand bundle id drifted!");
   (void)CFGuardTargetEntry;
 
   auto *PreallocatedEntry = pImpl->getOrInsertBundleTag("preallocated");
   assert(PreallocatedEntry->second == LLVMContext::OB_preallocated &&
          "preallocated operand bundle id drifted!");
   (void)PreallocatedEntry;
 
+  auto *GCLiveEntry = pImpl->getOrInsertBundleTag("gc-live");
+  assert(GCLiveEntry->second == LLVMContext::OB_gc_live &&
+         "gc-transition operand bundle id drifted!");
+  (void)GCLiveEntry;
+
   SyncScope::ID SingleThreadSSID =
       pImpl->getOrInsertSyncScopeID("singlethread");
   assert(SingleThreadSSID == SyncScope::SingleThread &&
          "singlethread synchronization scope ID drifted!");
   (void)SingleThreadSSID;
 
   SyncScope::ID SystemSSID =
       pImpl->getOrInsertSyncScopeID("");

llvm/lib/IR/Verifier.cpp

if (Function *F = Call.getCalledFunction())
     if (Intrinsic::ID ID = (Intrinsic::ID)F->getIntrinsicID())
       visitIntrinsicCall(ID, Call);
 
   // Verify that a callsite has at most one "deopt", at most one "funclet", at
   // most one "gc-transition", at most one "cfguardtarget",
   // and at most one "preallocated" operand bundle.
   bool FoundDeoptBundle = false, FoundFuncletBundle = false,
        FoundGCTransitionBundle = false, FoundCFGuardTargetBundle = false,
-       FoundPreallocatedBundle = false;
+       FoundPreallocatedBundle = false, FoundGCLiveBundle = false;;
   for (unsigned i = 0, e = Call.getNumOperandBundles(); i < e; ++i) {
     OperandBundleUse BU = Call.getOperandBundleAt(i);
     uint32_t Tag = BU.getTagID();
     if (Tag == LLVMContext::OB_deopt) {
       Assert(!FoundDeoptBundle, "Multiple deopt operand bundles", Call);
       FoundDeoptBundle = true;
     } else if (Tag == LLVMContext::OB_gc_transition) {
       Assert(!FoundGCTransitionBundle, "Multiple gc-transition operand bundles",
if (Tag == LLVMContext::OB_deopt) {
       Assert(BU.Inputs.size() == 1,
              "Expected exactly one preallocated bundle operand", Call);
       auto Input = dyn_cast<IntrinsicInst>(BU.Inputs.front());
       Assert(Input &&
                  Input->getIntrinsicID() == Intrinsic::call_preallocated_setup,
              "\"preallocated\" argument must be a token from "
              "llvm.call.preallocated.setup",
              Call);
+    } else if (Tag == LLVMContext::OB_gc_live) {
+      Assert(!FoundGCLiveBundle, "Multiple gc-live operand bundles",
+             Call);
+      FoundGCLiveBundle = true;
     }
   }
 
   // Verify that each inlinable callsite of a debug-info-bearing function in a
   // debug-info-bearing function has a debug location attached to it. Failure to
   // do so causes assertion failures when the inliner sets up inline scope info.
   if (Call.getFunction()->getSubprogram() && Call.getCalledFunction() &&
       Call.getCalledFunction()->getSubprogram())
Assert(isa<ConstantInt>(Base),
            "gc.relocate operand #2 must be integer offset", Call);
 
     Value *Derived = Call.getArgOperand(2);
     Assert(isa<ConstantInt>(Derived),
            "gc.relocate operand #3 must be integer offset", Call);
 
     const int BaseIndex = cast<ConstantInt>(Base)->getZExtValue();
     const int DerivedIndex = cast<ConstantInt>(Derived)->getZExtValue();
+
     // Check the bounds
+    if (auto Opt = StatepointCall.getOperandBundle(LLVMContext::OB_gc_live)) {
+      Assert(0 <= BaseIndex && BaseIndex < (int)Opt->Inputs.size(),
+             "gc.relocate: statepoint base index out of bounds", Call);
+      Assert(0 <= DerivedIndex && DerivedIndex < (int)Opt->Inputs.size(),
+             "gc.relocate: statepoint derived index out of bounds", Call);
+    } else {
       Assert(0 <= BaseIndex && BaseIndex < (int)StatepointCall.arg_size(),
              "gc.relocate: statepoint base index out of bounds", Call);
       Assert(0 <= DerivedIndex && DerivedIndex < (int)StatepointCall.arg_size(),
              "gc.relocate: statepoint derived index out of bounds", Call);
 
       // Check that BaseIndex and DerivedIndex fall within the 'gc parameters'
       // section of the statepoint's argument.
       Assert(StatepointCall.arg_size() > 0,
              "gc.statepoint: insufficient arguments");
       Assert(isa<ConstantInt>(StatepointCall.getArgOperand(3)),
              "gc.statement: number of call arguments must be constant integer");
       const unsigned NumCallArgs =
         cast<ConstantInt>(StatepointCall.getArgOperand(3))->getZExtValue();
       Assert(StatepointCall.arg_size() > NumCallArgs + 5,
              "gc.statepoint: mismatch in number of call arguments");
       Assert(isa<ConstantInt>(StatepointCall.getArgOperand(NumCallArgs + 5)),
              "gc.statepoint: number of transition arguments must be "
              "a constant integer");
       const int NumTransitionArgs =
           cast<ConstantInt>(StatepointCall.getArgOperand(NumCallArgs + 5))
               ->getZExtValue();
       const int DeoptArgsStart = 4 + NumCallArgs + 1 + NumTransitionArgs + 1;
       Assert(isa<ConstantInt>(StatepointCall.getArgOperand(DeoptArgsStart)),
              "gc.statepoint: number of deoptimization arguments must be "
              "a constant integer");
       const int NumDeoptArgs =
           cast<ConstantInt>(StatepointCall.getArgOperand(DeoptArgsStart))
               ->getZExtValue();
       const int GCParamArgsStart = DeoptArgsStart + 1 + NumDeoptArgs;
       const int GCParamArgsEnd = StatepointCall.arg_size();
       Assert(GCParamArgsStart <= BaseIndex && BaseIndex < GCParamArgsEnd,
              "gc.relocate: statepoint base index doesn't fall within the "
              "'gc parameters' section of the statepoint call",
              Call);
       Assert(GCParamArgsStart <= DerivedIndex && DerivedIndex < GCParamArgsEnd,
              "gc.relocate: statepoint derived index doesn't fall within the "
              "'gc parameters' section of the statepoint call",
              Call);
+    }
 
     // Relocated value must be either a pointer type or vector-of-pointer type,
     // but gc_relocate does not need to return the same pointer type as the
     // relocated pointer. It can be casted to the correct type later if it's
     // desired. However, they must have the same address space and 'vectorness'
     GCRelocateInst &Relocate = cast<GCRelocateInst>(Call);
     Assert(Relocate.getDerivedPtr()->getType()->isPtrOrPtrVectorTy(),
            "gc.relocate: relocated value must be a gc pointer", Call);

llvm/test/Bitcode/operand-bundles-bc-analyzer.ll

 ; RUN: llvm-as < %s | llvm-bcanalyzer -dump -disable-histogram | FileCheck %s
 
 ; CHECK:  <OPERAND_BUNDLE_TAGS_BLOCK
 ; CHECK-NEXT:    <OPERAND_BUNDLE_TAG
 ; CHECK-NEXT:    <OPERAND_BUNDLE_TAG
 ; CHECK-NEXT:    <OPERAND_BUNDLE_TAG
 ; CHECK-NEXT:    <OPERAND_BUNDLE_TAG
 ; CHECK-NEXT:    <OPERAND_BUNDLE_TAG
 ; CHECK-NEXT:    <OPERAND_BUNDLE_TAG
 ; CHECK-NEXT:    <OPERAND_BUNDLE_TAG
+; CHECK-NEXT:    <OPERAND_BUNDLE_TAG
 ; CHECK-NEXT:  </OPERAND_BUNDLE_TAGS_BLOCK
 
 ; CHECK:   <FUNCTION_BLOCK
 ; CHECK:    <OPERAND_BUNDLE
 ; CHECK:    <OPERAND_BUNDLE
 ; CHECK-NOT: <OPERAND_BUNDLE
 ; CHECK:  </FUNCTION_BLOCK
 

llvm/test/CodeGen/X86/statepoint-gc-live.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs < %s | FileCheck %s
+
+target datalayout = "e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-win64"
+
+declare void @foo()
+
+define void @test_empty() gc "statepoint-example" {
+; CHECK-LABEL: test_empty:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    pushq %rax
+; CHECK-NEXT:    .cfi_def_cfa_offset 16
+; CHECK-NEXT:    callq foo
+; CHECK-NEXT:  .Ltmp0:
+; CHECK-NEXT:    popq %rax
+; CHECK-NEXT:    .cfi_def_cfa_offset 8
+; CHECK-NEXT:    retq
+entry:
+  %token = call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* @foo, i32 0, i32 0, i32 0, i32 0) ["gc-live" ()]
+  ret void
+}
+
+define void @test_dead(i8 addrspace(1)* %p) gc "statepoint-example" {
+; CHECK-LABEL: test_dead:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    pushq %rax
+; CHECK-NEXT:    .cfi_def_cfa_offset 16
+; CHECK-NEXT:    callq foo
+; CHECK-NEXT:  .Ltmp1:
+; CHECK-NEXT:    popq %rax
+; CHECK-NEXT:    .cfi_def_cfa_offset 8
+; CHECK-NEXT:    retq
+entry:
+  %token = call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* @foo, i32 0, i32 0, i32 0, i32 0) ["gc-live" (i8 addrspace(1)* %p)]
+  ret void
+}
+
+define i8 addrspace(1)* @test_one(i8 addrspace(1)* %p) gc "statepoint-example" {
+; CHECK-LABEL: test_one:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    pushq %rax
+; CHECK-NEXT:    .cfi_def_cfa_offset 16
+; CHECK-NEXT:    movq %rdi, (%rsp)
+; CHECK-NEXT:    callq foo
+; CHECK-NEXT:  .Ltmp2:
+; CHECK-NEXT:    movq (%rsp), %rax
+; CHECK-NEXT:    popq %rcx
+; CHECK-NEXT:    .cfi_def_cfa_offset 8
+; CHECK-NEXT:    retq
+entry:
+  %token = call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* @foo, i32 0, i32 0, i32 0, i32 0) ["gc-live" (i8 addrspace(1)* %p)]
+  %p2 = call i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token %token, i32 0, i32 0)
+  ret i8 addrspace(1)* %p2
+}
+
+
+declare token @llvm.experimental.gc.statepoint.p0f_isVoidf(i64, i32, void ()*, i32, i32, ...)
+declare i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token, i32, i32)
+