[PATCH] D52825: [llvm-exegesis] Resolve variant classes in analysis.

[Guillaume Chatelet via Phabricator via llvm-commits]

gchatelet added inline comments.


================
Comment at: tools/llvm-exegesis/lib/Analysis.cpp:27
+  const auto &SM = STI.getSchedModel();
+  if (WasVariant) {
+    *WasVariant =
----------------
You may want to split this function in two functions, the last argument is awkward.
```
bool IsVariant(const llvm::MCSubtargetInfo &STI, unsigned SchedClassId);
unsigned resolveSchedClassId(const llvm::MCSubtargetInfo &STI, unsigned SchedClassId, const llvm::MCInst &MCI);
```


================
Comment at: tools/llvm-exegesis/lib/Analysis.h:102
 
-  // Builds a map of Sched Class -> indices of points that belong to the sched
-  // class.
-  std::unordered_map<unsigned, std::vector<size_t>>
-  makePointsPerSchedClass() const;
+  // Builds a list of (Sched Class, indices of points that belong to the sched
+  // class).
----------------
Move the documentation next to the function and add relevant documentation for the struct


================
Comment at: tools/llvm-exegesis/lib/Analysis.h:106
+    ResolvedSchedClassAndPoints(ResolvedSchedClass &&RSC)
+        : RSC(std::move(RSC)) {}
+    ResolvedSchedClass RSC;
----------------
move to cc?


================
Comment at: tools/llvm-exegesis/lib/Analysis.h:107
+        : RSC(std::move(RSC)) {}
+    ResolvedSchedClass RSC;
+    std::vector<size_t> PointIds;
----------------
newline


Repository:
  rL LLVM

https://reviews.llvm.org/D52825