MIR Statepoint refactoring. Part 1: Basic MI level changes.

[dantrushin]

Summary

Basic support for variadic-def MIR Statepoint:

• Change TableGen STATEPOINT description to variadic out list (For self-documentation purpose; by itself it does not affect code generation in any way).
• Update StatepointOpers helper class to handle variadic defs.
• Update MachineVerifier to properly handle them, too.

With this change, new Statepoint instruction can be passed through
backend (excluding ISEL) without errors.

Full change set is available at D81603.

[reames]

LGTM with one minor stylistic question. Can be addressed in separate change if desired.

llvm/include/llvm/CodeGen/StackMaps.h

170

Since we have MI available, why add a new variable? I'd expect the accessors to just use MI->getNumDefs() directly?

[thegameg]

Hi, I see CodeGen/X86/statepoint-vreg.mir failing with UBSAN enabled on our internal bots:

llvm-project/llvm/lib/CodeGen/MachineVerifier.cpp:1573:33: runtime error: reference binding to null pointer of type 'const llvm::MCOperandInfo'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior llvm-project/llvm/lib/CodeGen/MachineVerifier.cpp:1573:33 in 
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace.
Stack dump:
0.	Program arguments: build/bin/llc -o - llvm-project/llvm/test/CodeGen/X86/statepoint-vreg.mir -start-after=finalize-isel 
FileCheck error: '<stdin>' is empty.
FileCheck command line:  build/bin/FileCheck llvm-project/llvm/test/CodeGen/X86/statepoint-vreg.mir

--

Do you mind taking a look please?

[dantrushin]

In D81645#2159907, @thegameg wrote:

Hi, I see CodeGen/X86/statepoint-vreg.mir failing with UBSAN enabled on our internal bots:

Should be fixed already.
Does your sources include e04fe9aefd4?

[thegameg]

In D81645#2160087, @dantrushin wrote:

In D81645#2159907, @thegameg wrote:

Hi, I see CodeGen/X86/statepoint-vreg.mir failing with UBSAN enabled on our internal bots:

Should be fixed already.
Does your sources include e04fe9aefd4?

That's what was missing. Looks fixed now, thank you!

llvm/include/llvm/CodeGen/StackMaps.h

class StatepointOpers {
   // instruction.
   enum { IDPos, NBytesPos, NCallArgsPos, CallTargetPos, MetaEnd };
 
   // These values are relative offsets from the start of the statepoint meta
   // arguments (i.e. the end of the call arguments).
   enum { CCOffset = 1, FlagsOffset = 3, NumDeoptOperandsOffset = 5 };
 
 public:
-  explicit StatepointOpers(const MachineInstr *MI) : MI(MI) {}
+  explicit StatepointOpers(const MachineInstr *MI) : MI(MI) {
+    NumDefs = MI->getNumDefs();
+  }
 
   /// Get index of statepoint ID operand.
-  unsigned getIDPos() const { return IDPos; }
+  unsigned getIDPos() const { return NumDefs + IDPos; }
 
   /// Get index of Num Patch Bytes operand.
-  unsigned getNBytesPos() const { return NBytesPos; }
+  unsigned getNBytesPos() const { return NumDefs + NBytesPos; }
 
   /// Get index of Num Call Arguments operand.
-  unsigned getNCallArgsPos() const { return NCallArgsPos; }
+  unsigned getNCallArgsPos() const { return NumDefs + NCallArgsPos; }
 
   /// Get starting index of non call related arguments
   /// (calling convention, statepoint flags, vm state and gc state).
   unsigned getVarIdx() const {
-    return MI->getOperand(NCallArgsPos).getImm() + MetaEnd;
+    return MI->getOperand(NumDefs + NCallArgsPos).getImm() + MetaEnd + NumDefs;
   }
 
   /// Get index of Calling Convention operand.
   unsigned getCCIdx() const { return getVarIdx() + CCOffset; }
 
   /// Get index of Flags operand.
   unsigned getFlagsIdx() const { return getVarIdx() + FlagsOffset; }
 
   /// Get index of Number Deopt Arguments operand.
   unsigned getNumDeoptArgsIdx() const {
     return getVarIdx() + NumDeoptOperandsOffset;
   }
 
   /// Return the ID for the given statepoint.
-  uint64_t getID() const { return MI->getOperand(IDPos).getImm(); }
+  uint64_t getID() const { return MI->getOperand(NumDefs + IDPos).getImm(); }
 
   /// Return the number of patchable bytes the given statepoint should emit.
   uint32_t getNumPatchBytes() const {
-    return MI->getOperand(NBytesPos).getImm();
+    return MI->getOperand(NumDefs + NBytesPos).getImm();
   }
 
   /// Return the target of the underlying call.
   const MachineOperand &getCallTarget() const {
-    return MI->getOperand(CallTargetPos);
+    return MI->getOperand(NumDefs + CallTargetPos);
   }
 
   /// Return the calling convention.
   CallingConv::ID getCallingConv() const {
     return MI->getOperand(getCCIdx()).getImm();
   }
 
   /// Return the statepoint flags.
   uint64_t getFlags() const { return MI->getOperand(getFlagsIdx()).getImm(); }
 
