[PATCH] D140702: [exegesis] "Skip codegen" dry-run mode

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: llvm/tools/llvm-exegesis/lib/BenchmarkRunner.h:40
+                           bool BenchmarkSkipCodegen,
                            bool BenchmarkSkipMeasurements);
 
----------------
gchatelet wrote:
> [nit] It's probably better to restructure the `BenchmarkRunner` to only run the benchmark instead of also being responsible for doing codegen.
> 
> Also passing multiple consecutive `bool` arguments seems like a red flag. Using an enum seems more compelling (since one flag implies the other).
> [nit] It's probably better to restructure the BenchmarkRunner to only run the benchmark instead of also being responsible for doing codegen.

The separation of `BenchmarkRunner::getRunnableConfiguration()` and `BenchmarkRunner::runConfiguration()`
only appeared in refactorings towards parallelization: D140271
It might be doable, but i'm not sure if it makes sense yet.

> Also passing multiple consecutive bool arguments seems like a red flag. Using an enum seems more compelling (since one flag implies the other).

Yeah, i've tried that. Let me try once more i guess.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140702