[PATCH] D31519: [XRay][tools] Remove some assertions in llvm-xray graph

[David Blaikie via llvm-commits]

Sure - sounds plausible to me.

On Thu, Mar 30, 2017 at 5:32 PM Dean Michael Berris via Phabricator <
reviews at reviews.llvm.org> wrote:

> dberris created this revision.
>
> Assertions assuming that function calls may not have zero durations do
> not seem to hold in the wild. There are valid cases where the conversion
> of the tsc counters end up becoming zero-length durations. These
> assertions don't really hold and the algorithms don't need those to be
> true for them to work.
>
>
> https://reviews.llvm.org/D31519
>
> Files:
>   test/tools/llvm-xray/X86/graph-zero-latency-calls.yaml
>   tools/llvm-xray/xray-graph.cc
>
>
> Index: tools/llvm-xray/xray-graph.cc
> ===================================================================
> --- tools/llvm-xray/xray-graph.cc
> +++ tools/llvm-xray/xray-graph.cc
> @@ -259,7 +259,7 @@
>
>  template <typename U>
>  void GraphRenderer::getStats(U begin, U end, GraphRenderer::TimeStat &S) {
> -  assert(begin != end);
> +  if (begin == end) return;
>    std::ptrdiff_t MedianOff = S.Count / 2;
>    std::nth_element(begin, begin + MedianOff, end);
>    S.Median = *(begin + MedianOff);
> @@ -287,25 +287,20 @@
>    for (auto &E : G.edges()) {
>      auto &A = E.second;
>      assert(!A.Timings.empty());
> -    assert((A.Timings[0] > 0));
>      getStats(A.Timings.begin(), A.Timings.end(), A.S);
> -    assert(A.S.Sum > 0);
>      updateMaxStats(A.S, G.GraphEdgeMax);
>    }
>  }
>
>  void GraphRenderer::calculateVertexStatistics() {
>    std::vector<uint64_t> TempTimings;
>    for (auto &V : G.vertices()) {
> -    assert((V.first == 0 || G[V.first].S.Sum != 0) &&
> -           "Every non-root vertex should have at least one call");
>      if (V.first != 0) {
>        for (auto &E : G.inEdges(V.first)) {
>          auto &A = E.second;
>          TempTimings.insert(TempTimings.end(), A.Timings.begin(),
>                             A.Timings.end());
>        }
> -      assert(!TempTimings.empty() && TempTimings[0] > 0);
>        getStats(TempTimings.begin(), TempTimings.end(), G[V.first].S);
>        updateMaxStats(G[V.first].S, G.GraphVertexMax);
>        TempTimings.clear();
> Index: test/tools/llvm-xray/X86/graph-zero-latency-calls.yaml
> ===================================================================
> --- /dev/null
> +++ test/tools/llvm-xray/X86/graph-zero-latency-calls.yaml
> @@ -0,0 +1,20 @@
> +#RUN: llvm-xray graph %s -o - -m %S/Inputs/simple-instrmap.yaml |
> FileCheck %s
> +
> +---
> +header:
> +  version: 1
> +  type: 0
> +  constant-tsc: true
> +  nonstop-tsc: true
> +  cycle-frequency: 2601000000
> +records:
> +  - { type: 0, func-id: 1, cpu: 1, thread: 111, kind: function-enter,
> tsc: 10001 }
> +  - { type: 0, func-id: 2, cpu: 1, thread: 111, kind: function-enter,
> tsc: 10002 }
> +  - { type: 0, func-id: 2, cpu: 1, thread: 111, kind: function-exit, tsc:
> 10002 }
> +  - { type: 0, func-id: 1, cpu: 1, thread: 111, kind: function-exit, tsc:
> 10100 }
> +...
> +
> +#CHECK:     digraph xray {
> +#CHECK-DAG:   F0 -> F1 [{{.*}}];
> +#CHECK-DAG:   F1 -> F2 [{{.*}}];
> +#CHECK-DAG: }
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170331/dc8283ec/attachment.html>