Start migrating away from statepoint's inline length prefixed argument bundles

[reames]

Summary

In the current statepoint design, we have four distinct groups of operands to the call: call args, gc transition args, deopt args, and gc args. This format prexisted the support in IR for operand bundles and was in fact one of the inspirations for the extension. However, we never went back and rearchitected statepoints to fully leverage bundles.

This change is the first in a small sequence to do so. All this does is extend the SelectionDAG lowering code to allow deopt and gc transition operands to be specified in either inline argument bundles or operand bundles.

Once this is in, I plan to do a few follow on changes:

1. Switch RS4GC to default to the bundle representation where possible. This will involve auto-updating a bunch of tests and should greatly improve readability.

2a) Manually migrate most of the codegen tests to using bundles.
2b) Mark the inline bundles as deprecated in LangRef.

3. Propose a new "gc" bundle type to remove the second to last embedded bundle. (Call args seem to make sense where they are, so I'm not planning to remove those.)
4. Default RS4GC to generating the operand bundle form.
5. Repeat 2a/2b for gc operands. 5a (test update) is the most error prone manual step unfortunately.
6. Once all of the above has landed, and baked for a bit, change the format in LangRef and drop lowering support for the old format.

[reames]

Fix a couple of stylistic points noticed when looking at the web diff

[anna]

LGTM. thanks for working on this! Apart from dropping all of the extra handling code in RS4GC and verifier (which itself is a really good reason for this support btw), I'm assuming this for optimizations which know about the deopt_op bundle (or gc_transition bundle) after relocations are made explicit? i.e., a general robustness patch.

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

2778

hmm. so we currently don't actually have any optimizations leveraging OB_gc_transition.

[reames]

In D80598#2056404, @anna wrote:

LGTM. thanks for working on this! Apart from dropping all of the extra handling code in RS4GC and verifier (which itself is a really good reason for this support btw), I'm assuming this for optimizations which know about the deopt_op bundle (or gc_transition bundle) after relocations are made explicit? i.e., a general robustness patch.

I can't quite parse what you meant with that last sentence.

The current plan is to use operand bundles throughout. We'll still have statepoints to wrap the actual call, but all of the special operands will stay in bundles.

I did find one minor annoyance. We apparently treat deopt bundles differently than statepoints with deopt args in the inliner and that's causing a crash with the follow up patch. I'm going to have to restructure things a bit for the next step to be NFC. (All in reference to patch not yet posted for review.)

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

2778

Yes, though this line is about supporting invokable statepoint points with transition bundles.

[reames]

Closed w/commit 1af3705c7fe23db9d5308bfdf07bfbd04398b895

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

void SelectionDAGBuilder::visitInvoke(const InvokeInst &I) {
   // Retrieve successors. Look through artificial IR level blocks like
   // catchswitch for successors.
   MachineBasicBlock *Return = FuncInfo.MBBMap[I.getSuccessor(0)];
   const BasicBlock *EHPadBB = I.getSuccessor(1);
 
   // Deopt bundles are lowered in LowerCallSiteWithDeoptBundle, and we don't
   // have to do anything here to lower funclet bundles.
   assert(!I.hasOperandBundlesOtherThan({LLVMContext::OB_deopt,
+                                        LLVMContext::OB_gc_transition,
                                         LLVMContext::OB_funclet,
                                         LLVMContext::OB_cfguardtarget}) &&
          "Cannot lower invokes with arbitrary operand bundles yet!");
 
   const Value *Callee(I.getCalledOperand());
   const Function *Fn = dyn_cast<Function>(Callee);
   if (isa<InlineAsm>(Callee))
     visitInlineAsm(I);

llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp

for (const GCRelocateInst *Relocate : ISP.getRelocates()) {
     if (Seen.insert(DerivedSD).second) {
       SI.Bases.push_back(Relocate->getBasePtr());
       SI.Ptrs.push_back(Relocate->getDerivedPtr());
     }
   }
 
   SI.GCArgs = ArrayRef<const Use>(ISP.gc_args_begin(), ISP.gc_args_end());
   SI.StatepointInstr = ISP.getInstruction();
-  SI.GCTransitionArgs = ArrayRef<const Use>(ISP.gc_transition_args_begin(),
-                                            ISP.gc_transition_args_end());
   SI.ID = ISP.getID();
+
+  if (auto Opt = ISP.getCall()->getOperandBundle(LLVMContext::OB_deopt)) {
+    assert(ISP.deopt_begin() == ISP.deopt_end() &&
+           "can't list both deopt operands and deopt bundle");
+    auto &Inputs = Opt->Inputs;
+    SI.DeoptState = ArrayRef<const Use>(Inputs.begin(), Inputs.end());
+  } else {
     SI.DeoptState = ArrayRef<const Use>(ISP.deopt_begin(), ISP.deopt_end());
+  }
+  if (auto Opt = ISP.getCall()->getOperandBundle(LLVMContext::OB_gc_transition)) {
+    assert(ISP.gc_transition_args_begin() == ISP.gc_transition_args_end() &&
+           "can't list both gc_transition operands and bundle");
+    auto &Inputs = Opt->Inputs;
+    SI.GCTransitionArgs = ArrayRef<const Use>(Inputs.begin(), Inputs.end());
+  } else {
+    SI.GCTransitionArgs = ArrayRef<const Use>(ISP.gc_transition_args_begin(),
+                                              ISP.gc_transition_args_end());
+  }
+
   SI.StatepointFlags = ISP.getFlags();
   SI.NumPatchBytes = ISP.getNumPatchBytes();
   SI.EHPadBB = EHPadBB;
 
   SDValue ReturnValue = LowerAsSTATEPOINT(SI);
 
   // Export the result value if needed
   const GCResultInst *GCResult = ISP.getGCResult();

llvm/lib/IR/Verifier.cpp

Assert(isa<ConstantInt>(NumTransitionArgsV),
          "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");
+  }
+
   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");
+  }
+
   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
   // of the same statepoint sequence

