MIR: Freeze reserved regs after parsing everything

[arsenm]

Summary

The AMDGPU implementation of getReservedRegs depends on
MachineFunctionInfo fields that are parsed from the YAML section. This
was reserving the wrong register since it was setting the reserved
regs before parsing the correct one.

Some tests were relying on the default reserved set for the assumed
default calling convention.

[thegameg]

This LGTM with one comment.

Also I'm not sure if there is a way to avoid this kind of bugs in the future but maybe we want to at least make sure MFI is not depending on parsing the other bits.

lib/CodeGen/MIRParser/MIRParser.cpp

430

Can you add a comment on why this needs to be set after parsing the MFI?

[arsenm]

r357083

lib/CodeGen/MIRParser/MIRParser.cpp

if (YamlMF.MachineFuncInfo) {
 
     SMRange SrcRange;
     if (TM.parseMachineFunctionInfo(*YamlMF.MachineFuncInfo, PFS, Error,
                                     SrcRange)) {
       return error(Error, SrcRange);
     }
   }
 
+  // FIXME: This is a temporary workaround until the reserved registers can be
+  // serialized.
+  MachineRegisterInfo &MRI = MF.getRegInfo();
+  MRI.freezeReservedRegs(MF);
+
   computeFunctionProperties(MF);
 
   MF.getSubtarget().mirFileLoaded(MF);
 
   MF.verify();
   return false;
 }
 
for (const MachineInstr &MI : MBB) {
       for (const MachineOperand &MO : MI.operands()) {
         if (!MO.isRegMask())
           continue;
         MRI.addPhysRegsUsedFromRegMask(MO.getRegMask());
       }
     }
   }
 
