Introduce a "gc-live" bundle for the gc arguments of a statepoint
authorPhilip Reames <listmail@philipreames.com>
Wed, 3 Jun 2020 21:56:24 +0000 (14:56 -0700)
committerPhilip Reames <listmail@philipreames.com>
Wed, 3 Jun 2020 22:00:24 +0000 (15:00 -0700)
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.

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).

Differential Revision: https://reviews.llvm.org/D80937

llvm/docs/LangRef.rst
llvm/docs/Statepoints.rst
llvm/include/llvm/IR/LLVMContext.h
llvm/include/llvm/IR/Statepoint.h
llvm/lib/IR/LLVMContext.cpp
llvm/lib/IR/Verifier.cpp
llvm/test/Bitcode/operand-bundles-bc-analyzer.ll
llvm/test/CodeGen/X86/statepoint-gc-live.ll [new file with mode: 0644]

index 5ffe826..de628a9 100644 (file)
@@ -2240,6 +2240,19 @@ that will have been done by one of the ``@llvm.call.preallocated.*`` intrinsics.
       ; 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
index 5d6033b..034cbff 100644 (file)
@@ -434,6 +434,8 @@ removed by the backend during dead machine instruction elimination.
 Intrinsics
 ===========
 
+.. _gc_statepoint:
+
 'llvm.experimental.gc.statepoint' Intrinsic
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
@@ -530,7 +532,9 @@ 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:
 """"""""""
@@ -611,21 +615,25 @@ 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 both arguments are indices 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.
 
 Semantics:
 """"""""""
index 769fc01..c465e02 100644 (file)
@@ -92,6 +92,7 @@ public:
     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.
index d314842..2a0094c 100644 (file)
@@ -157,6 +157,9 @@ public:
   /// 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();
@@ -170,10 +173,16 @@ public:
   }
 
   /// 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();
   }
 
@@ -458,10 +467,14 @@ public:
   }
 
   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());
   }
 };
index f8efe2c..7ebca52 100644 (file)
@@ -73,6 +73,11 @@ LLVMContext::LLVMContext() : pImpl(new LLVMContextImpl(*this)) {
          "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 &&
index 677dc02..00225c2 100644 (file)
@@ -3102,7 +3102,7 @@ void Verifier::visitCallBase(CallBase &Call) {
   // 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();
@@ -3139,6 +3139,10 @@ void Verifier::visitCallBase(CallBase &Call) {
              "\"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;
     }
   }
 
@@ -4746,45 +4750,53 @@ void Verifier::visitIntrinsicCall(Intrinsic::ID ID, CallBase &Call) {
 
     const int BaseIndex = cast<ConstantInt>(Base)->getZExtValue();
     const int DerivedIndex = cast<ConstantInt>(Derived)->getZExtValue();
+
     // Check the bounds
-    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 =
+    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);
+      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
index 6d7d4c0..f22fcbe 100644 (file)
@@ -8,6 +8,7 @@
 ; 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
diff --git a/llvm/test/CodeGen/X86/statepoint-gc-live.ll b/llvm/test/CodeGen/X86/statepoint-gc-live.ll
new file mode 100644 (file)
index 0000000..fbbd320
--- /dev/null
@@ -0,0 +1,82 @@
+; 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
+}
+
+define i8 addrspace(1)* @test_one_derived(i8 addrspace(1)* %p) gc "statepoint-example" {
+; CHECK-LABEL: test_one_derived:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    subq $24, %rsp
+; CHECK-NEXT:    .cfi_def_cfa_offset 32
+; CHECK-NEXT:    movq %rdi, {{[0-9]+}}(%rsp)
+; CHECK-NEXT:    addq $8, %rdi
+; CHECK-NEXT:    movq %rdi, {{[0-9]+}}(%rsp)
+; CHECK-NEXT:    callq foo
+; CHECK-NEXT:  .Ltmp3:
+; CHECK-NEXT:    movq {{[0-9]+}}(%rsp), %rax
+; CHECK-NEXT:    addq $24, %rsp
+; CHECK-NEXT:    .cfi_def_cfa_offset 8
+; CHECK-NEXT:    retq
+entry:
+  %gep = getelementptr i8, i8 addrspace(1)* %p, i32 8
+  %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, i8 addrspace(1)* %gep)]
+  %gep2 = call i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token %token, i32 0, i32 1)
+  ret i8 addrspace(1)* %gep2
+}
+
+
+
+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)
+