[PATCH] D77422: [llvm-exegesis] Add benchmark mode that uses LBR for more precise measurements.

[Clement Courbet via Phabricator via llvm-commits]

courbet added a comment.

Only style comments left



================
Comment at: llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp:110
+
+      auto *From = (void *)Function.getFunctionBytes().data();
+      auto *To = (void *)(((char *)From) + Function.getFunctionBytes().size());
----------------
let's use a c++ cast here.


================
Comment at: llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp:112
+      auto *To = (void *)(((char *)From) + Function.getFunctionBytes().size());
+      auto ValueOrError = Counter->readOrError(From, To);
       if (!ValueOrError)
----------------
Actually I'm thinking you can just pass a `StringRef` to `readOrError`


================
Comment at: llvm/tools/llvm-exegesis/lib/PerfHelper.h:90
 
   /// Returns the current value of the counter or error if it cannot be read.
+  virtual llvm::Expected<llvm::SmallVector<int64_t, 4>>
----------------
Please explain what the arguments are (and switch to StringRef as mentioned above ?)


================
Comment at: llvm/tools/llvm-exegesis/lib/X86/X86Counter.cpp:149
+                       PROT_READ | PROT_WRITE, MAP_SHARED, FileDescriptor, 0);
+  if (MMappedBuffer == MAP_FAILED) {
+    llvm::errs() << "Failed to mmap buffer.";
----------------
braces


================
Comment at: llvm/tools/llvm-exegesis/lib/X86/X86Counter.cpp:204
+
+  if (!error) {
+    return CycleArray;
----------------
style: no braces


================
Comment at: llvm/tools/llvm-exegesis/lib/X86/X86Counter.h:8
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_TOOLS_LLVM_EXEGESIS_LIB_X86_X86COUNTER_H
----------------
This could use a file comment with a link to LBR documentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77422