After llvm commit f240e528cea25fd2a9ae01b1e1fe77f507ed7a2c
Author: Kazu Hirata <k...@google.com>

    [llvm] Use range-based for loops (NFC)

the following benchmarks slowed down by more than 2%:
- 453.povray slowed down by 3% from 4906 to 5047 perf samples

Below reproducer instructions can be used to re-build both "first_bad" and 
"last_good" cross-toolchains used in this bisection.  Naturally, the scripts 
will fail when triggerring benchmarking jobs if you don't have access to Linaro 
TCWG CI.

For your convenience, we have uploaded tarballs with pre-processed source and 
assembly files at:
- First_bad save-temps: 
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_tx1-llvm-master-aarch64-spec2k6-O2_LTO/33/artifact/artifacts/build-f240e528cea25fd2a9ae01b1e1fe77f507ed7a2c/save-temps/
- Last_good save-temps: 
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_tx1-llvm-master-aarch64-spec2k6-O2_LTO/33/artifact/artifacts/build-c572eb1ad9d8a528bcaff0160888aff31b1f4b5f/save-temps/
- Baseline save-temps: 
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_tx1-llvm-master-aarch64-spec2k6-O2_LTO/33/artifact/artifacts/build-baseline/save-temps/

Configuration:
- Benchmark: SPEC CPU2006
- Toolchain: Clang + Glibc + LLVM Linker
- Version: all components were built from their tip of trunk
- Target: aarch64-linux-gnu
- Compiler flags: -O2 -flto
- Hardware: NVidia TX1 4x Cortex-A57

This benchmarking CI is work-in-progress, and we welcome feedback and 
suggestions at linaro-toolchain@lists.linaro.org .  In our improvement plans is 
to add support for SPEC CPU2017 benchmarks and provide "perf report/annotate" 
data behind these reports.

THIS IS THE END OF INTERESTING STUFF.  BELOW ARE LINKS TO BUILDS, REPRODUCTION 
INSTRUCTIONS, AND THE RAW COMMIT.

This commit has regressed these CI configurations:
 - tcwg_bmk_llvm_tx1/llvm-master-aarch64-spec2k6-O2_LTO

First_bad build: 
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_tx1-llvm-master-aarch64-spec2k6-O2_LTO/33/artifact/artifacts/build-f240e528cea25fd2a9ae01b1e1fe77f507ed7a2c/
Last_good build: 
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_tx1-llvm-master-aarch64-spec2k6-O2_LTO/33/artifact/artifacts/build-c572eb1ad9d8a528bcaff0160888aff31b1f4b5f/
Baseline build: 
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_tx1-llvm-master-aarch64-spec2k6-O2_LTO/33/artifact/artifacts/build-baseline/
Even more details: 
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_tx1-llvm-master-aarch64-spec2k6-O2_LTO/33/artifact/artifacts/

Reproduce builds:
<cut>
mkdir investigate-llvm-f240e528cea25fd2a9ae01b1e1fe77f507ed7a2c
cd investigate-llvm-f240e528cea25fd2a9ae01b1e1fe77f507ed7a2c

# Fetch scripts
git clone https://git.linaro.org/toolchain/jenkins-scripts

# Fetch manifests and test.sh script
mkdir -p artifacts/manifests
curl -o artifacts/manifests/build-baseline.sh 
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_tx1-llvm-master-aarch64-spec2k6-O2_LTO/33/artifact/artifacts/manifests/build-baseline.sh
 --fail
curl -o artifacts/manifests/build-parameters.sh 
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_tx1-llvm-master-aarch64-spec2k6-O2_LTO/33/artifact/artifacts/manifests/build-parameters.sh
 --fail
curl -o artifacts/test.sh 
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_tx1-llvm-master-aarch64-spec2k6-O2_LTO/33/artifact/artifacts/test.sh
 --fail
chmod +x artifacts/test.sh

# Reproduce the baseline build (build all pre-requisites)
./jenkins-scripts/tcwg_bmk-build.sh @@ artifacts/manifests/build-baseline.sh

