Squash.

- Rebase.
- Tests updated and passing.

Minor tweaks and fixes:
- '-prera-lat-red' option for experiments with latency reduction.
- Don't reduce latency with a wide DAG that is acyclically latency limited.
- Simplify and factor out check for IsStore.
- Disable IsRedefining[] with PressureDiffs.
- Add check for vreg uses in biasPhysRegExtra()
- NFC fix for ShouldTrackPressure init.
- Remove lenghty code in initializePressureSets() - there is a test.
- Remove changes in SystemZElimCompare as that is not used anymore.

Author: JonPsson1

llvm/include/llvm/CodeGen/MachineScheduler.h

@@ -1224,7 +1224,7 @@ class GenericSchedulerBase : public MachineSchedStrategy {
   void traceCandidate(const SchedCandidate &Cand);
 #endif
 
-private:
+protected:
   bool shouldReduceLatency(const CandPolicy &Policy, SchedBoundary &CurrZone,
                            bool ComputeRemLatency, unsigned &RemLatency) const;
 };

llvm/lib/Target/SystemZ/SystemZElimCompare.cpp

@@ -150,6 +150,30 @@ Reference SystemZElimCompare::getRegReferences(MachineInstr &MI, unsigned Reg) {
   return Ref;
 }
 
+// Return true if this is a load and test which can be optimized the
+// same way as compare instruction.
+static bool isLoadAndTestAsCmp(MachineInstr &MI) {
+  // If we during isel used a load-and-test as a compare with 0, the
+  // def operand is dead.
+  return (MI.getOpcode() == SystemZ::LTEBR ||
+          MI.getOpcode() == SystemZ::LTDBR ||
+          MI.getOpcode() == SystemZ::LTXBR) &&
+         MI.getOperand(0).isDead();
+}
+
+// Return the source register of Compare, which is the unknown value
+// being tested.
+static unsigned getCompareSourceReg(MachineInstr &Compare) {
+  unsigned reg = 0;
+  if (Compare.isCompare())
+    reg = Compare.getOperand(0).getReg();
+  else if (isLoadAndTestAsCmp(Compare))
+    reg = Compare.getOperand(1).getReg();
+  assert(reg);
+
+  return reg;
+}
+
 // Compare compares the result of MI against zero.  If MI is an addition
 // of -1 and if CCUsers is a single branch on nonzero, eliminate the addition
 // and convert the branch to a BRCT(G) or BRCTH.  Return true on success.
