[IfCvt] Don't use pristine register for counting liveins for predicated instructions.

[dmgreen]

Summary

The test case here hits machine verifier problems. There are volatile long loads that the results of do not get used, loading into two dead registers. IfCvt will predicate them and as it does will add implicit uses of the predicating registers due to thinking they are live in. As nothing has used the register, the machine verifier disagrees that they are really live and we end up with a failure.

The registers come from Pristine regs that LivePhysRegs counts as live. This patch adds a addLiveInsNoPristines method to be used instead in IfCvt, so that only really live in regs need to be added as implicit operands.

[dmgreen]

llvm/lib/CodeGen/IfConversion.cpp

1518

This is what is really being fixed here, with the Reg being counted as live when it should not be.

[dmgreen]

Ping

[dmgreen]

Ping

[efriedma]

Let me see if I understand what's going on here:

1. You have a load with a dead result register.
2. ifcvt doesn't care that it's dead; it treats it like a live result.
3. Since we're treating the result as live, if the input is live, we need to add it as an implicit use.
4. ifcvt thinks the pristine register is live, but it really isn't.

I guess this is the right fix, then; LGTM.

That said, ifconversion should probably be fixed at some point to step backwards, rather than forwards. Stepping through a block forwards is sort of deprecated.

llvm/include/llvm/CodeGen/LivePhysRegs.h

public:
   void stepForward(const MachineInstr &MI,
         SmallVectorImpl<std::pair<MCPhysReg, const MachineOperand*>> &Clobbers);
 
   /// Adds all live-in registers of basic block \p MBB.
   /// Live in registers are the registers in the blocks live-in list and the
   /// pristine registers.
   void addLiveIns(const MachineBasicBlock &MBB);
 
+  /// Adds all live-in registers of basic block \p MBB but skips pristine
+  /// registers.
+  void addLiveInsNoPristines(const MachineBasicBlock &MBB);
+
   /// Adds all live-out registers of basic block \p MBB.
   /// Live out registers are the union of the live-in registers of the successor
   /// blocks and pristine registers. Live out registers of the end block are the
   /// callee saved registers.
   /// If a register is not added by this method, it is guaranteed to not be
   /// live out from MBB, although a sub-register may be. This is true
   /// both before and after regalloc.
   void addLiveOuts(const MachineBasicBlock &MBB);

llvm/lib/CodeGen/IfConversion.cpp

