[PATCH] D68629: [llvm-exegesis] Finish plumbing the `Config` field.

[Guillaume Chatelet via Phabricator via llvm-commits]

gchatelet added inline comments.


================
Comment at: llvm/tools/llvm-exegesis/lib/Clustering.cpp:241
+  // Given an instruction Opcode and Config, in which clusters do benchmarks of
+  // this instruction lie? Normally, they all should be in the same cluster.
+  struct OpcodeAndConfig {
----------------
Not related to this patch but why a question mark here?


================
Comment at: llvm/tools/llvm-exegesis/lib/Clustering.cpp:247
+    const std::string *Config;
+    bool operator<(const OpcodeAndConfig &O) const {
+      return std::tie(Opcode, *Config) < std::tie(O.Opcode, *O.Config);
----------------
How about factoring Tie:
```
  inline auto Tie() const { return std::tie(Opcode, *Config); }
  bool operator<(const A &O) const { return Tie() < O.Tie(); }
  bool operator==(const A &O) const { return Tie() == O.Tie(); }
```


================
Comment at: llvm/tools/llvm-exegesis/lib/Clustering.cpp:296
+          [this, &Key](size_t P) {
+            return !(OpcodeAndConfig(Points_[P]) == Key);
           });
----------------
maybe add the != operator to the struct, with the `Tie()` function it's a no brainer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68629