# Save baseline build state (which is then restored in artifacts/test.sh)
mkdir -p ./bisect
rsync -a --del --delete-excluded --exclude /bisect/ --exclude /artifacts/ 
--exclude /llvm/ ./ ./bisect/baseline/

cd llvm

# Reproduce first_bad build
git checkout --detach f240e528cea25fd2a9ae01b1e1fe77f507ed7a2c
../artifacts/test.sh

# Reproduce last_good build
git checkout --detach c572eb1ad9d8a528bcaff0160888aff31b1f4b5f
../artifacts/test.sh

cd ..
</cut>

Full commit (up to 1000 lines):
<cut>
commit f240e528cea25fd2a9ae01b1e1fe77f507ed7a2c
Author: Kazu Hirata <k...@google.com>
Date:   Mon Nov 29 09:04:44 2021 -0800

    [llvm] Use range-based for loops (NFC)
---
 llvm/lib/CodeGen/MachinePipeliner.cpp              |  7 ++---
 .../CodeGen/SelectionDAG/ResourcePriorityQueue.cpp |  4 +--
 llvm/lib/Object/ELFObjectFile.cpp                  |  4 +--
 llvm/lib/ObjectYAML/COFFEmitter.cpp                | 32 ++++++++++------------
 llvm/lib/Passes/StandardInstrumentations.cpp       |  4 +--
 llvm/lib/ProfileData/InstrProf.cpp                 | 11 ++++----
 llvm/lib/Target/Hexagon/HexagonCommonGEP.cpp       | 18 ++++++------
 llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp          | 32 ++++++++++------------
 8 files changed, 50 insertions(+), 62 deletions(-)

