[PATCH] D81050: [llvm-exegesis] Let Counter returns up to 16 entries.

[Vy Nguyen via Phabricator via llvm-commits]

oontvoo added inline comments.


================
Comment at: llvm/tools/llvm-exegesis/lib/LatencyBenchmarkRunner.cpp:42
+
 Expected<std::vector<BenchmarkMeasure>> LatencyBenchmarkRunner::runMeasurements(
     const FunctionExecutor &Executor) const {
----------------
courbet wrote:
> So computing the min and stddev across values in `runAndMeasureMulti()` requires them to be measuring the same thing. From the documentation it's not clear to me what the values represent. Are they always homogeneous ? Can you give an example of what they are in the LBR case.




Yes, they're supposed to be homogenous, representing the measurements of the same block sampled at fixed rate.

For the LBR specifically: 

* Each value in the vector is the number of elapsed cycles since last branch-retire
* When we take a sample, we have 16 of such values, representing the last 16 branches.
* If we have a loop whose body is a basic-block, then effectively these are the measurements for the last 16 iterations[0].  if the loop body has some branches then we'd need a different aggregation strategy. 

[0] I think this is where it's a bit "wrong" right now. We always only read the last 16 branches. What we probably want is to be able to have the BenchmarkRunner pause at fixed period and take a sample.  


*** (Just made a note so we could talk about this tomorrow @courbet  @ondrasej )





================
Comment at: llvm/tools/llvm-exegesis/lib/LatencyBenchmarkRunner.cpp:56
+    // We'll keep the reading with lowest stdev (ie., most stable)
+    double CurStdev = 0;
+    if (WithMinStdev.empty() ||
----------------
courbet wrote:
> Technically you're computing a variance.
¯\_(ツ)_/¯ indeed!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81050