[PATCH] D44519: Add llvm-exegesis tool.

[Clement Courbet via Phabricator via llvm-commits]

courbet added inline comments.


================
Comment at: tools/llvm-exegesis/lib/BenchmarkRunner.cpp:19
+
+BenchmarkRunner::InstructionFilter::~InstructionFilter() = default;
+
----------------
RKSimon wrote:
> Move to header?
I'm using the dtor as vtable anchor.


================
Comment at: tools/llvm-exegesis/lib/BenchmarkRunner.cpp:21
+
+BenchmarkRunner::~BenchmarkRunner() = default;
+
----------------
RKSimon wrote:
> Move to header?
ditto.


================
Comment at: tools/llvm-exegesis/lib/InstructionSnippetGenerator.cpp:55
+  if (IsDef == false)
+    Var.IsUse = true;
+  Var.ExplicitOperands.push_back(OpIndex);
----------------
RKSimon wrote:
> ```
>   Var.IsDef = IsDef;
>   Var.IsUse = !IsDef;
> ```
We're actually conditionally updating the Var values here.


================
Comment at: tools/llvm-exegesis/lib/InstructionSnippetGenerator.cpp:72
+    if (llvm::is_contained(Var.ExplicitOperands, OpIndex))
+      return Var;
+  }
----------------
RKSimon wrote:
> use llvm::any_of ?
We're returning Var here, so I'm not sure I can.


================
Comment at: tools/llvm-exegesis/lib/OperandGraph.h:59
+  void connect(const Node From, const Node To);
+  void disconnect(const Node From, const Node To);
+
----------------
RKSimon wrote:
> Should these be pass by value or ref?
Node is very small, we decided to pass by value.


Repository:
  rL LLVM

https://reviews.llvm.org/D44519