[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