[PATCH] D47329: [llvm-exegesis] Analysis: Display idealized sched class port pressure.

[Guillaume Chatelet via Phabricator via llvm-commits]

gchatelet requested changes to this revision.
gchatelet added inline comments.
This revision now requires changes to proceed.


================
Comment at: tools/llvm-exegesis/lib/Analysis.cpp:309
+        "resource (port) pressure assuming ideal distribution\">Idealized "
+        "Resource "
+        "Pressure</th></tr>";
----------------
Bring last word on the same line.


================
Comment at: tools/llvm-exegesis/lib/Analysis.cpp:468
+    llvm::SmallVector<llvm::MCWriteProcResEntry, 8> WPRS) {
+  llvm::SmallVector<float, 32> DensePressure(SM.getNumProcResourceKinds());
+  llvm::sort(WPRS.begin(), WPRS.end(),
----------------
Can you describe what Dense and Sparse pressure are?


================
Comment at: tools/llvm-exegesis/lib/Analysis.cpp:485
+      float RemainingPressure = WPR.Cycles;
+      // The algorith is as follows: while there is remaining pressure to
+      // distribute, find the subunits with minimal pressure, and distribute
----------------
algorithm


================
Comment at: tools/llvm-exegesis/lib/Analysis.cpp:489
+      // second-to-minimal pressure.
+      llvm::SmallVector<uint16_t, 32> Subunits(ProcResDesc->SubUnitsIdxBegin,
+                                               ProcResDesc->SubUnitsIdxBegin +
----------------
Running an example of help understand the algorithm here.


================
Comment at: tools/llvm-exegesis/lib/Analysis.cpp:500
+      while (NumMinimalSU < Subunits.size() &&
+             DensePressure[Subunits[NumMinimalSU]] ==
+                 DensePressure[Subunits[0]]) {
----------------
This pattern appears quite often in the next few lines `DensePressure[Subunits[XXX]]`.
Maybe a lambda would help.
```
const auto getPressureForSubUnit = [](...){...};
```

Or an object to encapsulate the logic?


================
Comment at: tools/llvm-exegesis/lib/Analysis.cpp:510
+          }
+          break;
+        }
----------------
How about using a function instead of this big body?
It would make the return path easier to understand.


Repository:
  rL LLVM

https://reviews.llvm.org/D47329