[PATCH] D151019: [llvm-exegesis] Refactor FunctionExecutorImpl and create factory

Aiden Grossman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 23 18:04:50 PDT 2023

aidengrossman added inline comments.

Comment at: llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp:195
+  RC.ExecutionMode = ExecutionMode;
courbet wrote:
> A runnable configuration exists outside of how it's executed, it feels weird to have this here. Why isn't this a member of `BenchmarkRunner` ? 
Good point. I've switched it over to being a member of `BenchmarkRunner`.

Comment at: llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp:204
+    object::OwningBinary<object::ObjectFile> ObjectFile,
+    const BenchmarkKey &Key) const {
+  if (ExecutionMode == ExecutionModeE::InProcess) {
courbet wrote:
> Why do we need `Key` ?
I'm using a `BenchmarkKey` in https://reviews.llvm.org/D151025 (and touching it in some previous patches in the stack) to pass around memory annotations, and the way I have it structured currently, I pass the key to the `FunctionExecutorImpl` for setup. I can move this to another diff/change this around if desired.

  rG LLVM Github Monorepo



More information about the llvm-commits mailing list