 private:
   const MachineInstr *MI;
+  unsigned NumDefs;
 };
 
 class StackMaps {
 public:
   struct Location {
     enum LocationType {
       Unprocessed,
       Register,

llvm/include/llvm/Target/Target.td

def PATCHPOINT : StandardPseudoInstruction {
   let InOperandList = (ins i64imm:$id, i32imm:$nbytes, unknown:$callee,
                        i32imm:$nargs, i32imm:$cc, variable_ops);
   let hasSideEffects = 1;
   let isCall = 1;
   let mayLoad = 1;
   let usesCustomInserter = 1;
 }
 def STATEPOINT : StandardPseudoInstruction {
-  let OutOperandList = (outs);
+  let OutOperandList = (outs variable_ops);
   let InOperandList = (ins variable_ops);
   let usesCustomInserter = 1;
   let mayLoad = 1;
   let mayStore = 1;
   let hasSideEffects = 1;
   let isCall = 1;
 }
 def LOAD_STACK_GUARD : StandardPseudoInstruction {

llvm/lib/CodeGen/MachineVerifier.cpp

 void
 MachineVerifier::visitMachineOperand(const MachineOperand *MO, unsigned MONum) {
   const MachineInstr *MI = MO->getParent();
   const MCInstrDesc &MCID = MI->getDesc();
   unsigned NumDefs = MCID.getNumDefs();
   if (MCID.getOpcode() == TargetOpcode::PATCHPOINT)
     NumDefs = (MONum == 0 && MO->isReg()) ? NumDefs : 0;
 
+  if (MCID.getOpcode() == TargetOpcode::STATEPOINT)
+    NumDefs = MI->getNumDefs();
+
   // The first MCID.NumDefs operands must be explicit register defines
   if (MONum < NumDefs) {
     const MCOperandInfo &MCOI = MCID.OpInfo[MONum];
     if (!MO->isReg())
       report("Explicit definition must be a register", MO, MONum);
     else if (!MO->isDef() && !MCOI.isOptionalDef())
       report("Explicit definition marked as use", MO, MONum);
     else if (MO->isImplicit())

llvm/test/CodeGen/X86/statepoint-vreg.mir

+# NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+# RUN: llc -o - %s -start-after=finalize-isel | FileCheck %s
+
+--- |
+  ; ModuleID = 'test/CodeGen/X86/statepoint-vreg.ll'
+  source_filename = "test/CodeGen/X86/statepoint-vreg.ll"
+  target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+  target triple = "x86_64-pc-linux-gnu"
+
+  declare void @bar()
+
+  define i32 @test_basic(i32 addrspace(1)* %obj1, i32 addrspace(1)* %obj2) gc "statepoint-example" {
+  ; CHECK-LABEL: test_basic:
+  ; CHECK:       # %bb.0:
+  ; CHECK-NEXT:    pushq %r14
+  ; CHECK-NEXT:    .cfi_def_cfa_offset 16
+  ; CHECK-NEXT:    pushq %rbx
+  ; CHECK-NEXT:    .cfi_def_cfa_offset 24
+  ; CHECK-NEXT:    pushq %rax
+  ; CHECK-NEXT:    .cfi_def_cfa_offset 32
+  ; CHECK-NEXT:    .cfi_offset %rbx, -24
+  ; CHECK-NEXT:    .cfi_offset %r14, -16
+  ; CHECK-NEXT:    movq %rsi, %r14
+  ; CHECK-NEXT:    movq %rdi, %rbx
+  ; CHECK-NEXT:    callq bar
+  ; CHECK-NEXT:  .Ltmp0:
+  ; CHECK-NEXT:    movl (%rbx), %eax
+  ; CHECK-NEXT:    addl (%r14), %eax
+  ; CHECK-NEXT:    addq $8, %rsp
+  ; CHECK-NEXT:    .cfi_def_cfa_offset 24
+  ; CHECK-NEXT:    popq %rbx
+  ; CHECK-NEXT:    .cfi_def_cfa_offset 16
+  ; CHECK-NEXT:    popq %r14
+  ; CHECK-NEXT:    .cfi_def_cfa_offset 8
+  ; CHECK-NEXT:    retq
+    %token = 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) [ "gc-live"(i32 addrspace(1)* %obj1, i32 addrspace(1)* %obj2) ]
+    %rel1 = call coldcc i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(token %token, i32 0, i32 0) ; (%obj1, %obj1)
+    %rel2 = call coldcc i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(token %token, i32 1, i32 1) ; (%obj2, %obj2)
+    %a = load i32, i32 addrspace(1)* %rel1, align 4
+    %b = load i32, i32 addrspace(1)* %rel2, align 4
+    %c = add i32 %a, %b
+    ret i32 %c
+  }
+
+  ; CHECK-LABEL:  __LLVM_StackMaps:
+  ; CHECK-NEXT:    .byte  3
+  ; CHECK-NEXT:    .byte  0
+  ; CHECK-NEXT:    .short 0
+  ; CHECK-NEXT:    .long  1
+  ; CHECK-NEXT:    .long  0
+  ; CHECK-NEXT:    .long  1
+  ; CHECK-NEXT:    .quad  test_basic
+  ; CHECK-NEXT:    .quad  24
+  ; CHECK-NEXT:    .quad  1
+  ; CHECK-NEXT:    .quad  2882400000
+  ; CHECK-NEXT:    .long  .Ltmp0-test_basic
+  ; CHECK-NEXT:    .short 0
+  ; CHECK-NEXT:    .short 8
+  ; CHECK-NEXT:    .byte  4
+  ; CHECK-NEXT:    .byte  0
+  ; CHECK-NEXT:    .short 8
+  ; CHECK-NEXT:    .short 0
+  ; CHECK-NEXT:    .short 0
+  ; CHECK-NEXT:    .long  0
+  ; CHECK-NEXT:    .byte  4
+  ; CHECK-NEXT:    .byte  0
+  ; CHECK-NEXT:    .short 8
+  ; CHECK-NEXT:    .short 0
+  ; CHECK-NEXT:    .short 0
+  ; CHECK-NEXT:    .long  0
+  ; CHECK-NEXT:    .byte  4
+  ; CHECK-NEXT:    .byte  0
+  ; CHECK-NEXT:    .short 8
+  ; CHECK-NEXT:    .short 0
+  ; CHECK-NEXT:    .short 0
+  ; CHECK-NEXT:    .long  1
+  ; CHECK-NEXT:    .byte  4
+  ; CHECK-NEXT:    .byte  0
+  ; CHECK-NEXT:    .short 8
+  ; CHECK-NEXT:    .short 0
+  ; CHECK-NEXT:    .short 0
+  ; CHECK-NEXT:    .long  0
+  ; CHECK-NEXT:    .byte  1
+  ; CHECK-NEXT:    .byte  0
+  ; CHECK-NEXT:    .short 8
+  ; CHECK-NEXT:    .short 14
+  ; CHECK-NEXT:    .short 0
+  ; CHECK-NEXT:    .long  0
+  ; CHECK-NEXT:    .byte  1
+  ; CHECK-NEXT:    .byte  0
+  ; CHECK-NEXT:    .short 8
+  ; CHECK-NEXT:    .short 14
+  ; CHECK-NEXT:    .short 0
+  ; CHECK-NEXT:    .long  0
+  ; CHECK-NEXT:    .byte  1
+  ; CHECK-NEXT:    .byte  0
+  ; CHECK-NEXT:    .short 8
+  ; CHECK-NEXT:    .short 3
+  ; CHECK-NEXT:    .short 0
+  ; CHECK-NEXT:    .long  0
+  ; CHECK-NEXT:    .byte  1
+  ; CHECK-NEXT:    .byte  0
+  ; CHECK-NEXT:    .short 8
+  ; CHECK-NEXT:    .short 3
+  ; CHECK-NEXT:    .short 0
+  ; CHECK-NEXT:    .long  0
+  ; CHECK-NEXT:    .p2align 3
+  ; CHECK-NEXT:    .short 0
+  ; CHECK-NEXT:    .short 0
+  ; CHECK-NEXT:    .p2align 3
+
+  declare token @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 immarg, i32 immarg, void ()*, i32 immarg, i32 immarg, ...)
+
+  ; Function Attrs: nounwind readonly
+  declare i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(token, i32 immarg, i32 immarg) #0
+
+  attributes #0 = { nounwind readonly }
+  attributes #1 = { nounwind }
+
+...
+---
+name:            test_basic
+alignment:       16
+selected:        false
+failedISel:      false
+tracksRegLiveness: true
+registers:
+  - { id: 0, class: gr64, preferred-register: '' }
+  - { id: 1, class: gr64, preferred-register: '' }
+  - { id: 2, class: gr64, preferred-register: '' }
+  - { id: 3, class: gr64, preferred-register: '' }
+  - { id: 4, class: gr32, preferred-register: '' }
+  - { id: 5, class: gr32, preferred-register: '' }
+liveins:
+  - { reg: '$rdi', virtual-reg: '%0' }
+  - { reg: '$rsi', virtual-reg: '%1' }
+fixedStack:      []
+stack:           []
+callSites:       []
+constants:       []
+machineFunctionInfo: {}
+body:             |
+  bb.0 (%ir-block.0):
+    liveins: $rdi, $rsi
+
+    %1:gr64 = COPY $rsi
+    %0:gr64 = COPY $rdi
+    ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+    %2:gr64, %3:gr64 = STATEPOINT 2882400000, 0, 0, @bar, 2, 0, 2, 0, 2, 1, 2, 0, %1, %1(tied-def 0), %0, %0(tied-def 1), csr_64, implicit-def $rsp, implicit-def $ssp
+    ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+    %4:gr32 = MOV32rm killed %3, 1, $noreg, 0, $noreg :: (load 4 from %ir.rel1, addrspace 1)
+    %5:gr32 = ADD32rm %4, killed %2, 1, $noreg, 0, $noreg, implicit-def dead $eflags :: (load 4 from %ir.rel2, addrspace 1)
+    $eax = COPY %5
+    RET 0, $eax
+
+...