<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Mon, Jan 9, 2017 at 2:43 PM Alexis Shaw via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">varno marked 4 inline comments as done.<br class="gmail_msg">
varno added inline comments.<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: test/tools/llvm-xray/X86/graph-deduce-tail-call.yaml:8-17<br class="gmail_msg">
+#RUN: llvm-xray graph %s -i=yaml -o - -m %S/Inputs/simple-instrmap.yaml -t yaml -d -e med \<br class="gmail_msg">
+#RUN:    | FileCheck %s -check-prefix=TIME<br class="gmail_msg">
+#RUN: llvm-xray graph %s -i=yaml -o - -m %S/Inputs/simple-instrmap.yaml -t yaml -d -e 90p \<br class="gmail_msg">
+#RUN:    | FileCheck %s -check-prefix=TIME<br class="gmail_msg">
+#RUN: llvm-xray graph %s -i=yaml -o - -m %S/Inputs/simple-instrmap.yaml -t yaml -d -e 99p \<br class="gmail_msg">
+#RUN:    | FileCheck %s -check-prefix=TIME<br class="gmail_msg">
+#RUN: llvm-xray graph %s -i=yaml -o - -m %S/Inputs/simple-instrmap.yaml -t yaml -d -e max \<br class="gmail_msg">
----------------<br class="gmail_msg">
dblaikie wrote:<br class="gmail_msg">
> This seems to be passing a variety of values for '-e' but tests that the behavior is the same in all of them. I'm assuming the flag does something - do you have tests that confirm that it does the right thing? (somewhat similar for the top two test cases too)<br class="gmail_msg">
I need to add additional test cases for these,  however the test case size for -e 99p must be quite long in order for it to work. Will add in next revision of patch.<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: test/tools/llvm-xray/X86/graph-deduce-tail-call.yaml:39-40<br class="gmail_msg">
+#<br class="gmail_msg">
+# Note that by default, tail/sibling call deduction is disabled, and is enabled<br class="gmail_msg">
+# with a flag "-d" or "-deduce-sibling-calls".<br class="gmail_msg">
+#<br class="gmail_msg">
----------------<br class="gmail_msg">
dblaikie wrote:<br class="gmail_msg">
> Is deduce-sibling-calls tested?<br class="gmail_msg">
Yes. Otherwise the graph would only have two nodes with timing information<br class="gmail_msg"></blockquote><div><br></div><div>Oh, I see - I went looking for the full flag name and saw no tests passing it. I now reread the comment and see the test case is passing the short name.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: tools/llvm-xray/xray-graph.cc:211-212<br class="gmail_msg">
+  }<br class="gmail_msg">
+  std::vector<uint64_t> TempTimings;<br class="gmail_msg">
+  TempTimings.reserve(MaxCount);<br class="gmail_msg">
+  for (auto &V : IncommingEdges) {<br class="gmail_msg">
----------------<br class="gmail_msg">
dblaikie wrote:<br class="gmail_msg">
> Is this optimization worthwhile? Or could we put this as a local variable in the outer loop below - no need to clear it, etc.<br class="gmail_msg">
I think it is worth while, however I have no testing results. At this time, as I am working on a patch which changes how this code works completely due to a new data structure, I don't know.<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
<a href="https://reviews.llvm.org/D27243" rel="noreferrer" class="gmail_msg" target="_blank">https://reviews.llvm.org/D27243</a><br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
</blockquote></div></div>