[PATCH] D29320: [XRay] A tool for Comparing xray function call graphs

Dean Michael Berris via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Apr 23 22:26:41 PDT 2017


dberris accepted this revision.
dberris added a comment.
This revision is now accepted and ready to land.

LGTM -- just a couple more nits, but I'll land it first and get the follow-up changes done myself. :)

Thanks for the work @varno!



================
Comment at: tools/llvm-xray/xray-graph-diff.cc:310-315
+template <typename T> static bool containsNullptr(const T &Collection) {
+  for (const auto &E : Collection)
+    if (E == nullptr)
+      return true;
+  return false;
+}
----------------
Why isn't the caller of this just doing:

```
llvm::find(Collection, nullptr) != Collection.end()
```


================
Comment at: tools/llvm-xray/xray-graph-diff.cc:350
+    double RelDiff = statRelDiff(LeftStat, RightStat, VL);
+    return formatv(R"({{{0}|{1:P}})", truncateString(VertexId, TrunLen).str(),
+                   RelDiff);
----------------
Is it just me, or are the squiggly brackets not matched?


================
Comment at: tools/llvm-xray/xray-graph.cc:339-341
+  double TimeStat::*DoubleStatPtrs[] = {&TimeStat::Min,   &TimeStat::Median,
+                                        &TimeStat::Pct90, &TimeStat::Pct99,
+                                        &TimeStat::Max,   &TimeStat::Sum};
----------------
This could be a `static constexpr` array, so it doesn't have to be initialised every time this function is called.


https://reviews.llvm.org/D29320





More information about the llvm-commits mailing list