LiveIntervalAnalysis: Fix missing defs in renameDisconnectedComponents().

Fix renameDisconnectedComponents() creating vreg uses that can be
reached from function begin withouthaving a definition (or explicit
live-in). Fix this by inserting IMPLICIT_DEF instruction before
control-flow joins as necessary.

Removes an assert from MachineScheduler because we may now get
additional IMPLICIT_DEF when preparing the scheduling policy.

This fixes the underlying problem of http://llvm.org/PR27705

llvm-svn: 270259

Author: MatzeB

llvm/include/llvm/CodeGen/LiveInterval.h

@@ -896,10 +896,12 @@ namespace llvm {
   class ConnectedSubRegClasses {
     LiveIntervals &LIS;
     MachineRegisterInfo &MRI;
+    const TargetInstrInfo &TII;
 
   public:
-    ConnectedSubRegClasses(LiveIntervals &LIS, MachineRegisterInfo &MRI)
-      : LIS(LIS), MRI(MRI) {}
+    ConnectedSubRegClasses(LiveIntervals &LIS, MachineRegisterInfo &MRI,
+                           const TargetInstrInfo &TII)
+      : LIS(LIS), MRI(MRI), TII(TII) {}
 
     /// Split unrelated subregister components and rename them to new vregs.
     void renameComponents(LiveInterval &LI) const;

llvm/lib/CodeGen/LiveInterval.cpp

@@ -19,13 +19,17 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/CodeGen/LiveInterval.h"
+
+#include "PHIEliminationUtils.h"
 #include "RegisterCoalescer.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/CodeGen/LiveIntervalAnalysis.h"
+#include "llvm/CodeGen/MachineInstrBuilder.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/Target/TargetInstrInfo.h"
 #include "llvm/Target/TargetRegisterInfo.h"
 #include <algorithm>
 using namespace llvm;
@@ -1654,19 +1658,65 @@ void ConnectedSubRegClasses::distribute(const IntEqClasses &Classes,
   }
 }
 
+static bool subRangeLiveAt(const LiveInterval &LI, SlotIndex Pos) {
+  for (const LiveInterval::SubRange &SR : LI.subranges()) {
+    if (SR.liveAt(Pos))
+      return true;
+  }
+  return false;
+}
+
 void ConnectedSubRegClasses::computeMainRangesFixFlags(
     const IntEqClasses &Classes,
     const SmallVectorImpl<SubRangeInfo> &SubRangeInfos,
     const SmallVectorImpl<LiveInterval*> &Intervals) const {
   BumpPtrAllocator &Allocator = LIS.getVNInfoAllocator();
+  const SlotIndexes &Indexes = *LIS.getSlotIndexes();
   for (size_t I = 0, E = Intervals.size(); I < E; ++I) {
-    LiveInterval *LI = Intervals[I];
-    LI->removeEmptySubRanges();
+    LiveInterval &LI = *Intervals[I];
+    unsigned Reg = LI.reg;
+
+    // There must be a def (or live-in) before every use. Splitting vregs may
+    // violate this principle as the splitted vreg may not have a definition on
+    // every path. Fix this by creating IMPLICIT_DEF instruction as necessary.
+    VNInfo::Allocator &VNInfoAllocator = LIS.getVNInfoAllocator();
+    for (const LiveInterval::SubRange &SR : LI.subranges()) {
+      // Search for "PHI" value numbers in the subranges. We must find a live
+      // value in each predecessor block, add an IMPLICIT_DEF where it is
+      // missing.
+      for (unsigned I = 0; I < SR.valnos.size(); ++I) {
+        const VNInfo &VNI = *SR.valnos[I];
+        if (VNI.isUnused() || !VNI.isPHIDef())
+          continue;
+
+        SlotIndex Def = VNI.def;
+        MachineBasicBlock &MBB = *Indexes.getMBBFromIndex(Def);
+        for (MachineBasicBlock *PredMBB : MBB.predecessors()) {
+          SlotIndex PredEnd = Indexes.getMBBEndIdx(PredMBB);
+          if (subRangeLiveAt(LI, PredEnd.getPrevSlot()))
+            continue;
+
+          MachineBasicBlock::iterator InsertPos =
+            llvm::findPHICopyInsertPoint(PredMBB, &MBB, Reg);
+          const MCInstrDesc &MCDesc = TII.get(TargetOpcode::IMPLICIT_DEF);
+          MachineInstrBuilder ImpDef = BuildMI(*PredMBB, InsertPos,
+                                               DebugLoc(), MCDesc, Reg);
+          SlotIndex DefIdx = LIS.InsertMachineInstrInMaps(*ImpDef);
+          SlotIndex RegDefIdx = DefIdx.getRegSlot();
+          for (LiveInterval::SubRange &SR : LI.subranges()) {
+            VNInfo *SRVNI = SR.getNextValue(RegDefIdx, VNInfoAllocator);
+            SR.addSegment(LiveRange::Segment(RegDefIdx, PredEnd, SRVNI));
+          }
+        }
+      }
+    }
+
+    LI.removeEmptySubRanges();
     if (I == 0)
-      LI->clear();
-    LI->constructMainRangeFromSubranges(*LIS.getSlotIndexes(), Allocator);
+      LI.clear();
+    LI.constructMainRangeFromSubranges(*LIS.getSlotIndexes(), Allocator);
 
-    for (MachineOperand &MO : MRI.reg_nodbg_operands(LI->reg)) {
+    for (MachineOperand &MO : MRI.reg_nodbg_operands(Reg)) {
       if (!MO.isDef())
         continue;
       unsigned SubRegIdx = MO.getSubReg();
@@ -1677,12 +1727,12 @@ void ConnectedSubRegClasses::computeMainRangesFixFlags(
       // flags in these cases.
       if (!MO.isUndef()) {
         SlotIndex Pos = LIS.getInstructionIndex(*MO.getParent());
-        if (!LI->liveAt(Pos.getBaseIndex()))
+        if (!LI.liveAt(Pos.getBaseIndex()))
           MO.setIsUndef();
       }
       if (!MO.isDead()) {
         SlotIndex Pos = LIS.getInstructionIndex(*MO.getParent());
-        if (!LI->liveAt(Pos.getDeadSlot()))
+        if (!LI.liveAt(Pos.getDeadSlot()))
           MO.setIsDead();
       }
     }

llvm/lib/CodeGen/LiveIntervalAnalysis.cpp

@@ -1571,7 +1571,7 @@ void LiveIntervals::splitSeparateComponents(LiveInterval &LI,
 }
 
 void LiveIntervals::renameDisconnectedComponents() {
-  ConnectedSubRegClasses SubRegClasses(*this, *MRI);
+  ConnectedSubRegClasses SubRegClasses(*this, *MRI, *TII);
 
   // Iterate over all vregs. Note that we query getNumVirtRegs() the newly
   // created vregs end up with higher numbers but do not need to be visited as

llvm/lib/CodeGen/MachineScheduler.cpp

@@ -444,23 +444,20 @@ void MachineSchedulerBase::scheduleRegions(ScheduleDAGInstrs &Scheduler,
     //
     // MBB::size() uses instr_iterator to count. Here we need a bundle to count
     // as a single instruction.
-    unsigned RemainingInstrs = std::distance(MBB->begin(), MBB->end());
     for(MachineBasicBlock::iterator RegionEnd = MBB->end();
         RegionEnd != MBB->begin(); RegionEnd = Scheduler.begin()) {
 
       // Avoid decrementing RegionEnd for blocks with no terminator.
       if (RegionEnd != MBB->end() ||
           isSchedBoundary(&*std::prev(RegionEnd), &*MBB, MF, TII)) {
         --RegionEnd;
-        // Count the boundary instruction.
-        --RemainingInstrs;
       }
 
       // The next region starts above the previous region. Look backward in the
       // instruction stream until we find the nearest boundary.
       unsigned NumRegionInstrs = 0;
       MachineBasicBlock::iterator I = RegionEnd;
-      for(;I != MBB->begin(); --I, --RemainingInstrs) {
+      for (;I != MBB->begin(); --I) {
         if (isSchedBoundary(&*std::prev(I), &*MBB, MF, TII))
           break;
         if (!I->isDebugValue())
@@ -483,8 +480,7 @@ void MachineSchedulerBase::scheduleRegions(ScheduleDAGInstrs &Scheduler,
             << "\n  From: " << *I << "    To: ";
             if (RegionEnd != MBB->end()) dbgs() << *RegionEnd;
             else dbgs() << "End";
-            dbgs() << " RegionInstrs: " << NumRegionInstrs
-            << " Remaining: " << RemainingInstrs << "\n");
+            dbgs() << " RegionInstrs: " << NumRegionInstrs << '\n');
       if (DumpCriticalPathLength) {
         errs() << MF->getName();
         errs() << ":BB# " << MBB->getNumber();
@@ -502,7 +498,6 @@ void MachineSchedulerBase::scheduleRegions(ScheduleDAGInstrs &Scheduler,
       // scheduler for the top of it's scheduled region.
       RegionEnd = Scheduler.begin();
     }
-    assert(RemainingInstrs == 0 && "Instruction count mismatch!");
     Scheduler.finishBlock();
     // FIXME: Ideally, no further passes should rely on kill flags. However,
     // thumb2 size reduction is currently an exception, so the PostMIScheduler

llvm/test/CodeGen/AMDGPU/rename-disconnected-bug.ll

@@ -0,0 +1,33 @@
+; RUN: llc -verify-machineinstrs -o /dev/null %s
+; Check that renameDisconnectedComponents() does not create vregs without a
+; definition on every path (there should at least be IMPLICIT_DEF instructions).
+target triple = "amdgcn--"
+
+define void @func() {
+B0:
+  br i1 undef, label %B1, label %B2
+
+B1:
+  br label %B2
+
+B2:
+  %v0 = phi <4 x float> [ zeroinitializer, %B1 ], [ <float 0.0, float 0.0, float 0.0, float undef>, %B0 ]
+  br i1 undef, label %B20.1, label %B20.2
+
+B20.1:
+  br label %B20.2
+
+B20.2:
+  %v2 = phi <4 x float> [ zeroinitializer, %B20.1 ], [ %v0, %B2 ]
+  br i1 undef, label %B30.1, label %B30.2
+
+B30.1:
+  %sub = fsub <4 x float> %v2, undef
+  br label %B30.2
+
+B30.2:
+  %v3 = phi <4 x float> [ %sub, %B30.1 ], [ %v2, %B20.2 ]
+  %ve0 = extractelement <4 x float> %v3, i32 0
+  store float %ve0, float addrspace(3)* undef, align 4
+  ret void
+}