[PATCH] D59820: [llvm-exegesis] Introduce a 'naive' clustering algorithm (PR40880)

[Clement Courbet via Phabricator via llvm-commits]

courbet added inline comments.


================
Comment at: tools/llvm-exegesis/lib/Clustering.h:149
 
+class BenchmarkGroup {
+public:
----------------
You're not using `PointIds` in `BenchmarkGroup`, so please move back `PointIds` and `getPointIds` to `SchedClassCluster`.

At that point it makes more sense to call this `SchedClassClusterCentroid` and make it a member of `SchedClassCluster`, with no virtual inheritance and protected section. Something like:

```

class SchedClassClusterCentroid {
public:
  const std::vector<PerInstructionStats> &getStats() const { return Representative; }
  std::vector<BenchmarkMeasure> getAsPoint() const;
  void addPoint(ArrayRef<BenchmarkMeasure> Point);

private:
  // Measurement stats for the points in the SchedClassCluster.
  std::vector<PerInstructionStats> Representative;
};

class SchedClassCluster {
public:
  const InstructionBenchmarkClustering::ClusterId &id() const;
  const std::vector<size_t> &getPointIds() const;
  void addPoint(size_t PointId, void addPoint(size_t PointId, const InstructionBenchmarkClustering &Clustering);
  const SchedClassClusterCentroid& getCentroid() const { return Centroid; }
  bool measurementsMatch(...);

private:
  std::vector<size_t> PointIds;
  InstructionBenchmarkClustering::ClusterId ClusterId;
  SchedClassClusterCentroid Centroid;
};
```

Ad then only use `SchedClassClusterCentroid` in `areAllNeighbours()`.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59820