[llvm] Revert "MTM: fix issues after cursory reading" (PR #100559)

Mehdi Amini via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 25 05:07:04 PDT 2024


https://github.com/joker-eph created https://github.com/llvm/llvm-project/pull/100559

Reverts llvm/llvm-project#100404

This broke the gcc7 build here: https://lab.llvm.org/buildbot/#/builders/116/builds/1724


>From cf55425639c201d8b3a80541bc222934485d5eda Mon Sep 17 00:00:00 2001
From: Mehdi Amini <joker.eph at gmail.com>
Date: Thu, 25 Jul 2024 14:06:44 +0200
Subject: [PATCH] Revert "MTM: fix issues after cursory reading (#100404)"

This reverts commit 0760aec54ca6f680f4786c4fc3bbae8f500deeab.
---
 llvm/lib/CodeGen/MachineTraceMetrics.cpp | 101 ++++++++++++-----------
 1 file changed, 55 insertions(+), 46 deletions(-)

diff --git a/llvm/lib/CodeGen/MachineTraceMetrics.cpp b/llvm/lib/CodeGen/MachineTraceMetrics.cpp
index dd1faff355b52..bf3add010574b 100644
--- a/llvm/lib/CodeGen/MachineTraceMetrics.cpp
+++ b/llvm/lib/CodeGen/MachineTraceMetrics.cpp
@@ -24,11 +24,17 @@
 #include "llvm/CodeGen/TargetSchedule.h"
 #include "llvm/CodeGen/TargetSubtargetInfo.h"
 #include "llvm/InitializePasses.h"
+#include "llvm/MC/MCRegisterInfo.h"
 #include "llvm/Pass.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/raw_ostream.h"
+#include <algorithm>
+#include <cassert>
+#include <iterator>
+#include <tuple>
+#include <utility>
 
 using namespace llvm;
 
@@ -127,7 +133,7 @@ MachineTraceMetrics::getResources(const MachineBasicBlock *MBB) {
 
   // Scale the resource cycles so they are comparable.
   unsigned PROffset = MBB->getNumber() * PRKinds;
-  for (unsigned K = 0; K < PRKinds; ++K)
+  for (unsigned K = 0; K != PRKinds; ++K)
     ProcReleaseAtCycles[PROffset + K] =
       PRCycles[K] * SchedModel.getResourceFactor(K);
 
@@ -140,14 +146,15 @@ MachineTraceMetrics::getProcReleaseAtCycles(unsigned MBBNum) const {
          "getResources() must be called before getProcReleaseAtCycles()");
   unsigned PRKinds = SchedModel.getNumProcResourceKinds();
   assert((MBBNum+1) * PRKinds <= ProcReleaseAtCycles.size());
-  return ArrayRef{ProcReleaseAtCycles.data() + MBBNum * PRKinds, PRKinds};
+  return ArrayRef(ProcReleaseAtCycles.data() + MBBNum * PRKinds, PRKinds);
 }
 
 //===----------------------------------------------------------------------===//
 //                         Ensemble utility functions
 //===----------------------------------------------------------------------===//
 
-MachineTraceMetrics::Ensemble::Ensemble(MachineTraceMetrics *CT) : MTM(*CT) {
+MachineTraceMetrics::Ensemble::Ensemble(MachineTraceMetrics *ct)
+  : MTM(*ct) {
   BlockInfo.resize(MTM.BlockInfo.size());
   unsigned PRKinds = MTM.SchedModel.getNumProcResourceKinds();
   ProcResourceDepths.resize(MTM.BlockInfo.size() * PRKinds);
@@ -191,7 +198,7 @@ computeDepthResources(const MachineBasicBlock *MBB) {
   // Compute per-resource depths.
   ArrayRef<unsigned> PredPRDepths = getProcResourceDepths(PredNum);
   ArrayRef<unsigned> PredPRCycles = MTM.getProcReleaseAtCycles(PredNum);
-  for (unsigned K = 0; K < PRKinds; ++K)
+  for (unsigned K = 0; K != PRKinds; ++K)
     ProcResourceDepths[PROffset + K] = PredPRDepths[K] + PredPRCycles[K];
 }
 
@@ -224,7 +231,7 @@ computeHeightResources(const MachineBasicBlock *MBB) {
 
   // Compute per-resource heights.
   ArrayRef<unsigned> SuccPRHeights = getProcResourceHeights(SuccNum);
-  for (unsigned K = 0; K < PRKinds; ++K)
+  for (unsigned K = 0; K != PRKinds; ++K)
     ProcResourceHeights[PROffset + K] = SuccPRHeights[K] + PRCycles[K];
 }
 
@@ -257,7 +264,7 @@ MachineTraceMetrics::Ensemble::
 getProcResourceDepths(unsigned MBBNum) const {
   unsigned PRKinds = MTM.SchedModel.getNumProcResourceKinds();
   assert((MBBNum+1) * PRKinds <= ProcResourceDepths.size());
-  return ArrayRef{ProcResourceDepths.data() + MBBNum * PRKinds, PRKinds};
+  return ArrayRef(ProcResourceDepths.data() + MBBNum * PRKinds, PRKinds);
 }
 
 /// Get an array of processor resource heights for MBB. Indexed by processor
@@ -270,7 +277,7 @@ MachineTraceMetrics::Ensemble::
 getProcResourceHeights(unsigned MBBNum) const {
   unsigned PRKinds = MTM.SchedModel.getNumProcResourceKinds();
   assert((MBBNum+1) * PRKinds <= ProcResourceHeights.size());
-  return ArrayRef{ProcResourceHeights.data() + MBBNum * PRKinds, PRKinds};
+  return ArrayRef(ProcResourceHeights.data() + MBBNum * PRKinds, PRKinds);
 }
 
 //===----------------------------------------------------------------------===//
@@ -307,8 +314,8 @@ class MinInstrCountEnsemble : public MachineTraceMetrics::Ensemble {
   const MachineBasicBlock *pickTraceSucc(const MachineBasicBlock*) override;
 
 public:
-  MinInstrCountEnsemble(MachineTraceMetrics *MTM)
-      : MachineTraceMetrics::Ensemble(MTM) {}
+  MinInstrCountEnsemble(MachineTraceMetrics *mtm)
+    : MachineTraceMetrics::Ensemble(mtm) {}
 };
 
 /// Pick only the current basic block for the trace and do not choose any
@@ -388,15 +395,15 @@ MinInstrCountEnsemble::pickTraceSucc(const MachineBasicBlock *MBB) {
 
 // Get an Ensemble sub-class for the requested trace strategy.
 MachineTraceMetrics::Ensemble *
-MachineTraceMetrics::getEnsemble(MachineTraceStrategy Strategy) {
-  assert(Strategy < MachineTraceStrategy::TS_NumStrategies &&
+MachineTraceMetrics::getEnsemble(MachineTraceStrategy strategy) {
+  assert(strategy < MachineTraceStrategy::TS_NumStrategies &&
          "Invalid trace strategy enum");
-  Ensemble *&E = Ensembles[static_cast<size_t>(Strategy)];
+  Ensemble *&E = Ensembles[static_cast<size_t>(strategy)];
   if (E)
     return E;
 
   // Allocate new Ensemble on demand.
-  switch (Strategy) {
+  switch (strategy) {
   case MachineTraceStrategy::TS_MinInstrCount:
     return (E = new MinInstrCountEnsemble(this));
   case MachineTraceStrategy::TS_Local:
@@ -441,9 +448,8 @@ struct LoopBounds {
   const MachineLoopInfo *Loops;
   bool Downward = false;
 
-  LoopBounds(MutableArrayRef<MachineTraceMetrics::TraceBlockInfo> Blocks,
-             const MachineLoopInfo *Loops)
-      : Blocks(Blocks), Loops(Loops) {}
+  LoopBounds(MutableArrayRef<MachineTraceMetrics::TraceBlockInfo> blocks,
+             const MachineLoopInfo *loops) : Blocks(blocks), Loops(loops) {}
 };
 
 } // end anonymous namespace
@@ -457,7 +463,7 @@ class po_iterator_storage<LoopBounds, true> {
   LoopBounds &LB;
 
 public:
-  po_iterator_storage(LoopBounds &LB) : LB(LB) {}
+  po_iterator_storage(LoopBounds &lb) : LB(lb) {}
 
   void finishPostorder(const MachineBasicBlock*) {}
 
@@ -540,7 +546,7 @@ MachineTraceMetrics::Ensemble::invalidate(const MachineBasicBlock *BadMBB) {
   if (BadTBI.hasValidHeight()) {
     BadTBI.invalidateHeight();
     WorkList.push_back(BadMBB);
-    while (!WorkList.empty()) {
+    do {
       const MachineBasicBlock *MBB = WorkList.pop_back_val();
       LLVM_DEBUG(dbgs() << "Invalidate " << printMBBReference(*MBB) << ' '
                         << getName() << " height.\n");
@@ -558,14 +564,14 @@ MachineTraceMetrics::Ensemble::invalidate(const MachineBasicBlock *BadMBB) {
         // Verify that TBI.Succ is actually a *I successor.
         assert((!TBI.Succ || Pred->isSuccessor(TBI.Succ)) && "CFG changed");
       }
-    }
+    } while (!WorkList.empty());
   }
 
   // Invalidate depth resources of blocks below MBB.
   if (BadTBI.hasValidDepth()) {
     BadTBI.invalidateDepth();
     WorkList.push_back(BadMBB);
-    while (!WorkList.empty()) {
+    do {
       const MachineBasicBlock *MBB = WorkList.pop_back_val();
       LLVM_DEBUG(dbgs() << "Invalidate " << printMBBReference(*MBB) << ' '
                         << getName() << " depth.\n");
@@ -583,7 +589,7 @@ MachineTraceMetrics::Ensemble::invalidate(const MachineBasicBlock *BadMBB) {
         // Verify that TBI.Pred is actually a *I predecessor.
         assert((!TBI.Pred || Succ->isPredecessor(TBI.Pred)) && "CFG changed");
       }
-    }
+    } while (!WorkList.empty());
   }
 
   // Clear any per-instruction data. We only have to do this for BadMBB itself
@@ -599,7 +605,7 @@ void MachineTraceMetrics::Ensemble::verify() const {
 #ifndef NDEBUG
   assert(BlockInfo.size() == MTM.MF->getNumBlockIDs() &&
          "Outdated BlockInfo size");
-  for (unsigned Num = 0; Num < BlockInfo.size(); ++Num) {
+  for (unsigned Num = 0, e = BlockInfo.size(); Num != e; ++Num) {
     const TraceBlockInfo &TBI = BlockInfo[Num];
     if (TBI.hasValidDepth() && TBI.Pred) {
       const MachineBasicBlock *MBB = MTM.MF->getBlockNumbered(Num);
@@ -680,7 +686,7 @@ static bool getDataDeps(const MachineInstr &UseMI,
     }
     // Collect virtual register reads.
     if (MO.readsReg())
-      Deps.emplace_back(MRI, Reg, MO.getOperandNo());
+      Deps.push_back(DataDep(MRI, Reg, MO.getOperandNo()));
   }
   return HasPhysRegs;
 }
@@ -696,10 +702,10 @@ static void getPHIDeps(const MachineInstr &UseMI,
   if (!Pred)
     return;
   assert(UseMI.isPHI() && UseMI.getNumOperands() % 2 && "Bad PHI");
-  for (unsigned Idx = 1; Idx < UseMI.getNumOperands(); Idx += 2) {
-    if (UseMI.getOperand(Idx + 1).getMBB() == Pred) {
-      Register Reg = UseMI.getOperand(Idx).getReg();
-      Deps.emplace_back(MRI, Reg, Idx);
+  for (unsigned i = 1; i != UseMI.getNumOperands(); i += 2) {
+    if (UseMI.getOperand(i + 1).getMBB() == Pred) {
+      Register Reg = UseMI.getOperand(i).getReg();
+      Deps.push_back(DataDep(MRI, Reg, i));
       return;
     }
   }
@@ -733,7 +739,7 @@ static void updatePhysDepsDownwards(const MachineInstr *UseMI,
       SparseSet<LiveRegUnit>::iterator I = RegUnits.find(Unit);
       if (I == RegUnits.end())
         continue;
-      Deps.emplace_back(I->MI, I->Op, MO.getOperandNo());
+      Deps.push_back(DataDep(I->MI, I->Op, MO.getOperandNo()));
       break;
     }
   }
@@ -846,14 +852,14 @@ computeInstrDepths(const MachineBasicBlock *MBB) {
   // implies Head->HasValidInstrDepths, so we only need to start from the first
   // block in the trace that needs to be recomputed.
   SmallVector<const MachineBasicBlock*, 8> Stack;
-  while (MBB) {
+  do {
     TraceBlockInfo &TBI = BlockInfo[MBB->getNumber()];
     assert(TBI.hasValidDepth() && "Incomplete trace");
     if (TBI.HasValidInstrDepths)
       break;
     Stack.push_back(MBB);
     MBB = TBI.Pred;
-  }
+  } while (MBB);
 
   // FIXME: If MBB is non-null at this point, it is the last pre-computed block
   // in the trace. We should track any live-out physregs that were defined in
@@ -874,7 +880,7 @@ computeInstrDepths(const MachineBasicBlock *MBB) {
     LLVM_DEBUG({
       dbgs() << format("%7u Instructions\n", TBI.InstrDepth);
       ArrayRef<unsigned> PRDepths = getProcResourceDepths(MBB->getNumber());
-      for (unsigned K = 0; K < PRDepths.size(); ++K)
+      for (unsigned K = 0; K != PRDepths.size(); ++K)
         if (PRDepths[K]) {
           unsigned Factor = MTM.SchedModel.getResourceFactor(K);
           dbgs() << format("%6uc @ ", MTM.getCycles(PRDepths[K]))
@@ -963,8 +969,10 @@ static bool pushDepHeight(const DataDep &Dep, const MachineInstr &UseMI,
                                                   Dep.UseOp);
 
   // Update Heights[DefMI] to be the maximum height seen.
-  const auto &[I, Inserted] = Heights.insert({Dep.DefMI, UseHeight});
-  if (Inserted)
+  MIHeightMap::iterator I;
+  bool New;
+  std::tie(I, New) = Heights.insert(std::make_pair(Dep.DefMI, UseHeight));
+  if (New)
     return true;
 
   // DefMI has been pushed before. Give it the max height.
@@ -1002,7 +1010,7 @@ computeInstrHeights(const MachineBasicBlock *MBB) {
   // The bottom of the trace may already be computed.
   // Find the blocks that need updating.
   SmallVector<const MachineBasicBlock*, 8> Stack;
-  while (MBB) {
+  do {
     TraceBlockInfo &TBI = BlockInfo[MBB->getNumber()];
     assert(TBI.hasValidHeight() && "Incomplete trace");
     if (TBI.HasValidInstrHeights)
@@ -1010,7 +1018,7 @@ computeInstrHeights(const MachineBasicBlock *MBB) {
     Stack.push_back(MBB);
     TBI.LiveIns.clear();
     MBB = TBI.Succ;
-  }
+  } while (MBB);
 
   // As we move upwards in the trace, keep track of instructions that are
   // required by deeper trace instructions. Map MI -> height required so far.
@@ -1052,7 +1060,7 @@ computeInstrHeights(const MachineBasicBlock *MBB) {
     LLVM_DEBUG({
       dbgs() << format("%7u Instructions\n", TBI.InstrHeight);
       ArrayRef<unsigned> PRHeights = getProcResourceHeights(MBB->getNumber());
-      for (unsigned K = 0; K < PRHeights.size(); ++K)
+      for (unsigned K = 0; K != PRHeights.size(); ++K)
         if (PRHeights[K]) {
           unsigned Factor = MTM.SchedModel.getResourceFactor(K);
           dbgs() << format("%6uc @ ", MTM.getCycles(PRHeights[K]))
@@ -1137,7 +1145,7 @@ computeInstrHeights(const MachineBasicBlock *MBB) {
 
     // Transfer the live regunits to the live-in list.
     for (const LiveRegUnit &RU : RegUnits) {
-      TBI.LiveIns.emplace_back(RU.RegUnit, RU.Cycle);
+      TBI.LiveIns.push_back(LiveInReg(RU.RegUnit, RU.Cycle));
       LLVM_DEBUG(dbgs() << ' ' << printRegUnit(RU.RegUnit, MTM.TRI) << '@'
                         << RU.Cycle);
     }
@@ -1197,7 +1205,7 @@ unsigned MachineTraceMetrics::Trace::getResourceDepth(bool Bottom) const {
   ArrayRef<unsigned> PRDepths = TE.getProcResourceDepths(getBlockNum());
   if (Bottom) {
     ArrayRef<unsigned> PRCycles = TE.MTM.getProcReleaseAtCycles(getBlockNum());
-    for (unsigned K = 0; K < PRDepths.size(); ++K)
+    for (unsigned K = 0; K != PRDepths.size(); ++K)
       PRMax = std::max(PRMax, PRDepths[K] + PRCycles[K]);
   } else {
     for (unsigned PRD : PRDepths)
@@ -1227,8 +1235,9 @@ unsigned MachineTraceMetrics::Trace::getResourceLength(
   unsigned PRMax = 0;
 
   // Capture computing cycles from extra instructions
-  auto ExtraCycles = [this](ArrayRef<const MCSchedClassDesc *> Instrs,
-                            unsigned ResourceIdx) -> unsigned {
+  auto extraCycles = [this](ArrayRef<const MCSchedClassDesc *> Instrs,
+                            unsigned ResourceIdx)
+                         ->unsigned {
     unsigned Cycles = 0;
     for (const MCSchedClassDesc *SC : Instrs) {
       if (!SC->isValid())
@@ -1246,12 +1255,12 @@ unsigned MachineTraceMetrics::Trace::getResourceLength(
     return Cycles;
   };
 
-  for (unsigned K = 0; K < PRDepths.size(); ++K) {
+  for (unsigned K = 0; K != PRDepths.size(); ++K) {
     unsigned PRCycles = PRDepths[K] + PRHeights[K];
     for (const MachineBasicBlock *MBB : Extrablocks)
       PRCycles += TE.MTM.getProcReleaseAtCycles(MBB->getNumber())[K];
-    PRCycles += ExtraCycles(ExtraInstrs, K);
-    PRCycles -= ExtraCycles(RemoveInstrs, K);
+    PRCycles += extraCycles(ExtraInstrs, K);
+    PRCycles -= extraCycles(RemoveInstrs, K);
     PRMax = std::max(PRMax, PRCycles);
   }
   // Convert to cycle count.
@@ -1283,9 +1292,9 @@ bool MachineTraceMetrics::Trace::isDepInTrace(const MachineInstr &DefMI,
 
 void MachineTraceMetrics::Ensemble::print(raw_ostream &OS) const {
   OS << getName() << " ensemble:\n";
-  for (unsigned Idx = 0; Idx < BlockInfo.size(); ++Idx) {
-    OS << "  %bb." << Idx << '\t';
-    BlockInfo[Idx].print(OS);
+  for (unsigned i = 0, e = BlockInfo.size(); i != e; ++i) {
+    OS << "  %bb." << i << '\t';
+    BlockInfo[i].print(OS);
     OS << '\n';
   }
 }



More information about the llvm-commits mailing list