-  // FIXME: This is a temporary workaround until the reserved registers can be
-  // serialized.
-  MRI.freezeReservedRegs(MF);
   return Error;
 }
 
 bool MIRParserImpl::initializeFrameInfo(PerFunctionMIParsingState &PFS,
                                         const yaml::MachineFunction &YamlMF) {
   MachineFunction &MF = PFS.MF;
   MachineFrameInfo &MFI = MF.getFrameInfo();
   const Function &F = MF.getFunction();

test/CodeGen/AMDGPU/GlobalISel/regbankselect-phi.mir

 
 body: |
   ; CHECK-LABEL: name: phi_s1_v_vcc_sbranch
   ; CHECK: bb.0:
   ; CHECK:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
   ; CHECK:   liveins: $vgpr0, $vgpr1, $sgpr0
   ; CHECK:   [[COPY:%[0-9]+]]:vgpr(s32) = COPY $vgpr0
   ; CHECK:   [[COPY1:%[0-9]+]]:vgpr(s32) = COPY $vgpr1
-  ; CHECK:   [[COPY2:%[0-9]+]]:sgpr(s32) = COPY $sgpr1
+  ; CHECK:   [[COPY2:%[0-9]+]]:sgpr(s32) = COPY $sgpr0
   ; CHECK:   [[C:%[0-9]+]]:sgpr(s32) = G_CONSTANT i32 0
   ; CHECK:   [[TRUNC:%[0-9]+]]:vgpr(s1) = G_TRUNC [[COPY]](s32)
   ; CHECK:   [[ICMP:%[0-9]+]]:scc(s1) = G_ICMP intpred(eq), [[COPY2]](s32), [[C]]
   ; CHECK:   G_BRCOND [[ICMP]](s1), %bb.1
   ; CHECK:   G_BR %bb.2
   ; CHECK: bb.1:
   ; CHECK:   successors: %bb.2(0x80000000)
   ; CHECK:   [[ICMP1:%[0-9]+]]:vcc(s1) = G_ICMP intpred(eq), [[COPY1]](s32), [[C]]
   ; CHECK:   G_BR %bb.2
   ; CHECK: bb.2:
   ; CHECK:   [[PHI:%[0-9]+]]:vgpr(s1) = G_PHI [[TRUNC]](s1), %bb.0, [[ICMP1]](s1), %bb.1
   ; CHECK:   [[COPY3:%[0-9]+]]:vcc(s1) = COPY [[PHI]](s1)
   ; CHECK:   [[COPY4:%[0-9]+]]:vgpr(s32) = COPY [[C]](s32)
   ; CHECK:   [[SELECT:%[0-9]+]]:vgpr(s32) = G_SELECT [[COPY3]](s1), [[COPY4]], [[COPY]]
   ; CHECK:   S_SETPC_B64 undef $sgpr30_sgpr31, implicit [[SELECT]](s32)
   bb.0:
     successors: %bb.1, %bb.2
     liveins: $vgpr0, $vgpr1, $sgpr0
 
     %0:_(s32) = COPY $vgpr0
     %1:_(s32) = COPY $vgpr1
-    %2:_(s32) = COPY $sgpr1
+    %2:_(s32) = COPY $sgpr0
     %3:_(s32) = G_CONSTANT i32 0
     %4:_(s1) = G_TRUNC %0
     %5:_(s1) = G_ICMP intpred(eq), %2, %3
     G_BRCOND %5, %bb.1
     G_BR %bb.2
 
   bb.1:
     successors: %bb.2

test/CodeGen/AMDGPU/endpgm-dce.mir

 # GCN-LABEL: name: call
 # GCN:      $sgpr0_sgpr1 = S_OR_B64 $exec, killed $vcc, implicit-def $scc
 # GCN-NEXT: $sgpr4_sgpr5 = S_SWAPPC_B64 $sgpr2_sgpr3
 # GCN-NEXT: S_ENDPGM 0
 name: call
 tracksRegLiveness: true
 body:             |
   bb.0:
+    liveins: $sgpr2_sgpr3
     $vcc = IMPLICIT_DEF
     $sgpr0_sgpr1 = S_OR_B64 $exec, killed $vcc, implicit-def $scc
     $sgpr4_sgpr5 = S_SWAPPC_B64 $sgpr2_sgpr3
     S_ENDPGM 0
 ...
 ---
 # GCN-LABEL: name: exp
 # GCN:      $sgpr0_sgpr1 = S_OR_B64 $exec, killed $vcc, implicit-def $scc

test/CodeGen/AMDGPU/misched-killflags.mir

 # RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=fiji -verify-machineinstrs -run-pass=post-RA-sched -o - %s | FileCheck %s
 # Make sure ScheduleDAGInstrs::fixupKills does not produce invalid kill flags.
 ---
 name: func0
 tracksRegLiveness: true
+machineFunctionInfo:
+  isEntryFunction: true
+  scratchRSrcReg:  '$sgpr0_sgpr1_sgpr2_sgpr3'
+  scratchWaveOffsetReg: '$sgpr7'
+  frameOffsetReg:  '$sgpr7'
 body: |
   bb.0:
-    liveins: $sgpr7, $sgpr0_sgpr1_sgpr2_sgpr3
 
     $sgpr33 = S_MOV_B32 $sgpr7
     $sgpr32 = S_MOV_B32 $sgpr33
     $sgpr10 = S_MOV_B32 5
     $sgpr9 = S_MOV_B32 4
     $sgpr8 = S_MOV_B32 3
     BUNDLE implicit-def $sgpr6_sgpr7, implicit-def $sgpr6, implicit-def $sgpr7, implicit-def $scc {
       $sgpr6_sgpr7 = S_GETPC_B64
       $sgpr6 = S_ADD_U32 internal $sgpr6, 0, implicit-def $scc
       $sgpr7 = S_ADDC_U32 internal $sgpr7,0, implicit-def $scc, implicit internal $scc
     }
     $sgpr4 = S_MOV_B32 $sgpr33
     $vgpr0 = V_MOV_B32_e32 $sgpr8, implicit $exec, implicit-def $vgpr0_vgpr1_vgpr2_vgpr3, implicit $sgpr8_sgpr9_sgpr10_sgpr11
     $vgpr1 = V_MOV_B32_e32 $sgpr9, implicit $exec, implicit $sgpr8_sgpr9_sgpr10_sgpr11
     $vgpr2 = V_MOV_B32_e32 $sgpr10, implicit $exec, implicit $sgpr8_sgpr9_sgpr10_sgpr11
     $vgpr3 = V_MOV_B32_e32 $sgpr11, implicit $exec, implicit $sgpr8_sgpr9_sgpr10_sgpr11, implicit $exec
     S_NOP 0, implicit killed $sgpr6_sgpr7, implicit $sgpr0_sgpr1_sgpr2_sgpr3, implicit $sgpr4, implicit killed $vgpr0_vgpr1_vgpr2_vgpr3
     S_ENDPGM 0
 ...
 # CHECK-LABEL: name: func0
 # CHECK-DAG: $sgpr10 = S_MOV_B32 5
 # CHECK-DAG: $sgpr9 = S_MOV_B32 4
 # CHECK-DAG: $sgpr8 = S_MOV_B32 3
-# CHECK-DAG: $sgpr33 = S_MOV_B32 killed $sgpr7
+# CHECK-DAG: $sgpr33 = S_MOV_B32 $sgpr7
 # CHECK: $vgpr0 = V_MOV_B32_e32 $sgpr8, implicit $exec, implicit-def $vgpr0_vgpr1_vgpr2_vgpr3, implicit $sgpr8_sgpr9_sgpr10_sgpr11
 # CHECK: $sgpr32 = S_MOV_B32 $sgpr33
 # CHECK: BUNDLE implicit-def $sgpr6_sgpr7, implicit-def $sgpr6, implicit-def $sgpr7, implicit-def $scc {
 # CHECK:   $sgpr6_sgpr7 = S_GETPC_B64
 # CHECK:   $sgpr6 = S_ADD_U32 internal $sgpr6, 0, implicit-def $scc
 # CHECK:   $sgpr7 = S_ADDC_U32 internal $sgpr7, 0, implicit-def $scc, implicit internal $scc
 # CHECK: }
 # CHECK: $sgpr4 = S_MOV_B32 killed $sgpr33
 # CHECK: $vgpr1 = V_MOV_B32_e32 $sgpr9, implicit $exec, implicit $sgpr8_sgpr9_sgpr10_sgpr11
 # CHECK: $vgpr2 = V_MOV_B32_e32 $sgpr10, implicit $exec, implicit $sgpr8_sgpr9_sgpr10_sgpr11
 # CHECK: $vgpr3 = V_MOV_B32_e32 killed $sgpr11, implicit $exec, implicit $sgpr8_sgpr9_sgpr10_sgpr11, implicit $exec
-# CHECK: S_NOP 0, implicit killed $sgpr6_sgpr7, implicit $sgpr0_sgpr1_sgpr2_sgpr3, implicit $sgpr4, implicit killed $vgpr0_vgpr1_vgpr2_vgpr3
+# CHECK: S_NOP 0, implicit $sgpr6_sgpr7, implicit $sgpr0_sgpr1_sgpr2_sgpr3, implicit killed $sgpr4, implicit killed $vgpr0_vgpr1_vgpr2_vgpr3
 # CHECK: S_ENDPGM 0

test/CodeGen/AMDGPU/rename-independent-subregs-mac-operands.mir

 # RUN: llc -march=amdgcn -verify-machineinstrs -run-pass=simple-register-coalescing,rename-independent-subregs -o - %s | FileCheck -check-prefix=GCN %s
 ---
 
 # GCN-LABEL: name: mac_invalid_operands
 # GCN: undef %18.sub0:vreg_128 = V_MAC_F32_e32 undef %3:vgpr_32, undef %9:vgpr_32, undef %18.sub0, implicit $exec
 
 name:            mac_invalid_operands
 alignment:       0
 exposesReturnsTwice: false
 legalized:       false
 regBankSelected: false
 selected:        false
 tracksRegLiveness: true
+machineFunctionInfo:
+  scratchRSrcReg:  '$sgpr0_sgpr1_sgpr2_sgpr3'
+  scratchWaveOffsetReg: '$sgpr4'
+  frameOffsetReg:  '$sgpr4'
+
 registers:
   - { id: 0, class: vreg_128 }
   - { id: 1, class: vreg_128 }
   - { id: 2, class: sgpr_64 }
   - { id: 3, class: vgpr_32 }
   - { id: 4, class: vgpr_32 }
   - { id: 5, class: vgpr_32 }
   - { id: 6, class: vgpr_32 }

 # GCN: BUFFER_STORE_DWORD_OFFEN %7.sub0, %0,
 name:            vreg_does_not_dominate
 alignment:       0
 exposesReturnsTwice: false
 legalized:       false
 regBankSelected: false
 selected:        false
 tracksRegLiveness: true
+machineFunctionInfo:
+  scratchRSrcReg:  '$sgpr0_sgpr1_sgpr2_sgpr3'
+  scratchWaveOffsetReg: '$sgpr4'
+  frameOffsetReg:  '$sgpr4'
 registers:
   - { id: 0, class: vgpr_32, preferred-register: '' }
   - { id: 1, class: vgpr_32, preferred-register: '' }
   - { id: 2, class: vgpr_32, preferred-register: '' }
   - { id: 3, class: vgpr_32, preferred-register: '' }
   - { id: 4, class: vgpr_32, preferred-register: '' }
   - { id: 5, class: sreg_64, preferred-register: '' }
   - { id: 6, class: vreg_128, preferred-register: '' }
bb.1:
     %6.sub2 = COPY %6.sub0
 
   bb.2:
     BUFFER_STORE_DWORD_OFFEN %6.sub3, %0, $sgpr0_sgpr1_sgpr2_sgpr3, $sgpr4, 12, 0, 0, 0, implicit $exec
     BUFFER_STORE_DWORD_OFFEN %6.sub2, %0, $sgpr0_sgpr1_sgpr2_sgpr3, $sgpr4, 8, 0, 0, 0, implicit $exec
     BUFFER_STORE_DWORD_OFFEN %6.sub1, %0, $sgpr0_sgpr1_sgpr2_sgpr3, $sgpr4, 4, 0, 0, 0, implicit $exec
     BUFFER_STORE_DWORD_OFFEN %6.sub0, %0, $sgpr0_sgpr1_sgpr2_sgpr3, $sgpr4, 0, 0, 0, 0, implicit $exec
     $sgpr30_sgpr31 = COPY %5
-    $sgpr5 = COPY $sgpr5
-    S_SETPC_B64_return $sgpr30_sgpr31, implicit $sgpr5
+    S_SETPC_B64_return $sgpr30_sgpr31
 
 ...
 
 # GCN-LABEL: name: inf_loop_tied_operand
 # GCN: bb.0:
 # GCN-NEXT: undef %2.sub0:vreg_128 = V_MAC_F32_e32 1073741824, undef %0:vgpr_32, undef %2.sub0, implicit $exec
 # GCN-NEXT: dead undef %3.sub1:vreg_128 = COPY %2.sub0
 

test/CodeGen/MIR/AMDGPU/parse-order-reserved-regs.mir

+# RUN: llc -march=amdgcn -run-pass=none -verify-machineinstrs -o - %s | FileCheck %s
+
+# Previously getReservedRegs was called before parsing
+# machineFunctionInfo, but the AMDGPU implementation depends on
+# setting register fields to reserve there. $sgpr50 would then not be
+# reserved, resulting in a verifier error from an undefined register.
+
+---
+# CHECK: machineFunctionInfo:
+# CHECK: isEntryFunction: true
+# CHECK: scratchRSrcReg:  '$sgpr0_sgpr1_sgpr2_sgpr3'
+# CHECK: scratchWaveOffsetReg: '$sgpr50'
+# CHECK: frameOffsetReg:  '$sgpr50'
+# CHECK: renamable $vgpr0 = BUFFER_LOAD_DWORD_OFFEN %stack.0, $sgpr0_sgpr1_sgpr2_sgpr3, $sgpr50, 4, 0, 0, 0, implicit $exec :: (load 4, addrspace 5)
+name: reserve_correct_register
+tracksRegLiveness: true
+machineFunctionInfo:
+  isEntryFunction: true
+  scratchRSrcReg:  '$sgpr0_sgpr1_sgpr2_sgpr3'
+  scratchWaveOffsetReg: '$sgpr50'
+  frameOffsetReg:  '$sgpr50'
+stack:
+  - { id: 0, type: default, offset: 0, size: 4, alignment: 4 }
+
+body:             |
+  bb.0:
+    renamable $vgpr0 = BUFFER_LOAD_DWORD_OFFEN %stack.0, $sgpr0_sgpr1_sgpr2_sgpr3, $sgpr50, 4, 0, 0, 0, implicit $exec :: (load 4, addrspace 5)
+    S_ENDPGM 0
+...