[MachineVerifier] Verify LiveIntervals for PHIs

[critson]

Summary

Implement basic support for verifying LiveIntervals for PHIs.

[foad]

Interesting. Currently I don't think anything in LLVM tries to use LiveIntervals in SSA form, does it? Even -early-live-intervals only creates them just after PHIElimination.

llvm/lib/CodeGen/MachineVerifier.cpp

2521

Could the phi and non-phi cases be commoned some more? Is there any good reason to use SR.getVNInfoAt in one case vs SR.Query in the other?

[critson]

• Simplify implementation to reuse existing query logic

[critson]

In D156872#4553407, @foad wrote:

Interesting. Currently I don't think anything in LLVM tries to use LiveIntervals in SSA form, does it? Even -early-live-intervals only creates them just after PHIElimination.

PHIElimination has code to preserve LiveIntervals, although it is incorrect/incomplete which I am currently fixing.
The code for '-early-live-intervals` also suggests the intention is for it to be run before PHIEliminination.

[arsenm]

This probably fixes some of the issues I had getting llvm-reduce to always use LiveIntervals

llvm/lib/CodeGen/MachineVerifier.cpp

2529

Don't need parentheses around compare

llvm/test/CodeGen/AMDGPU/split-mbb-lis-subrange.mir

6–11

Don't need the IR section

[critson]

• Address reviewer comments

llvm/lib/CodeGen/MachineVerifier.cpp

