[PATCH] D147700: [llvm-exegesis] Allow setting dump file name

[Clement Courbet via Phabricator via llvm-commits]

courbet added inline comments.


================
Comment at: llvm/tools/llvm-exegesis/lib/BenchmarkRunner.h:67-68
   Expected<Benchmark> runConfiguration(RunnableConfiguration &&RC,
-                                                  bool DumpObjectToDisk) const;
+                                       bool DumpObjectToDisk,
+                                       StringRef DumpFileName) const;
 
----------------
kpdev42 wrote:
> courbet wrote:
> > kpdev42 wrote:
> > > courbet wrote:
> > > > Given that `DumpFileName` only makes sense if `DumpObjectToDisk`, use a single `optional<StringRef>` parameter ?
> > > If we provide just -dump-object-to-disk then llvm-exegesis will use autogenerated name. This is useful when batch opcode testing is used (with -opcode-index=-1). We can use some special file name for this mode, but it looks slightly counter-intuitive. 
> > > If we provide just -dump-object-to-disk then llvm-exegesis will use autogenerated name
> > 
> > Yes, and that would be `{""}`. My point is that we can condense two parameters in one and ensure that we can't write `runConfiguration(RC, false, "file")`, which makes no sense.
> Oh, sorry, now I understood what you meant. I will update the revision
I don't see the `optional<StringRef>` :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147700