[PATCH] D54393: [llvm-exegesis] Move InstructionBenchmarkClustering::() into header

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: tools/llvm-exegesis/lib/Clustering.cpp:44
     const auto &PMeasurements = Points_[P].Measurements;
     if (PMeasurements.empty()) // Error point.
       continue;
----------------
I think the emptiness check can get folded into `isNeighbour`


================
Comment at: tools/llvm-exegesis/lib/Clustering.h:92
   // Returns true if the given point is within a distance Epsilon of each other.
-  bool isNeighbour(const std::vector<BenchmarkMeasure> &P,
-                   const std::vector<BenchmarkMeasure> &Q) const;
+  inline bool isNeighbour(const std::vector<BenchmarkMeasure> &P,
+                          const std::vector<BenchmarkMeasure> &Q) const {
----------------
It is interesting that it does not get inlined... Does an `inline`specifier help?

`isNeighbour` is also used in `tools/llvm-exegesis/lib/Analysis.cpp#L503`. I'm not familiar with the code but I guess there the performance may not matter too much.


Repository:
  rL LLVM

https://reviews.llvm.org/D54393