MachineVerifier::visitMachineOperand(const MachineOperand *MO, unsigned MONum) {
   }
 }
 
 void MachineVerifier::checkLivenessAtUse(const MachineOperand *MO,
                                          unsigned MONum, SlotIndex UseIdx,
                                          const LiveRange &LR,
                                          Register VRegOrUnit,
                                          LaneBitmask LaneMask) {
+  const MachineInstr *MI = MO->getParent();
   LiveQueryResult LRQ = LR.Query(UseIdx);
+  bool HasValue = LRQ.valueIn() || (MI->isPHI() && LRQ.valueOut());
   // Check if we have a segment at the use, note however that we only need one
   // live subregister range, the others may be dead.
-  if (!LRQ.valueIn() && LaneMask.none()) {
+  if (!HasValue && LaneMask.none()) {
     report("No live segment at use", MO, MONum);
     report_context_liverange(LR);
     report_context_vreg_regunit(VRegOrUnit);
     report_context(UseIdx);
   }
   if (MO->isKill() && !LRQ.isKill()) {
     report("Live range continues after kill flag", MO, MONum);
     report_context_liverange(LR);
if (LiveVars && Reg.isVirtual() && MO->isKill() &&
         !MI->isBundledWithPred()) {
       LiveVariables::VarInfo &VI = LiveVars->getVarInfo(Reg);
       if (!is_contained(VI.Kills, MI))
         report("Kill missing from LiveVariables", MO, MONum);
     }
 
     // Check LiveInts liveness and kill.
     if (LiveInts && !LiveInts->isNotInMIMap(*MI)) {
-      SlotIndex UseIdx = LiveInts->getInstructionIndex(*MI);
+      SlotIndex UseIdx;
+      if (MI->isPHI()) {
+        // PHI use occurs on the edge, so check for live out here instead.
+        UseIdx = LiveInts->getMBBEndIdx(
+          MI->getOperand(MONum + 1).getMBB()).getPrevSlot();
+      } else {
+        UseIdx = LiveInts->getInstructionIndex(*MI);
+      }
       // Check the cached regunit intervals.
       if (Reg.isPhysical() && !isReserved(Reg)) {
         for (MCRegUnit Unit : TRI->regunits(Reg.asMCReg())) {
           if (MRI->isReservedRegUnit(Unit))
             continue;
           if (const LiveRange *LR = LiveInts->getCachedRegUnit(Unit))
             checkLivenessAtUse(MO, MONum, UseIdx, *LR, Unit);
         }
       }
 
       if (Reg.isVirtual()) {
         // This is a virtual register interval.
         checkLivenessAtUse(MO, MONum, UseIdx, *LI, Reg);
 
         if (LI->hasSubRanges() && !MO->isDef()) {
           LaneBitmask MOMask = SubRegIdx != 0
                                    ? TRI->getSubRegIndexLaneMask(SubRegIdx)
                                    : MRI->getMaxLaneMaskForVReg(Reg);
           LaneBitmask LiveInMask;
           for (const LiveInterval::SubRange &SR : LI->subranges()) {
             if ((MOMask & SR.LaneMask).none())
               continue;
             checkLivenessAtUse(MO, MONum, UseIdx, SR, Reg, SR.LaneMask);
             LiveQueryResult LRQ = SR.Query(UseIdx);
-            if (LRQ.valueIn())
+            if (LRQ.valueIn() || (MI->isPHI() && LRQ.valueOut()))
               LiveInMask |= SR.LaneMask;
           }
           // At least parts of the register has to be live at the use.
           if ((LiveInMask & MOMask).none()) {
             report("No live subrange at use", MO, MONum);
             report_context(*LI);
             report_context(UseIdx);
           }
+          // For PHIs all lanes should be live
+          if (MI->isPHI() && (LiveInMask != MOMask)) {
+            report("Not all lanes of PHI source live at use", MO, MONum);
+            report_context(*LI);
+            report_context(UseIdx);
+          }
         }
       }
     }
 
     // Use of a dead register.
     if (!regsLive.count(Reg)) {
       if (Reg.isPhysical()) {
         // Reserved registers may be used even when 'dead'.

llvm/test/CodeGen/AMDGPU/split-mbb-lis-subrange.mir

+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 2
+# RUN: llc -march=amdgcn -mcpu=gfx1100 -verify-machineinstrs -run-pass liveintervals -o - %s | FileCheck -check-prefixes=GCN %s
+
+# This test simply checks that liveintervals pass verification.
+
+--- |
+  define amdgpu_kernel void @split_critical_edge_subranges() {
+    entry:
+      ret void
+  }
+---
+name: split_critical_edge_subranges
+tracksRegLiveness: true
+body:             |
+  ; GCN-LABEL: name: split_critical_edge_subranges
+  ; GCN: bb.0:
+  ; GCN-NEXT:   successors: %bb.3(0x40000000), %bb.1(0x40000000)
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT:   %coord:vreg_64 = IMPLICIT_DEF
+  ; GCN-NEXT:   %desc:sgpr_256 = IMPLICIT_DEF
+  ; GCN-NEXT:   %c0:sreg_32 = IMPLICIT_DEF
+  ; GCN-NEXT:   %c1:sreg_32 = IMPLICIT_DEF
+  ; GCN-NEXT:   %const:vgpr_32 = IMPLICIT_DEF
+  ; GCN-NEXT:   %load:vreg_64 = IMAGE_LOAD_V2_V2_gfx11 %coord, %desc, 3, 1, -1, 0, 0, 0, 0, 0, 0, implicit $exec :: (dereferenceable load (s64), align 16, addrspace 4)
+  ; GCN-NEXT:   %s0a:vgpr_32 = COPY %load.sub0
+  ; GCN-NEXT:   %s0b:vgpr_32 = COPY %load.sub1
+  ; GCN-NEXT:   S_CMP_EQ_U32 %c0, %c1, implicit-def $scc
+  ; GCN-NEXT:   S_CBRANCH_SCC1 %bb.3, implicit $scc
+  ; GCN-NEXT:   S_BRANCH %bb.1
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT: bb.1:
+  ; GCN-NEXT:   successors: %bb.3(0x80000000)
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT:   %s0c:vgpr_32 = V_ADD_F32_e64 0, %s0a, 0, %const, 0, 0, implicit $mode, implicit $exec
+  ; GCN-NEXT:   %s0d:vgpr_32 = V_ADD_F32_e64 0, %s0b, 0, %const, 0, 0, implicit $mode, implicit $exec
+  ; GCN-NEXT:   S_BRANCH %bb.3
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT: bb.2:
+  ; GCN-NEXT:   S_NOP 0
+  ; GCN-NEXT:   S_ENDPGM 0
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT: bb.3:
+  ; GCN-NEXT:   successors: %bb.4(0x80000000)
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT:   %phi0:vgpr_32 = PHI %s0a, %bb.0, %s0c, %bb.1
+  ; GCN-NEXT:   %phi1:vgpr_32 = PHI %s0b, %bb.0, %s0d, %bb.1
+  ; GCN-NEXT:   S_BRANCH %bb.4
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT: bb.4:
+  ; GCN-NEXT:   S_ENDPGM 0, implicit %phi0, implicit %phi1
+  bb.0:
+    %coord:vreg_64 = IMPLICIT_DEF
+    %desc:sgpr_256 = IMPLICIT_DEF
+    %c0:sreg_32 = IMPLICIT_DEF
+    %c1:sreg_32 = IMPLICIT_DEF
+    %const:vgpr_32 = IMPLICIT_DEF
+    %load:vreg_64 = IMAGE_LOAD_V2_V2_gfx11 %coord:vreg_64, killed %desc:sgpr_256, 3, 1, -1, 0, 0, 0, 0, 0, 0, implicit $exec :: (dereferenceable load (s64), align 16, addrspace 4)
+    %s0a:vgpr_32 = COPY %load.sub0:vreg_64
+    %s0b:vgpr_32 = COPY %load.sub1:vreg_64
+    S_CMP_EQ_U32 killed %c0:sreg_32, killed %c1:sreg_32, implicit-def $scc
+    S_CBRANCH_SCC1 %bb.3, implicit $scc
+    S_BRANCH %bb.1
+
+  bb.1:
+    %s0c:vgpr_32 = V_ADD_F32_e64 0, %s0a:vgpr_32, 0, %const:vgpr_32, 0, 0, implicit $mode, implicit $exec
+    %s0d:vgpr_32 = V_ADD_F32_e64 0, %s0b:vgpr_32, 0, %const:vgpr_32, 0, 0, implicit $mode, implicit $exec
+    S_BRANCH %bb.3
+
+  bb.2:
+    S_NOP 0
+    S_ENDPGM 0
+
+  bb.3:
+    %phi0:vgpr_32 = PHI %s0a:vgpr_32, %bb.0, %s0c:vgpr_32, %bb.1
+    %phi1:vgpr_32 = PHI %s0b:vgpr_32, %bb.0, %s0d:vgpr_32, %bb.1
+    S_BRANCH %bb.4
+
+  bb.4:
+    S_ENDPGM 0, implicit %phi0:vgpr_32, implicit %phi1:vgpr_32
+...