if (Op.isRegMask()) {
       // use to read from.
       // For the register allocator to have allocated a register clobbered
       // by the call which is used later, it must be the case that
       // the call doesn't return.
       MIB.addReg(Reg, RegState::Implicit | RegState::Define);
       continue;
     }
     if (LiveBeforeMI.count(Reg))
       MIB.addReg(Reg, RegState::Implicit);
     else {
       bool HasLiveSubReg = false;
       for (MCSubRegIterator S(Reg, TRI); S.isValid(); ++S) {
         if (!LiveBeforeMI.count(*S))
           continue;
         HasLiveSubReg = true;
         break;
       }
if (Kind == ICSimpleFalse)
     if (TII->reverseBranchCondition(Cond))
       llvm_unreachable("Unable to reverse branch condition!");
 
   Redefs.init(*TRI);
 
   if (MRI->tracksLiveness()) {
     // Initialize liveins to the first BB. These are potentially redefined by
     // predicated instructions.
-    Redefs.addLiveIns(CvtMBB);
-    Redefs.addLiveIns(NextMBB);
+    Redefs.addLiveInsNoPristines(CvtMBB);
+    Redefs.addLiveInsNoPristines(NextMBB);
   }
 
   // Remove the branches from the entry so we can add the contents of the true
   // block to it.
   BBI.NonPredSize -= TII->removeBranch(*BBI.BB);
 
   if (CvtMBB.pred_size() > 1) {
     // Copy instructions in the true block, predicate them, and add them to
if (reverseBranchCondition(*CvtBBI)) {
       }
     }
   }
 
   // Initialize liveins to the first BB. These are potentially redefined by
   // predicated instructions.
   Redefs.init(*TRI);
   if (MRI->tracksLiveness()) {
-    Redefs.addLiveIns(CvtMBB);
-    Redefs.addLiveIns(NextMBB);
+    Redefs.addLiveInsNoPristines(CvtMBB);
+    Redefs.addLiveInsNoPristines(NextMBB);
   }
 
   bool HasEarlyExit = CvtBBI->FalseBB != nullptr;
   BranchProbability CvtNext, CvtFalse, BBNext, BBCvt;
 
   if (HasEarlyExit) {
     // Get probabilities before modifying CvtMBB and BBI.BB.
     CvtNext = MBPI->getEdgeProbability(&CvtMBB, &NextMBB);
bool IfConverter::IfConvertDiamondCommon(
   // Initialize the Redefs:
   // - BB2 live-in regs need implicit uses before being redefined by BB1
   //   instructions.
   // - BB1 live-out regs need implicit uses before being redefined by BB2
   //   instructions. We start with BB1 live-ins so we have the live-out regs
   //   after tracking the BB1 instructions.
   Redefs.init(*TRI);
   if (MRI->tracksLiveness()) {
-    Redefs.addLiveIns(MBB1);
-    Redefs.addLiveIns(MBB2);
+    Redefs.addLiveInsNoPristines(MBB1);
+    Redefs.addLiveInsNoPristines(MBB2);
   }
 
   // Remove the duplicated instructions at the beginnings of both paths.
   // Skip dbg_value instructions.
   MachineBasicBlock::iterator DI1 = MBB1.getFirstNonDebugInstr();
   MachineBasicBlock::iterator DI2 = MBB2.getFirstNonDebugInstr();
   BBI1->NonPredSize -= NumDups1;
   BBI2->NonPredSize -= NumDups1;

llvm/lib/CodeGen/LivePhysRegs.cpp

 }
 
 void LivePhysRegs::addLiveIns(const MachineBasicBlock &MBB) {
   const MachineFunction &MF = *MBB.getParent();
   addPristines(MF);
   addBlockLiveIns(MBB);
 }
 
+void LivePhysRegs::addLiveInsNoPristines(const MachineBasicBlock &MBB) {
+  addBlockLiveIns(MBB);
+}
+
 void llvm::computeLiveIns(LivePhysRegs &LiveRegs,
                           const MachineBasicBlock &MBB) {
   const MachineFunction &MF = *MBB.getParent();
   const MachineRegisterInfo &MRI = MF.getRegInfo();
   const TargetRegisterInfo &TRI = *MRI.getTargetRegisterInfo();
   LiveRegs.init(TRI);
   LiveRegs.addLiveOutsNoPristines(MBB);
   for (const MachineInstr &MI : make_range(MBB.rbegin(), MBB.rend()))

llvm/test/CodeGen/ARM/ldrd_ifcvt.ll

+; RUN: llc -mtriple=armv7a-none-eabi %s -o -  -verify-machineinstrs | FileCheck %s
+; RUN: llc -mtriple=thumbv7a-none-eabi %s -o -  -verify-machineinstrs | FileCheck %s
+; RUN: llc -mtriple=thumbv7m-none-eabi %s -o -  -verify-machineinstrs | FileCheck %s
+
+; Check we do not hit verifier errors from ifcvting volatile ldrd's
+; CHECK: ldrdne
+; CHECK: ldrdne
+; CHECK: ldrdne
+; CHECK: ldrdne
+
+define void @c(i64* %b) noreturn nounwind {
+entry:
+  br label %for.cond
+
+for.cond:                                         ; preds = %land.end.3, %entry
+  %a.0 = phi i32 [ 0, %entry ], [ %conv2.3, %land.end.3 ]
+  %tobool.not = icmp eq i32 %a.0, 0
+  br i1 %tobool.not, label %land.end, label %land.rhs
+
+land.rhs:                                         ; preds = %for.cond
+  %0 = load volatile i64, i64* %b, align 8
+  br label %land.end
+
+land.end:                                         ; preds = %land.rhs, %for.cond
+  %sub.i = add nuw nsw i32 %a.0, 65533
+  %conv2 = and i32 %sub.i, 65535
+  %tobool.not.1 = icmp eq i32 %conv2, 0
+  br i1 %tobool.not.1, label %land.end.1, label %land.rhs.1
+
+land.rhs.1:                                       ; preds = %land.end
+  %1 = load volatile i64, i64* %b, align 8
+  br label %land.end.1
+
+land.end.1:                                       ; preds = %land.rhs.1, %land.end
+  %sub.i.1 = add nuw nsw i32 %a.0, 65530
+  %conv2.1 = and i32 %sub.i.1, 65535
+  %tobool.not.2 = icmp eq i32 %conv2.1, 0
+  br i1 %tobool.not.2, label %land.end.2, label %land.rhs.2
+
+land.rhs.2:                                       ; preds = %land.end.1
+  %2 = load volatile i64, i64* %b, align 8
+  br label %land.end.2
+
+land.end.2:                                       ; preds = %land.rhs.2, %land.end.1
+  %sub.i.2 = add nuw nsw i32 %a.0, 65527
+  %conv2.2 = and i32 %sub.i.2, 65535
+  %tobool.not.3 = icmp eq i32 %conv2.2, 0
+  br i1 %tobool.not.3, label %land.end.3, label %land.rhs.3
+
+land.rhs.3:                                       ; preds = %land.end.2
+  %3 = load volatile i64, i64* %b, align 8
+  br label %land.end.3
+
+land.end.3:                                       ; preds = %land.rhs.3, %land.end.2
+  %sub.i.3 = add nuw nsw i32 %a.0, 65524
+  %conv2.3 = and i32 %sub.i.3, 65535
+  br label %for.cond
+}