[PATCH] D68714: [MCA] Show aggregate over Average Wait times for the whole snippet (PR43219)

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: llvm/tools/llvm-mca/Views/TimelineView.cpp:154-155
+
+  int BufferSize = PrintingTotals ? std::numeric_limits<int>::max()
+                                  : UsedBuffer[SourceIndex].second;
+  tryChangeColor(OS, Entry.CyclesSpentInSchedulerQueue, CumulativeExecutions,
----------------
andreadb wrote:
> if you use `std::numeric_limits<int>::max()` then you introduce an unsigned wrap in function `chooseColor` (lines 111 and 113) when `CumulativeExecutions` is bigger than 2.
> 
> Two options:
> 1) you either avoid calling `tryChangeColor` if you are printing the totals, or
> 2) you modify function `chooseColor` so that `raw_ostream::SAVEDCOLOR` is returned when BufferSize is 0. This requires that BufferSize is initialized to 0 (here at line 154) when printing the totals.
Let's just not change color then.
This is starting to get somewhat ugly, which is what i was initially afraid of though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68714