[PATCH] D54383: [llvm-exegesis] Analysis: writeMeasurementValue(): don't alloc string for double each time.

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 12 03:10:19 PST 2018


lebedev.ri added a comment.

In https://reviews.llvm.org/D54383#1295235, @gchatelet wrote:

> I'm not entirely convinced here:
>
> - The code is harder to read,


I can totally bloat it will some comments/variables.

> - the formatting pattern and the SmallString size may go out of sync,

*May*.
Or may be removed completely, and this change was be pointless.
Or it may stay like this forever.
Who knows?
Right now this is a clear win from perf/memory standpoint.

> - the performance win is not that big.

Given the total perf improvement this patchset provides (or rather, the starting point) i will pretend i did not notice this point :)



================
Comment at: tools/llvm-exegesis/lib/Analysis.cpp:101
+      OS, llvm::formatv("{0:F}", Value)
+              .sstr<1 + std::numeric_limits<decltype(Value)>::max_digits10>());
 }
----------------
gchatelet wrote:
> Why `1 + ...`?
You will need at most max_digits10 digits PLUS the decimal separator (`.`) to print the `double`.


Repository:
  rL LLVM

https://reviews.llvm.org/D54383





More information about the llvm-commits mailing list