[PATCH] D139797: [exegesis] Benchmark: provide optional progress meter / ETA

[Clement Courbet via Phabricator via llvm-commits]

courbet added inline comments.


================
Comment at: llvm/tools/llvm-exegesis/lib/ProgressMeter.h:60
+inline AvgTy SimpleMovingAverage<NumTy, DenTy>::getAverage() const {
+  return AvgTy(Accumulated) / Steps;
+}
----------------
You should define what `getAverage` is when `step()` has never been called rather then crash here. Ar at least `assert` explicitly.


================
Comment at: llvm/tools/llvm-exegesis/lib/ProgressMeter.h:125
+ProgressMeter<ClockType>::ProgressMeter(int NumStepsTotal_, raw_ostream &out_)
+    : out(out_), NumStepsTotal(NumStepsTotal_) {}
+
----------------
`assert(NumStepsTotal > 0);`


================
Comment at: llvm/tools/llvm-exegesis/lib/ProgressMeter.h:139-140
+  int Seconds = Unaccounted % 60;
+  Unaccounted /= 60;
+  int Minutes = Unaccounted;
+
----------------
You're not using `Unaccounted` afterwards, so:

```
int Seconds = Unaccounted % 60;
int Minutes = Unaccounted / 60;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139797