[PATCH] D27243: Initial work on the XRay Graph tool.

[Alexis Shaw via Phabricator via llvm-commits]

varno added inline comments.


================
Comment at: tools/llvm-xray/xray-graph.cc:121
+      VertexAttrs[Record.FuncId].SymbolName = FuncIdHelper.SymbolOrNumber(Record.FuncId);
+    ThreadStack.push_back({Record.FuncId, Record.TSC});
+    break;
----------------
dberris wrote:
> Can you use `.emplace_back(...)` instead here?
No.


================
Comment at: tools/llvm-xray/xray-graph.h:115-124
+  /// Calculates latency statistics for each edge and stores the data in the
+  /// Graph
+  void calculateEdgeStatistics(void);
+
+  ///Calculates latency statistics for each vertex and stores the data in the
+  /// Graph
+  void calculateVertexStatistics(void);
----------------
dberris wrote:
> Can this not happen incrementally, as we're adding the records, or on demand when we export? i.e. why do they need to be public functions?
Not really, all the records need to be added before we calculate the statistics. And I was wanting the statistics calculated to do graph transformations later (before output).


================
Comment at: tools/llvm-xray/xray-graph.h:126-127
+private:
+  /// A private function to facilitate the output of edge labels;
+  void outputEdgeInfo(const TimeStat &S, StatType T, raw_ostream &OS) const;
+};
----------------
dberris wrote:
> Does this need to be part of the class? Could this not be a function in the implementation?
I suppose it could be a friend function, it uses private types (the TimeStat type).


https://reviews.llvm.org/D27243