Rebase. Updates per latest review.

Author: JonPsson1

llvm/lib/Target/SystemZ/SystemZInstrInfo.cpp

@@ -2162,26 +2162,20 @@ bool SystemZInstrInfo::isLoadAndTestAsCmp(const MachineInstr &MI) const {
   return (MI.getOpcode() == SystemZ::LTEBR ||
           MI.getOpcode() == SystemZ::LTDBR ||
           MI.getOpcode() == SystemZ::LTXBR) &&
-    MI.getOperand(0).isDead();
+         MI.getOperand(0).isDead();
 }
 
 bool SystemZInstrInfo::isCompareZero(const MachineInstr &Compare) const {
   if (isLoadAndTestAsCmp(Compare))
     return true;
   return Compare.isCompare() && Compare.getNumExplicitOperands() == 2 &&
-    Compare.getOperand(1).isImm() && Compare.getOperand(1).getImm() == 0;
+         Compare.getOperand(1).isImm() && Compare.getOperand(1).getImm() == 0;
 }
 
-unsigned SystemZInstrInfo::
-getCompareSourceReg(const MachineInstr &Compare) const {
-  unsigned reg = 0;
-  if (Compare.isCompare())
-    reg = Compare.getOperand(0).getReg();
-  else if (isLoadAndTestAsCmp(Compare))
-    reg = Compare.getOperand(1).getReg();
-  assert(reg);
-
-  return reg;
+Register
+SystemZInstrInfo::getCompareSourceReg(const MachineInstr &Compare) const {
+  assert(isCompareZero(Compare) && "Expected a compare with 0.");
+  return Compare.getOperand(isLoadAndTestAsCmp(Compare) ? 1 : 0).getReg();
 }
 
 bool SystemZInstrInfo::

llvm/lib/Target/SystemZ/SystemZInstrInfo.h

@@ -365,7 +365,7 @@ class SystemZInstrInfo : public SystemZGenInstrInfo {
 
   // Return the source register of Compare, which is the unknown value
   // being tested.
-  unsigned getCompareSourceReg(const MachineInstr &Compare) const;
+  Register getCompareSourceReg(const MachineInstr &Compare) const;
 
   // Try to find all CC users of the compare instruction (MBBI) and update
   // all of them to maintain equivalent behavior after swapping the compare

llvm/lib/Target/SystemZ/SystemZMachineScheduler.cpp

@@ -57,15 +57,29 @@ void SystemZPreRASchedStrategy::initializePrioRegClasses(
   }
 }
 
-void SystemZPreRASchedStrategy::VRegSet::dump(std::string Msg) {
-  dbgs() << Msg.c_str();
+void SystemZPreRASchedStrategy::VRegSet::insert(Register Reg) {
+  assert(Reg.isVirtual());
+  Regs.insert(Reg);
+}
+
+void SystemZPreRASchedStrategy::VRegSet::erase(Register Reg) {
+  assert(Reg.isVirtual());
+  Regs.erase(Reg);
+}
+
+bool SystemZPreRASchedStrategy::VRegSet::count(Register Reg) const {
+  assert(Reg.isVirtual());
+  return Regs.count(Reg);
+}
+
+void SystemZPreRASchedStrategy::VRegSet::dump() const {
   bool First = true;
-  for (auto R : *this) {
+  for (auto R : Regs) {
     if (!First)
       dbgs() << ", ";
     else
       First = false;
-    dbgs() << "%" << R.virtRegIndex();
+    dbgs() << printReg(R);
   }
   dbgs() << "\n";
 }
@@ -109,8 +123,8 @@ void SystemZPreRASchedStrategy::initializeStoresGroup() {
         return;
       if (IsStore)
         StoresGroup.insert(SU);
-    }
-    else if (IsStore && !StoresGroup.empty() && SU->getDepth() == CurrMaxDepth) {
+    } else if (IsStore && !StoresGroup.empty() &&
+               SU->getDepth() == CurrMaxDepth) {
       // The group members should all have the same opcode.
       if ((*StoresGroup.begin())->getInstr()->getOpcode() != MI->getOpcode()) {
         StoresGroup.clear();
@@ -142,9 +156,8 @@ static int biasPhysRegExtra(const SUnit *SU) {
   return 0;
 }
 
-int SystemZPreRASchedStrategy::
-computeSULivenessScore(SchedCandidate &C, ScheduleDAGMILive *DAG,
-                       SchedBoundary *Zone) const {
+int SystemZPreRASchedStrategy::computeSULivenessScore(
+    SchedCandidate &C, ScheduleDAGMILive *DAG, SchedBoundary *Zone) const {
   // Not all data deps are modelled around the SUnit - some data edges near
   // boundaries are missing: Look directly at the MI operands instead.
   const SUnit *SU = C.SU;
@@ -246,22 +259,24 @@ bool SystemZPreRASchedStrategy::tryCandidate(SchedCandidate &Cand,
       return TryCand.Reason != NoCand;
 
     // Don't extend the scheduled latency.
-    if (ShouldReduceLatency && TryCand.SU->getHeight() != Cand.SU->getHeight() &&
+    if (ShouldReduceLatency &&
+        TryCand.SU->getHeight() != Cand.SU->getHeight() &&
         (std::max(TryCand.SU->getHeight(), Cand.SU->getHeight()) >
          Zone->getScheduledLatency())) {
-      unsigned HigherSUDepth = TryCand.SU->getHeight() < Cand.SU->getHeight() ?
-        Cand.SU->getDepth() : TryCand.SU->getDepth();
+      unsigned HigherSUDepth = TryCand.SU->getHeight() < Cand.SU->getHeight()
+                                   ? Cand.SU->getDepth()
+                                   : TryCand.SU->getDepth();
       if (HigherSUDepth != getRemLat(Zone) &&
-          tryLess(TryCand.SU->getHeight(), Cand.SU->getHeight(),
-                  TryCand, Cand, GenericSchedulerBase::BotHeightReduce)) {
+          tryLess(TryCand.SU->getHeight(), Cand.SU->getHeight(), TryCand, Cand,
+                  GenericSchedulerBase::BotHeightReduce)) {
         return TryCand.Reason != NoCand;
       }
     }
   }
 
   // Weak edges are for clustering and other constraints.
-  if (tryLess(TryCand.SU->WeakSuccsLeft, Cand.SU->WeakSuccsLeft,
-              TryCand, Cand, Weak))
+  if (tryLess(TryCand.SU->WeakSuccsLeft, Cand.SU->WeakSuccsLeft, TryCand, Cand,
+              Weak))
     return TryCand.Reason != NoCand;
 
   // Fall through to original instruction order.
@@ -361,17 +376,22 @@ void SystemZPreRASchedStrategy::initialize(ScheduleDAGMI *dag) {
   LLVM_DEBUG(if (ShouldReduceLatency) dbgs() << "Latency scheduling enabled.\n";
              else dbgs() << "Latency scheduling disabled.\n";);
 
-  // Find the registers that are live at the bottom, before scheduling.
+  // Find the registers used in the region that are live out.
   LiveRegs.clear();
-  for (unsigned I = 0, E = DAG->MRI.getNumVirtRegs(); I != E; ++I) {
-    Register VirtReg = Register::index2VirtReg(I);
-    const LiveInterval &LI = DAG->getLIS()->getInterval(VirtReg);
-    LiveQueryResult LRQ = LI.Query(
-        DAG->getLIS()->getInstructionIndex(*DAG->SUnits.back().getInstr()));
-    if (LRQ.valueOut())
-      LiveRegs.insert(VirtReg);
+  std::set<Register> Visited;
+  for (unsigned Idx = 0, End = DAG->SUnits.size(); Idx != End; ++Idx) {
+    const MachineInstr *MI = DAG->SUnits[Idx].getInstr();
+    for (auto &MO : MI->explicit_operands())
+      if (MO.isReg() && MO.getReg().isVirtual() &&
+          Visited.insert(MO.getReg()).second) {
+        const LiveInterval &LI = DAG->getLIS()->getInterval(MO.getReg());
+        LiveQueryResult LRQ = LI.Query(
+            DAG->getLIS()->getInstructionIndex(*DAG->SUnits.back().getInstr()));
+        if (LRQ.valueOut())
+          LiveRegs.insert(MO.getReg());
+      }
   }
-  LLVM_DEBUG(LiveRegs.dump("Live out at bottom: "););
+  LLVM_DEBUG(dbgs() << "Live out at bottom: "; LiveRegs.dump(););
 
   // If MI uses the register it defines, record it one time here.
   IsRedefining = std::vector<bool>(DAG->SUnits.size(), false);
@@ -395,7 +415,7 @@ void SystemZPreRASchedStrategy::schedNode(SUnit *SU, bool IsTopNode) {
   if (TinyRegion)
     return;
 
-  LLVM_DEBUG(LiveRegs.dump("Live regs was: "););
+  LLVM_DEBUG(dbgs() << "Live regs was: "; LiveRegs.dump(););
 
   if (!FirstStoreInGroupScheduled && StoresGroup.count(SU))
     FirstStoreInGroupScheduled = true;

llvm/lib/Target/SystemZ/SystemZMachineScheduler.h

@@ -50,16 +50,19 @@ class SystemZPreRASchedStrategy : public GenericScheduler {
   // Num instructions left to schedule.
   unsigned NumLeft;
 
-  // Tru if latency scheduling is enabled.
+  // True if latency scheduling is enabled.
   bool ShouldReduceLatency;
 
   // Keep track of currently live registers.
-  struct VRegSet : std::set<Register> {
-    void dump(std::string Msg);
-    size_type count(Register Reg) const {
-      assert(Reg.isVirtual());
-      return std::set<Register>::count(Reg);
-    }
+  class VRegSet {
+    std::set<Register> Regs;
+
+  public:
+    void clear() { Regs.clear(); }
+    void insert(Register Reg);
+    void erase(Register Reg);
+    bool count(Register Reg) const;
+    void dump() const;
   } LiveRegs;
 
   // True if MI is also using the register it defines.
@@ -70,7 +73,7 @@ class SystemZPreRASchedStrategy : public GenericScheduler {
   unsigned getRemLat(SchedBoundary *Zone) const;
 
   // A large group of stores at the bottom is spread upwards.
-  std::set<const SUnit*> StoresGroup;
+  std::set<const SUnit *> StoresGroup;
   bool FirstStoreInGroupScheduled;
   void initializeStoresGroup();
 
@@ -86,7 +89,8 @@ class SystemZPreRASchedStrategy : public GenericScheduler {
                     SchedBoundary *Zone) const override;
 
 public:
-  SystemZPreRASchedStrategy(const MachineSchedContext *C) : GenericScheduler(C) {
+  SystemZPreRASchedStrategy(const MachineSchedContext *C)
+      : GenericScheduler(C) {
     initializePrioRegClasses(C->MF->getRegInfo().getTargetRegisterInfo());
   }
 

llvm/lib/Target/SystemZ/SystemZTargetMachine.cpp

@@ -212,15 +212,15 @@ SystemZTargetMachine::getSubtargetImpl(const Function &F) const {
 }
 
 ScheduleDAGInstrs *
-SystemZTargetMachine::createMachineScheduler(MachineSchedContext *C) const  {
+SystemZTargetMachine::createMachineScheduler(MachineSchedContext *C) const {
   // Use GenericScheduler if requested on CL or for Z10, which has no sched
   // model.
   if (GenericSched ||
       !C->MF->getSubtarget().getSchedModel().hasInstrSchedModel())
     return nullptr;
 
   ScheduleDAGMILive *DAG =
-    new ScheduleDAGMILive(C, std::make_unique<SystemZPreRASchedStrategy>(C));
+      new ScheduleDAGMILive(C, std::make_unique<SystemZPreRASchedStrategy>(C));
   DAG->addMutation(createCopyConstrainDAGMutation(DAG->TII, DAG->TRI));
   return DAG;
 }