[PATCH] D66411: Fix -ftime-trace breaking flame-graph assumptions

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

Code looks good.



================
Comment at: llvm/lib/Support/TimeProfiler.cpp:92
     for (const auto &E : Entries) {
-      auto StartUs = duration_cast<microseconds>(E.Start - StartTime).count();
-      auto DurUs = duration_cast<microseconds>(E.Duration).count();
+      // Cast time points to us precision rather than casting duration.
+      // This avoid truncation issues causing inner scopes overruning outer
----------------
nit: "microsecond precision" instead of "us precision", I read it wrong. Variables names seem clear, though.


================
Comment at: llvm/unittests/Support/TimeProfilerTest.cpp:19
+// Run enough iterations to reliably see this issue in Release.
+#define ITERATIONS 100000
+#else
----------------
I'm not sure this test has that much value because it will not fail deterministically if you revert the fix to bring back the truncation bugs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66411