@@ -182,7 +206,7 @@ bool SystemZElimCompare::convertToBRCT(
   // We already know that there are no references to the register between
   // MI and Compare.  Make sure that there are also no references between
   // Compare and Branch.
-  unsigned SrcReg = TII->getCompareSourceReg(Compare);
+  unsigned SrcReg = getCompareSourceReg(Compare);
   MachineBasicBlock::iterator MBBI = Compare, MBBE = Branch;
   for (++MBBI; MBBI != MBBE; ++MBBI)
     if (getRegReferences(*MBBI, SrcReg))
@@ -229,7 +253,7 @@ bool SystemZElimCompare::convertToLoadAndTrap(
   // We already know that there are no references to the register between
   // MI and Compare.  Make sure that there are also no references between
   // Compare and Branch.
-  unsigned SrcReg = TII->getCompareSourceReg(Compare);
+  unsigned SrcReg = getCompareSourceReg(Compare);
   MachineBasicBlock::iterator MBBI = Compare, MBBE = Branch;
   for (++MBBI; MBBI != MBBE; ++MBBI)
     if (getRegReferences(*MBBI, SrcReg))
@@ -470,17 +494,25 @@ bool SystemZElimCompare::adjustCCMasksForInstr(
   return true;
 }
 
+// Return true if Compare is a comparison against zero.
+static bool isCompareZero(MachineInstr &Compare) {
+  if (isLoadAndTestAsCmp(Compare))
+    return true;
+  return Compare.getNumExplicitOperands() == 2 &&
+    Compare.getOperand(1).isImm() && Compare.getOperand(1).getImm() == 0;
+}
+
 // Try to optimize cases where comparison instruction Compare is testing
 // a value against zero.  Return true on success and if Compare should be
 // deleted as dead.  CCUsers is the list of instructions that use the CC
 // value produced by Compare.
 bool SystemZElimCompare::optimizeCompareZero(
     MachineInstr &Compare, SmallVectorImpl<MachineInstr *> &CCUsers) {
-  if (!TII->isCompareZero(Compare))
+  if (!isCompareZero(Compare))
     return false;
 
   // Search back for CC results that are based on the first operand.
-  unsigned SrcReg = TII->getCompareSourceReg(Compare);
+  unsigned SrcReg = getCompareSourceReg(Compare);
   MachineBasicBlock &MBB = *Compare.getParent();
   Reference CCRefs;
   Reference SrcRefs;
@@ -669,7 +701,7 @@ bool SystemZElimCompare::processBlock(MachineBasicBlock &MBB) {
   MachineBasicBlock::iterator MBBI = MBB.end();
   while (MBBI != MBB.begin()) {
     MachineInstr &MI = *--MBBI;
-    if (CompleteCCUsers && (MI.isCompare() || TII->isLoadAndTestAsCmp(MI)) &&
+    if (CompleteCCUsers && (MI.isCompare() || isLoadAndTestAsCmp(MI)) &&
         (optimizeCompareZero(MI, CCUsers) ||
          fuseCompareOperations(MI, CCUsers))) {
       ++MBBI;

llvm/lib/Target/SystemZ/SystemZInstrInfo.cpp

@@ -2156,28 +2156,6 @@ unsigned SystemZInstrInfo::getFusedCompare(unsigned Opcode,
   return 0;
 }
 
-bool SystemZInstrInfo::isLoadAndTestAsCmp(const MachineInstr &MI) const {
-  // If we during isel used a load-and-test as a compare with 0, the
-  // def operand is dead.
-  return (MI.getOpcode() == SystemZ::LTEBR ||
-          MI.getOpcode() == SystemZ::LTDBR ||
-          MI.getOpcode() == SystemZ::LTXBR) &&
-         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;
-}
-
-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::
 prepareCompareSwapOperands(MachineBasicBlock::iterator const MBBI) const {
   assert(MBBI->isCompare() && MBBI->getOperand(0).isReg() &&

llvm/lib/Target/SystemZ/SystemZInstrInfo.h

@@ -356,17 +356,6 @@ class SystemZInstrInfo : public SystemZGenInstrInfo {
                            SystemZII::FusedCompareType Type,
                            const MachineInstr *MI = nullptr) const;
 
-  // Return true if this is a load and test which can be optimized the
-  // same way as compare instruction.
-  bool isLoadAndTestAsCmp(const MachineInstr &MI) const;
-
-  // Return true if Compare is a comparison against zero.
-  bool isCompareZero(const MachineInstr &Compare) const;
-
-  // Return the source register of Compare, which is the unknown value
-  // being tested.
-  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
   // operands. Return false if not all users can be conclusively found and

llvm/lib/Target/SystemZ/SystemZMachineScheduler.cpp

@@ -20,7 +20,28 @@ using namespace llvm;
 static cl::opt<unsigned> TinyRegionLim(
     "tiny-region-lim", cl::Hidden, cl::init(10),
     cl::desc("Run limited pre-ra scheduling on regions of this size or "
-             "smaller. Mainly for testing."));
+             "smaller."));
+
+namespace SystemZSched {
+enum LatencyReduction { Always, Never, More, Heuristics, CycleBased };
+} // namespace SystemZSched
+
+static cl::opt<SystemZSched::LatencyReduction> PreRALatRed(
+    "prera-lat-red", cl::Hidden,
+    cl::desc("Tuning of latency reduction during pre-ra mi-scheduling."),
+    cl::init(SystemZSched::LatencyReduction::Heuristics),
+    cl::values(
+        clEnumValN(SystemZSched::LatencyReduction::Always, "always",
+                   "Reduce scheduled latency always."),
+        clEnumValN(SystemZSched::LatencyReduction::Never, "never",
+                   "Don't reduce scheduled latency."),
+        clEnumValN(SystemZSched::LatencyReduction::More, "more",
+                   "Reduce scheduled latency on most DAGs."),
+        clEnumValN(SystemZSched::LatencyReduction::Heuristics, "heuristics",
+                   "Use heuristics for reduction of scheduled latency."),
+        clEnumValN(SystemZSched::LatencyReduction::CycleBased, "cycle-based",
+                   "Use GenericSched cycle based decisions for reduction of "
+                   "scheduled latency.")));
 
 // EXPERIMENTAL
 static cl::opt<bool>
@@ -86,51 +107,36 @@ void SystemZPreRASchedStrategy::initializePressureSets(
   // If %7 is already live, there would also be 'VR16Bit -1', which is the
   // interesting case.
   //
-  // Rather than hard coding VR16Bit and GRX32Bit PressureSets, they are
-  // inferred below as the intersections of various register class groups.
-  //
-  // TODO: Could TableGen emit these directly instead?
+  // misched-prera-pdiffs.mir tests against any future change in the
+  // PressureSets, so simply hard-code them here:
+
   if (!WITHPDIFFS)
     return;
+  PrioPressureSet = SystemZ::VR16Bit;
+  GPRPressureSet = SystemZ::GRX32Bit;
+}
 
-  auto addPSets = [&TRI](std::set<unsigned> &S, const TargetRegisterClass *RC,
-                         std::set<unsigned> *Intersect = nullptr) {
-    for (const int *PS = TRI->getRegClassPressureSets(RC); *PS != -1; ++PS)
-      if (!Intersect || Intersect->count(*PS))
-        S.insert(*PS);
-  };
-
-  std::set<unsigned> SetA, SetB;
-  addPSets(SetA, &SystemZ::VR16BitRegClass);
-  addPSets(SetA, &SystemZ::VR32BitRegClass);
-  addPSets(SetA, &SystemZ::VR64BitRegClass);
-  addPSets(SetA, &SystemZ::VR128BitRegClass);
-  assert(SetA.size() == 1 && "Expected one pressure set (VR16Bit).");
-
-  addPSets(SetB, &SystemZ::FP16BitRegClass, &SetA);
-  addPSets(SetB, &SystemZ::FP32BitRegClass, &SetA);
-  addPSets(SetB, &SystemZ::FP64BitRegClass, &SetA);
-  addPSets(SetB, &SystemZ::VF128BitRegClass, &SetA);
-  addPSets(SetB, &SystemZ::FP128BitRegClass, &SetA);
-  assert(SetB.size() == 1 && *SetA.begin() == *SetB.begin() &&
-         "Expected one pressure set (VR16Bit).");
-  PrioPressureSet = *SetB.begin();
-
-  SetA.clear();
-  SetB.clear();
-  addPSets(SetA, &SystemZ::GRX32BitRegClass);
-  addPSets(SetA, &SystemZ::GR64BitRegClass);
-  addPSets(SetA, &SystemZ::ADDR64BitRegClass);
-  addPSets(SetA, &SystemZ::GR128BitRegClass);
-  addPSets(SetA, &SystemZ::ADDR128BitRegClass);
-  assert(SetA.size() == 1 && "Expected one pressure set (GRX32Bit).");
-
-  addPSets(SetB, &SystemZ::GR32BitRegClass, &SetA);
-  addPSets(SetB, &SystemZ::GRH32BitRegClass, &SetA);
-  addPSets(SetB, &SystemZ::ADDR32BitRegClass, &SetA);
-  assert(SetB.size() == 1 && *SetA.begin() == *SetB.begin() &&
-         "Expected one pressure set (GRX32Bit).");
-  GPRPressureSet = *SetB.begin();
+bool SystemZPreRASchedStrategy::shouldReduceLatency(SchedBoundary *Zone) const {
+  if (PreRALatRed == SystemZSched::Always)
+    return true;
+  if (PreRALatRed == SystemZSched::Never)
+    return false;
+
+  if (IsWideDAG)
+    return false;
+
+  if (PreRALatRed == SystemZSched::More)
+    return true;
+  if (PreRALatRed == SystemZSched::Heuristics)
+    return HasDataSequences || Rem.IsAcyclicLatencyLimited;
+
+  if (PreRALatRed == SystemZSched::CycleBased) {
+    CandPolicy P;
+    getRemLat(Zone);
+    return GenericScheduler::shouldReduceLatency(P, *Zone, false, RemLat);
+  }
+
+  llvm_unreachable("Unhandled option value.");
 }
 
 unsigned SystemZPreRASchedStrategy::getRemLat(SchedBoundary *Zone) const {
@@ -139,6 +145,12 @@ unsigned SystemZPreRASchedStrategy::getRemLat(SchedBoundary *Zone) const {
   return RemLat;
 }
 
+static bool isStoreOfVReg(const MachineInstr *MI) {
+  return MI->mayStore() && !MI->mayLoad() && MI->getNumOperands() &&
+         isVirtRegUse(MI->getOperand(0)) &&
+         MI->getDesc().operands()[0].OperandType != MCOI::OPERAND_MEMORY;
+}
+
 void SystemZPreRASchedStrategy::initializeStoresGroup() {
   StoresGroup.clear();
   FirstStoreInGroupScheduled = false;
@@ -149,18 +161,7 @@ void SystemZPreRASchedStrategy::initializeStoresGroup() {
     const MachineInstr *MI = SU->getInstr();
     if (!MI->getNumOperands() || MI->isCopy())
       continue;
-
-    bool HasVirtDef = false;
-    bool HasVirtUse = false;
-    for (unsigned I = 0; I < MI->getDesc().getNumOperands(); ++I) {
-      const MachineOperand &MO = MI->getOperand(I);
-      if (isVirtRegDef(MO) && !MO.isDead())
-        HasVirtDef = true;
-      else if (isVirtRegUse(MO) &&
-               MI->getDesc().operands()[I].OperandType != MCOI::OPERAND_MEMORY)
-        HasVirtUse = true;
-    }
-    bool IsStore = !HasVirtDef && HasVirtUse;
+    bool IsStore = isStoreOfVReg(MI);
 
     // Find a group of stores that all are at the bottom while avoiding
     // regions with any additional group of lesser depth.
@@ -198,8 +199,14 @@ static int biasPhysRegExtra(const SUnit *SU) {
   const MachineInstr *MI = SU->getInstr();
   if (MI->getNumOperands() && !MI->isCopy()) {
     const MachineOperand &DefMO = MI->getOperand(0);
-    if (isPhysRegDef(DefMO))
+    if (isPhysRegDef(DefMO)) {
+#ifndef NDEBUG
+      for (const MachineOperand &MO : MI->all_uses())
+        assert(!MO.getReg().isVirtual() &&
+               "Did not expect a virtual register use operand.");
+#endif
       return 1;
+    }
   }
 
   return 0;
@@ -216,9 +223,8 @@ int SystemZPreRASchedStrategy::computeSULivenessScore(
 
   const MachineOperand &MO0 = MI->getOperand(0);
   assert(!isPhysRegDef(MO0) && "Did not expect physreg def!");
-  bool IsLoad =
-      isRegDef(MO0) && !MO0.isDead() && !IsRedefining[SU->NodeNum];
-  bool IsStore = (!isRegDef(MO0) || MO0.isDead());
+  bool IsLoad = isRegDef(MO0) && !MO0.isDead() && !IsRedefining[SU->NodeNum];
+  bool IsStore = isStoreOfVReg(MI);
   bool PreservesSchedLat = SU->getHeight() <= Zone->getScheduledLatency();
   const unsigned Cycles = 2;
   unsigned Margin = SchedModel->getIssueWidth() * (Cycles + SU->Latency - 1);
@@ -272,12 +278,10 @@ int SystemZPreRASchedStrategy::computeSULivenessScore(
     if (IsLoad) {
       bool PrioDefNoKill = PrioPressureChange == -RegWeight;
       bool GPRDefNoKill = GPRPressureChange == -RegWeight;
-      UsesLivePrio =
-          (PrioDefNoKill || (!PrioPressureChange && GPRDefNoKill));
+      UsesLivePrio = (PrioDefNoKill || (!PrioPressureChange && GPRDefNoKill));
       UsesLiveAll = (PrioDefNoKill && !GPRPressureChange) ||
                     (!PrioPressureChange && GPRDefNoKill);
-    }
-    else if (IsStore && FirstStoreInGroupScheduled && StoresGroup.count(SU)) {
+    } else if (IsStore && FirstStoreInGroupScheduled && StoresGroup.count(SU)) {
       bool SrcKill = !DAG->getBotRPTracker().isRegLive(MO0.getReg());
       StoreKill =
           SrcKill && (PrioPressureChange == RegWeight ||
@@ -344,7 +348,7 @@ bool SystemZPreRASchedStrategy::tryCandidate(SchedCandidate &Cand,
     // Don't extend the scheduled latency in regions with many nodes in
     // simple data sequences, or for (single block loop) regions that are
     // acyclically (within a single loop iteration) latency limited.
-    if ((HasDataSequences || Rem.IsAcyclicLatencyLimited) &&
+    if (shouldReduceLatency(Zone) &&
         TryCand.SU->getHeight() != Cand.SU->getHeight() &&
         (std::max(TryCand.SU->getHeight(), Cand.SU->getHeight()) >
          Zone->getScheduledLatency())) {
@@ -359,7 +363,7 @@ bool SystemZPreRASchedStrategy::tryCandidate(SchedCandidate &Cand,
     }
   }
 
-  // Weak edges are for clustering and other constraints.
+  // Weak edges help copy coalescing.
   if (tryLess(TryCand.SU->WeakSuccsLeft, Cand.SU->WeakSuccsLeft, TryCand, Cand,
               Weak))
     return TryCand.Reason != NoCand;
@@ -378,9 +382,7 @@ void SystemZPreRASchedStrategy::initPolicy(MachineBasicBlock::iterator Begin,
                                            unsigned NumRegionInstrs) {
   TinyRegion = NumRegionInstrs <= TinyRegionLim;
 
-  //  RegionPolicy.ShouldTrackPressure = !TinyRegion;
-  // Some exceptions are made, see initialize().
-  RegionPolicy.ShouldTrackPressure = NumRegionInstrs > 6;
+  RegionPolicy.ShouldTrackPressure = !TinyRegion;
 
   // These heuristics has so far seemed to work better without adding a
   // top-down boundary.
@@ -407,7 +409,8 @@ void SystemZPreRASchedStrategy::initialize(ScheduleDAGMI *dag) {
   unsigned DAGHeight = 0;
   for (unsigned Idx = 0, End = DAG->SUnits.size(); Idx != End; ++Idx)
     DAGHeight = std::max(DAGHeight, DAG->SUnits[Idx].getHeight());
-  if ((HasDataSequences = DAG->SUnits.size() < 3 * std::max(DAGHeight, 1u))) {
+  IsWideDAG = DAG->SUnits.size() >= 3 * std::max(DAGHeight, 1u);
+  if ((HasDataSequences = !IsWideDAG)) {
     unsigned CurrSequence = 0, NumSeqNodes = 0;
     auto countSequence = [&CurrSequence, &NumSeqNodes]() {
       NumSeqNodes += CurrSequence >= 2 ? CurrSequence : 0;
@@ -443,14 +446,15 @@ void SystemZPreRASchedStrategy::initialize(ScheduleDAGMI *dag) {
 
   // If MI uses the register it defines, record it one time here.
   IsRedefining = std::vector<bool>(DAG->SUnits.size(), false);
-  for (unsigned Idx = 0, End = DAG->SUnits.size(); Idx != End; ++Idx) {
-    const MachineInstr *MI = DAG->SUnits[Idx].getInstr();
-    if (MI->getNumOperands()) {
-      const MachineOperand &DefMO = MI->getOperand(0);
-      if (isVirtRegDef(DefMO))
-        IsRedefining[Idx] = MI->readsVirtualRegister(DefMO.getReg());
+  if (!WITHPDIFFS) // This is not needed if using PressureDiffs.
+    for (unsigned Idx = 0, End = DAG->SUnits.size(); Idx != End; ++Idx) {
+      const MachineInstr *MI = DAG->SUnits[Idx].getInstr();
+      if (MI->getNumOperands()) {
+        const MachineOperand &DefMO = MI->getOperand(0);
+        if (isVirtRegDef(DefMO))
+          IsRedefining[Idx] = MI->readsVirtualRegister(DefMO.getReg());
+      }
     }
-  }
 
   initializeStoresGroup();
   LLVM_DEBUG(if (!StoresGroup.empty()) dbgs()