diff --git a/llvm/lib/CodeGen/MachinePipeliner.cpp 
b/llvm/lib/CodeGen/MachinePipeliner.cpp
index 21be6718b7d9..8d6459a627fa 100644
--- a/llvm/lib/CodeGen/MachinePipeliner.cpp
+++ b/llvm/lib/CodeGen/MachinePipeliner.cpp
@@ -1519,9 +1519,8 @@ static bool pred_L(SetVector<SUnit *> &NodeOrder,
                    SmallSetVector<SUnit *, 8> &Preds,
                    const NodeSet *S = nullptr) {
   Preds.clear();
-  for (SetVector<SUnit *>::iterator I = NodeOrder.begin(), E = NodeOrder.end();
-       I != E; ++I) {
-    for (const SDep &Pred : (*I)->Preds) {
+  for (const SUnit *SU : NodeOrder) {
+    for (const SDep &Pred : SU->Preds) {
       if (S && S->count(Pred.getSUnit()) == 0)
         continue;
       if (ignoreDependence(Pred, true))
@@ -1530,7 +1529,7 @@ static bool pred_L(SetVector<SUnit *> &NodeOrder,
         Preds.insert(Pred.getSUnit());
     }
     // Back-edges are predecessors with an anti-dependence.
-    for (const SDep &Succ : (*I)->Succs) {
+    for (const SDep &Succ : SU->Succs) {
       if (Succ.getKind() != SDep::Anti)
         continue;
       if (S && S->count(Succ.getSUnit()) == 0)
diff --git a/llvm/lib/CodeGen/SelectionDAG/ResourcePriorityQueue.cpp 
b/llvm/lib/CodeGen/SelectionDAG/ResourcePriorityQueue.cpp
index 55fe26eb64cd..2695ed36991c 100644
--- a/llvm/lib/CodeGen/SelectionDAG/ResourcePriorityQueue.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/ResourcePriorityQueue.cpp
@@ -268,8 +268,8 @@ bool ResourcePriorityQueue::isResourceAvailable(SUnit *SU) {
 
   // Now see if there are no other dependencies
   // to instructions already in the packet.
-  for (unsigned i = 0, e = Packet.size(); i != e; ++i)
-    for (const SDep &Succ : Packet[i]->Succs) {
+  for (const SUnit *S : Packet)
+    for (const SDep &Succ : S->Succs) {
       // Since we do not add pseudos to packets, might as well
       // ignore order deps.
       if (Succ.isCtrl())
diff --git a/llvm/lib/Object/ELFObjectFile.cpp 
b/llvm/lib/Object/ELFObjectFile.cpp
index 50035d6c7523..cf1f12d9a9a7 100644
--- a/llvm/lib/Object/ELFObjectFile.cpp
+++ b/llvm/lib/Object/ELFObjectFile.cpp
@@ -682,7 +682,7 @@ readDynsymVersionsImpl(const ELFFile<ELFT> &EF,
 
   std::vector<VersionEntry> Ret;
   size_t I = 0;
-  for (auto It = Symbols.begin(), E = Symbols.end(); It != E; ++It) {
+  for (const ELFSymbolRef &Sym : Symbols) {
     ++I;
     Expected<const typename ELFT::Versym *> VerEntryOrErr =
         EF.template getEntry<typename ELFT::Versym>(*VerSec, I);
@@ -691,7 +691,7 @@ readDynsymVersionsImpl(const ELFFile<ELFT> &EF,
                          " from " + describe(EF, *VerSec) + ": " +
                          toString(VerEntryOrErr.takeError()));
 
-    Expected<uint32_t> FlagsOrErr = It->getFlags();
+    Expected<uint32_t> FlagsOrErr = Sym.getFlags();
     if (!FlagsOrErr)
       return createError("unable to read flags for symbol with index " +
                          Twine(I) + ": " + toString(FlagsOrErr.takeError()));
diff --git a/llvm/lib/ObjectYAML/COFFEmitter.cpp 
b/llvm/lib/ObjectYAML/COFFEmitter.cpp
index 5f38ca13cfc2..66ad16db1ba4 100644
--- a/llvm/lib/ObjectYAML/COFFEmitter.cpp
+++ b/llvm/lib/ObjectYAML/COFFEmitter.cpp
@@ -476,29 +476,25 @@ static bool writeCOFF(COFFParser &CP, raw_ostream &OS) {
 
   assert(OS.tell() == CP.SectionTableStart);
   // Output section table.
-  for (std::vector<COFFYAML::Section>::iterator i = CP.Obj.Sections.begin(),
-                                                e = CP.Obj.Sections.end();
-       i != e; ++i) {
-    OS.write(i->Header.Name, COFF::NameSize);
-    OS << binary_le(i->Header.VirtualSize)
-       << binary_le(i->Header.VirtualAddress)
-       << binary_le(i->Header.SizeOfRawData)
-       << binary_le(i->Header.PointerToRawData)
-       << binary_le(i->Header.PointerToRelocations)
-       << binary_le(i->Header.PointerToLineNumbers)
-       << binary_le(i->Header.NumberOfRelocations)
-       << binary_le(i->Header.NumberOfLineNumbers)
-       << binary_le(i->Header.Characteristics);
+  for (const COFFYAML::Section &S : CP.Obj.Sections) {
+    OS.write(S.Header.Name, COFF::NameSize);
+    OS << binary_le(S.Header.VirtualSize)
+       << binary_le(S.Header.VirtualAddress)
+       << binary_le(S.Header.SizeOfRawData)
+       << binary_le(S.Header.PointerToRawData)
+       << binary_le(S.Header.PointerToRelocations)
+       << binary_le(S.Header.PointerToLineNumbers)
+       << binary_le(S.Header.NumberOfRelocations)
+       << binary_le(S.Header.NumberOfLineNumbers)
+       << binary_le(S.Header.Characteristics);
   }
   assert(OS.tell() == CP.SectionTableStart + CP.SectionTableSize);
 
   unsigned CurSymbol = 0;
   StringMap<unsigned> SymbolTableIndexMap;
-  for (std::vector<COFFYAML::Symbol>::iterator I = CP.Obj.Symbols.begin(),
-                                               E = CP.Obj.Symbols.end();
-       I != E; ++I) {
-    SymbolTableIndexMap[I->Name] = CurSymbol;
-    CurSymbol += 1 + I->Header.NumberOfAuxSymbols;
+  for (const COFFYAML::Symbol &Sym : CP.Obj.Symbols) {
+    SymbolTableIndexMap[Sym.Name] = CurSymbol;
+    CurSymbol += 1 + Sym.Header.NumberOfAuxSymbols;
   }
 
   // Output section data.
diff --git a/llvm/lib/Passes/StandardInstrumentations.cpp 
b/llvm/lib/Passes/StandardInstrumentations.cpp
index 8e6be6730ea4..27a6c519ff82 100644
--- a/llvm/lib/Passes/StandardInstrumentations.cpp
+++ b/llvm/lib/Passes/StandardInstrumentations.cpp
@@ -225,8 +225,8 @@ std::string doSystemDiff(StringRef Before, StringRef After,
     return "Unable to read result.";
 
   // Clean up.
-  for (unsigned I = 0; I < NumFiles; ++I) {
-    std::error_code EC = sys::fs::remove(FileName[I]);
+  for (const std::string &I : FileName) {
+    std::error_code EC = sys::fs::remove(I);
     if (EC)
       return "Unable to remove temporary file.";
   }
diff --git a/llvm/lib/ProfileData/InstrProf.cpp 
b/llvm/lib/ProfileData/InstrProf.cpp
index 1168ad27fe52..ab3487ecffe8 100644
--- a/llvm/lib/ProfileData/InstrProf.cpp
+++ b/llvm/lib/ProfileData/InstrProf.cpp
@@ -657,19 +657,18 @@ void 
InstrProfValueSiteRecord::merge(InstrProfValueSiteRecord &Input,
   Input.sortByTargetValues();
   auto I = ValueData.begin();
   auto IE = ValueData.end();
-  for (auto J = Input.ValueData.begin(), JE = Input.ValueData.end(); J != JE;
-       ++J) {
-    while (I != IE && I->Value < J->Value)
+  for (const InstrProfValueData &J : Input.ValueData) {
+    while (I != IE && I->Value < J.Value)
       ++I;
-    if (I != IE && I->Value == J->Value) {
+    if (I != IE && I->Value == J.Value) {
       bool Overflowed;
-      I->Count = SaturatingMultiplyAdd(J->Count, Weight, I->Count, 
&Overflowed);
+      I->Count = SaturatingMultiplyAdd(J.Count, Weight, I->Count, &Overflowed);
       if (Overflowed)
         Warn(instrprof_error::counter_overflow);
       ++I;
       continue;
     }
-    ValueData.insert(I, *J);
+    ValueData.insert(I, J);
   }
 }
 
diff --git a/llvm/lib/Target/Hexagon/HexagonCommonGEP.cpp 
b/llvm/lib/Target/Hexagon/HexagonCommonGEP.cpp
index 43f0758f6598..8c3b9572201e 100644
--- a/llvm/lib/Target/Hexagon/HexagonCommonGEP.cpp
+++ b/llvm/lib/Target/Hexagon/HexagonCommonGEP.cpp
@@ -476,10 +476,10 @@ namespace {
 } // end anonymous namespace
 
 static const NodeSet *node_class(GepNode *N, NodeSymRel &Rel) {
-    for (NodeSymRel::iterator I = Rel.begin(), E = Rel.end(); I != E; ++I)
-      if (I->count(N))
-        return &*I;
-    return nullptr;
+  for (const NodeSet &S : Rel)
+    if (S.count(N))
+      return &S;
+  return nullptr;
 }
 
   // Create an ordered pair of GepNode pointers. The pair will be used in
@@ -589,9 +589,8 @@ void HexagonCommonGEP::common() {
       dbgs() << "{ " << I->first << ", " << I->second << " }\n";
 
     dbgs() << "Gep equivalence classes:\n";
-    for (NodeSymRel::iterator I = EqRel.begin(), E = EqRel.end(); I != E; ++I) 
{
+    for (const NodeSet &S : EqRel) {
       dbgs() << '{';
-      const NodeSet &S = *I;
       for (NodeSet::const_iterator J = S.begin(), F = S.end(); J != F; ++J) {
         if (J != S.begin())
           dbgs() << ',';
@@ -604,8 +603,7 @@ void HexagonCommonGEP::common() {
   // Create a projection from a NodeSet to the minimal element in it.
   using ProjMap = std::map<const NodeSet *, GepNode *>;
   ProjMap PM;
-  for (NodeSymRel::iterator I = EqRel.begin(), E = EqRel.end(); I != E; ++I) {
-    const NodeSet &S = *I;
+  for (const NodeSet &S : EqRel) {
     GepNode *Min = *std::min_element(S.begin(), S.end(), NodeOrder);
     std::pair<ProjMap::iterator,bool> Ins = PM.insert(std::make_pair(&S, Min));
     (void)Ins;
@@ -1280,8 +1278,8 @@ bool HexagonCommonGEP::runOnFunction(Function &F) {
     return false;
 
   // For now bail out on C++ exception handling.
-  for (Function::iterator A = F.begin(), Z = F.end(); A != Z; ++A)
-    for (BasicBlock::iterator I = A->begin(), E = A->end(); I != E; ++I)
+  for (const BasicBlock &BB : F)
+    for (const Instruction &I : BB)
       if (isa<InvokeInst>(I) || isa<LandingPadInst>(I))
         return false;
 
diff --git a/llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp 
b/llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
index c2ddfd6164f4..c35e67d6726f 100644
--- a/llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
+++ b/llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
@@ -130,10 +130,8 @@ VisitGlobalVariableForEmission(const GlobalVariable *GV,
   for (unsigned i = 0, e = GV->getNumOperands(); i != e; ++i)
     DiscoverDependentGlobals(GV->getOperand(i), Others);
 
-  for (DenseSet<const GlobalVariable *>::iterator I = Others.begin(),
-                                                  E = Others.end();
-       I != E; ++I)
-    VisitGlobalVariableForEmission(*I, Order, Visited, Visiting);
+  for (const GlobalVariable *GV : Others)
+    VisitGlobalVariableForEmission(GV, Order, Visited, Visiting);
 
   // Now we can visit ourself
   Order.push_back(GV);
@@ -699,35 +697,33 @@ static bool useFuncSeen(const Constant *C,
 
 void NVPTXAsmPrinter::emitDeclarations(const Module &M, raw_ostream &O) {
   DenseMap<const Function *, bool> seenMap;
-  for (Module::const_iterator FI = M.begin(), FE = M.end(); FI != FE; ++FI) {
-    const Function *F = &*FI;
-
-    if (F->getAttributes().hasFnAttr("nvptx-libcall-callee")) {
-      emitDeclaration(F, O);
+  for (const Function &F : M) {
+    if (F.getAttributes().hasFnAttr("nvptx-libcall-callee")) {
+      emitDeclaration(&F, O);
       continue;
     }
 
-    if (F->isDeclaration()) {
-      if (F->use_empty())
+    if (F.isDeclaration()) {
+      if (F.use_empty())
         continue;
-      if (F->getIntrinsicID())
+      if (F.getIntrinsicID())
         continue;
-      emitDeclaration(F, O);
+      emitDeclaration(&F, O);
       continue;
     }
-    for (const User *U : F->users()) {
+    for (const User *U : F.users()) {
       if (const Constant *C = dyn_cast<Constant>(U)) {
         if (usedInGlobalVarDef(C)) {
           // The use is in the initialization of a global variable
           // that is a function pointer, so print a declaration
           // for the original function
-          emitDeclaration(F, O);
+          emitDeclaration(&F, O);
           break;
         }
         // Emit a declaration of this function if the function that
         // uses this constant expr has already been seen.
         if (useFuncSeen(C, seenMap)) {
-          emitDeclaration(F, O);
+          emitDeclaration(&F, O);
           break;
         }
       }
@@ -746,11 +742,11 @@ void NVPTXAsmPrinter::emitDeclarations(const Module &M, 
raw_ostream &O) {
       // appearing in the module before the callee. so print out
       // a declaration for the callee.
       if (seenMap.find(caller) != seenMap.end()) {
-        emitDeclaration(F, O);
+        emitDeclaration(&F, O);
         break;
       }
     }
-    seenMap[F] = true;
+    seenMap[&F] = true;
   }
 }
 
</cut>
_______________________________________________
linaro-toolchain mailing list
linaro-toolchain@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/linaro-toolchain

Reply via email to