[PATCH] D140271: [NFCI][llvm-exegesis] Benchmark: parallelize codegen (5x ... 8x less wallclock)

[Guillaume Chatelet via Phabricator via llvm-commits]

gchatelet added a comment.

Much better, thx.
I've added a few more comments.



================
Comment at: llvm/tools/llvm-exegesis/llvm-exegesis.cpp:408-411
+  for (auto C : zip(ConfigurationBatch, PerConfigRCs)) {
+    const BenchmarkCode &BC = std::get<0>(C);
+    SmallVectorImpl<ExpectedRunnableConfiguration> &RCsOfConfiguration =
+        std::get<1>(C);
----------------
Can you try with structured binding here (and below)?


================
Comment at: llvm/tools/llvm-exegesis/llvm-exegesis.cpp:419
+      // Prepare an output slot for the task, without invalidating iterators.
+      // Create asyncronous task to generage Runnable Configuration
+      // for this configuration given this repetitor. This is thread-safe.
----------------
typo


================
Comment at: llvm/tools/llvm-exegesis/llvm-exegesis.cpp:419
+      // Prepare an output slot for the task, without invalidating iterators.
+      // Create asyncronous task to generage Runnable Configuration
+      // for this configuration given this repetitor. This is thread-safe.
----------------
gchatelet wrote:
> typo
typo


================
Comment at: llvm/tools/llvm-exegesis/llvm-exegesis.cpp:419
+      // Prepare an output slot for the task, without invalidating iterators.
+      // Create asyncronous task to generage Runnable Configuration
+      // for this configuration given this repetitor. This is thread-safe.
----------------
gchatelet wrote:
> gchatelet wrote:
> > typo
> typo
no whitespace since we're talking about the type


================
Comment at: llvm/tools/llvm-exegesis/llvm-exegesis.cpp:440
+    const BenchmarkRunner &Runner) {
+  // And they've completed! Now, for each configuration in this batch:
+  SmallVector<InstructionBenchmark, MaxRepetitors> AllResults;
----------------
remove


================
Comment at: llvm/tools/llvm-exegesis/llvm-exegesis.cpp:444
+  AllResults.reserve(RCsOfConfiguration.size());
+  // For each Runnable Configuration per repetitor:
+  for (ExpectedRunnableConfiguration &&ERC :
----------------
ditto


================
Comment at: llvm/tools/llvm-exegesis/llvm-exegesis.cpp:464
+
+  if (RepetitionMode == InstructionBenchmark::RepetitionModeE::AggregateMin) {
+    for (const InstructionBenchmark &OtherResult :
----------------
Can we reverse this condition and return early to save on the nesting level?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140271