Potential bug in MachineVerifier::checkLiveness()

Hi,

I’m wondering if this is a latent bug in MachineVerifier::checkLiveness(). If the candidate operand is a PHI’s input, the live range ends at the tail of the predecessor block. This is done by LiveIntervalCalc::extendToUses(). But checkLivelessAtUse()
expects the PHI’s input lives at the PHI instruction.

It seems this issue is not exposed on targets like AArch64 because LiveIntervals is called after PHI elimination. But it could be a problem if LiveIntervals is called before PHI elimination.

void MachineVerifier::checkLiveness(const MachineOperand *MO, unsigned MONum) {
    ...
    // Check LiveInts liveness and kill.
    if (LiveInts && !LiveInts->isNotInMIMap(*MI)) {
      SlotIndex UseIdx = LiveInts->getInstructionIndex(*MI);                                 
      // Check the cached regunit intervals.
      if (Reg.isPhysical() && !isReserved(Reg)) {
        for (MCRegUnitIterator Units(Reg.asMCReg(), TRI); Units.isValid();
             ++Units) {
          if (MRI->isReservedRegUnit(*Units))
            continue;
          if (const LiveRange *LR = LiveInts->getCachedRegUnit(*Units))
            checkLivenessAtUse(MO, MONum, UseIdx, *LR, *Units);
        }
      }

      if (Reg.isVirtual()) {
        // This is a virtual register interval.
        checkLivenessAtUse(MO, MONum, UseIdx, *LI, Reg);

In LiveIntervalCalc::extendToUses():

    // Determine the actual place of the use.
    const MachineInstr *MI = MO.getParent();
    unsigned OpNo = (&MO - &MI->getOperand(0));
    SlotIndex UseIdx;
    if (MI->isPHI()) {
      assert(!MO.isDef() && "Cannot handle PHI def of partial register.");
      // The actual place where a phi operand is used is the end of the pred
      // MBB. PHI operands are paired: (Reg, PredMBB).
      UseIdx = Indexes->getMBBEndIdx(MI->getOperand(OpNo +
1).getMBB());           
    } else {

@jayfoad @Matthew
I understand you have expertise on this part of code. Could you help to confirm? Thanks.

Perhaps it is a latent bug. Currently LiveIntervals is only run after PHIElimination. See TargetPassConfig::addOptimizedRegAlloc:

  addPass(&PHIEliminationID);

  // Eventually, we want to run LiveIntervals before PHI elimination.
  if (EarlyLiveIntervals)
    addPass(&LiveIntervalsID);

N.B. EarlyLiveIntervals is not enabled by default yet. See [llvm-dev] Machine verification and LiveIntervals

I didn’t check in detail, but at a first glance it does indeed look like MachineVerifier has no code to deal with the odd behavior of PHI instructions, which likely nobody cared/noticed because we didn’t have LiveIntervals computed at points where PHI instructions where still around.