[PATCH] D101994: [TableGen] Use range-based for loops (NFC)

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/utils/TableGen/CodeGenRegisters.cpp:587
   unsigned Weight = 0;
-  for (RegUnitList::iterator I = RegUnits.begin(), E = RegUnits.end();
-       I != E; ++I) {
-    Weight += RegBank.getRegUnit(*I).Weight;
+  for (unsigned int RegUnit : RegUnits) {
+    Weight += RegBank.getRegUnit(RegUnit).Weight;
----------------
Code in LLVM almost always uses "unsigned" instead of "unsigned int". This occurs several more times.


================
Comment at: llvm/utils/TableGen/CodeGenSchedule.cpp:1787
 bool CodeGenSchedModels::hasSuperGroup(RecVec &SubUnits, CodeGenProcModel &PM) {
-  for (unsigned i = 0, e = PM.ProcResourceDefs.size(); i < e; ++i) {
-    if (!PM.ProcResourceDefs[i]->isSubClassOf("ProcResGroup"))
+  for (auto &ProcResourceDef : PM.ProcResourceDefs) {
+    if (!ProcResourceDef->isSubClassOf("ProcResGroup"))
----------------
Why are reference? Isn't ProctResourceDef a Record *?


================
Comment at: llvm/utils/TableGen/CodeGenSchedule.cpp:1953
         } dbgs() << "\nReadAdvanceDefs: ";
-        for (RecIter RI = PM.ReadAdvanceDefs.begin(),
-             RE = PM.ReadAdvanceDefs.end();
-             RI != RE; ++RI) {
-          if ((*RI)->isSubClassOf("ReadAdvance"))
-            dbgs() << (*RI)->getValueAsDef("ReadType")->getName() << " ";
+        for (auto ReadAdvanceDef
+             : PM.ReadAdvanceDefs) {
----------------
Why can't this be on one line now?


================
Comment at: llvm/utils/TableGen/CodeGenSchedule.cpp:1953
         } dbgs() << "\nReadAdvanceDefs: ";
-        for (RecIter RI = PM.ReadAdvanceDefs.begin(),
-             RE = PM.ReadAdvanceDefs.end();
-             RI != RE; ++RI) {
-          if ((*RI)->isSubClassOf("ReadAdvance"))
-            dbgs() << (*RI)->getValueAsDef("ReadType")->getName() << " ";
+        for (auto ReadAdvanceDef
+             : PM.ReadAdvanceDefs) {
----------------
craig.topper wrote:
> Why can't this be on one line now?
This is a pointer so should be "auto *". LLVM prefers not to obscure pointers with auto. I think this applies to other places in the patch. We might even want "Record *" here. We're pretty conservative about auto unless the type is obvious for the reader.


================
Comment at: llvm/utils/TableGen/CodeGenSchedule.cpp:1962
         << "\nProcResourceDefs: ";
-        for (RecIter RI = PM.ProcResourceDefs.begin(),
-             RE = PM.ProcResourceDefs.end();
-             RI != RE; ++RI) { dbgs() << (*RI)->getName() << " "; } dbgs()
+        for (auto ProcResourceDef
+             : PM.ProcResourceDefs) {
----------------
Single line?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101994/new/

https://reviews.llvm.org/D101994