llvm/test/CodeGen/X86/statepoint-gctransition-call-lowering.ll

 entry:
   %val = alloca i32
   %arg = alloca i8
   %safepoint_token = call token (i64, i32, i32 (i32, i8*)*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_i32i32p0i8f(i64 0, i32 0, i32 (i32, i8*)* @return_i32_with_args, i32 2, i32 1, i32 0, i8* %arg, i32 2, i32* %val, i64 42, i32 0)
   %call1 = call i32 @llvm.experimental.gc.result.i32(token %safepoint_token)
   ret i32 %call1
 }
 
+; Same as test_transition_args_2 except using bundle format
+define i32 @test_bundle() gc "statepoint-example" {
+; CHECK-LABEL: test_bundle
+; CHECK: pushq %rax
+; CHECK: callq return_i32
+; CHECK: popq %rcx
+; CHECK: retq
+entry:
+  %val = alloca i32
+  %arg = alloca i8
+  %safepoint_token = call token (i64, i32, i32 (i32, i8*)*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_i32i32p0i8f(i64 0, i32 0, i32 (i32, i8*)* @return_i32_with_args, i32 2, i32 1, i32 0, i8* %arg, i32 0, i32 0) ["gc-transition" (i32* %val, i64 42)]
+  %call1 = call i32 @llvm.experimental.gc.result.i32(token %safepoint_token)
+  ret i32 %call1
+}
+
 declare token @llvm.experimental.gc.statepoint.p0f_i1f(i64, i32, i1 ()*, i32, i32, ...)
 declare i1 @llvm.experimental.gc.result.i1(token)
 
 declare token @llvm.experimental.gc.statepoint.p0f_i32f(i64, i32, i32 ()*, i32, i32, ...)
 declare token @llvm.experimental.gc.statepoint.p0f_i32i32p0i8f(i64, i32, i32 (i32, i8*)*, i32, i32, ...)
 declare i32 @llvm.experimental.gc.result.i32(token)
 
 declare token @llvm.experimental.gc.statepoint.p0f_p0i32f(i64, i32, i32* ()*, i32, i32, ...)
 declare i32* @llvm.experimental.gc.result.p0i32(token)
 
 declare token @llvm.experimental.gc.statepoint.p0f_f32f(i64, i32, float ()*, i32, i32, ...)
 declare float @llvm.experimental.gc.result.f32(token)
 
 declare token @llvm.experimental.gc.statepoint.p0f_isVoidi32varargf(i64, i32, void (i32, ...)*, i32, i32, ...)
 
 declare i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(token, i32, i32)

llvm/test/CodeGen/X86/statepoint-regs.ll

; CHECK-NEXT:    retq
     %statepoint_token = call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 2, i32 5, void ()* nonnull @bar, i32 0, i32 0, i32 0, i32 20,
     float 0x40421A1CA0000000, float 0x40459A1CA0000000, float 0x40401A1CA0000000, float 0x40479A1CA0000000, float 0x403C343940000000,
     float 0x403E343940000000, float 0x40469A1CA0000000, float 0x40489A1CA0000000, float 0x404A9A1CA0000000, float 0x40499A1CA0000000,
     float 0xC05FCD2F20000000, float 0xC05C0D2F20000000, float 0xC060269780000000, float 0xC05B8D2F20000000, float 0xC060669780000000,
     float 0xC05B0D2F20000000, float 0xC060A69780000000, float 0xC05A8D2F20000000, float 0xC060E69780000000, float 0x40439A1CA0000000, i32 addrspace(1)* %in)
     %out = call coldcc i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(token %statepoint_token, i32 27, i32 27)
     ret i32 addrspace(1)* %out
 }
+
+; Same as test1, but using deopt bundle
+define void @test1b(i32 %a) gc "statepoint-example" {
+; CHECK-LABEL: test1b:
+; CHECK:       ## %bb.0: ## %entry
+; CHECK-NEXT:    pushq %rax
+; CHECK-NEXT:    .cfi_def_cfa_offset 16
+; CHECK-NEXT:    movl %edi, {{[-0-9]+}}(%r{{[sb]}}p) ## 4-byte Spill
+; CHECK-NEXT:    callq _bar ## 4-byte Folded Reload
+; CHECK-NEXT:  Ltmp19:
+; CHECK-NEXT:    popq %rax
+; CHECK-NEXT:    retq
+entry:
+  %statepoint_token1 = call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 2882400000, i32 0, void ()* @bar, i32 0, i32 0, i32 0, i32 0) ["deopt" (i32 %a)]
+  ret void
+}
+
 ; CHECK-LABEL: __LLVM_StackMaps:
 ; CHECK: .long   Ltmp18-_test_fpconst_deopt
 ; CHECK-NEXT: .short  0
 ; CHECK-NEXT: .short  25
 ; CHECK-NEXT: .byte 4
 ; CHECK-NEXT: .byte 0
 ; CHECK-NEXT: .short  8
 ; CHECK-NEXT: .short  0