<div dir="ltr">Thanks Evgenii.<div><br></div><div>I ran the 'check-llvm' target before submitting and it was successful.</div><div><br></div><div>I guess I forgot to provide a CMake option to get -Werror turned on.</div><div><br></div><div>It's trivial to fix the issue, and I'm running checks before fixing forward.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Oct 12, 2017 at 4:23 PM, Evgenii Stepanov <span dir="ltr"><<a href="mailto:eugeni.stepanov@gmail.com" target="_blank">eugeni.stepanov@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Please test your code with -Werror before committing.<br>
<br>
/code/llvm-project/llvm/tools/<wbr>llvm-xray/xray-stacks.cc:801:<wbr>7: error:<br>
default label in switch which covers all enumeration values<br>
[-Werror,-Wcovered-switch-<wbr>default]<br>
default:<br>
^<br>
/code/llvm-project/llvm/tools/<wbr>llvm-xray/xray-stacks.cc:816:<wbr>7: error:<br>
default label in switch which covers all enumeration values<br>
[-Werror,-Wcovered-switch-<wbr>default]<br>
default:<br>
^<br>
2 errors generated.<br>
<br>
On Thu, Oct 12, 2017 at 3:47 PM, Keith Wyss via llvm-commits<br>
<<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br>
> Author: kpw<br>
> Date: Thu Oct 12 15:47:42 2017<br>
> New Revision: 315635<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=315635&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=315635&view=rev</a><br>
> Log:<br>
> [XRay][tools] Updated stacks tool with flamegraph output.<br>
><br>
> Summary:<br>
> As the first step to allow analysis and visualization of xray collected data,<br>
> allow using the llvm-xray stacks tool to emit a complete listing of stacks in<br>
> the format consumable by a flamegraph tool.<br>
><br>
> Possible follow up formats include chrome trace viewer format and sql load<br>
> files.<br>
><br>
> As a POC, I'm able to generate flamegraphs of an xray instrumented llc compiling<br>
> hello world.<br>
><br>
> Reviewers: dberris, pelikan<br>
><br>
> Subscribers: llvm-commits<br>
><br>
> Differential Revision: <a href="https://reviews.llvm.org/D38650" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D38650</a><br>
><br>
> Modified:<br>
> llvm/trunk/tools/llvm-xray/<wbr>xray-stacks.cc<br>
><br>
> Modified: llvm/trunk/tools/llvm-xray/<wbr>xray-stacks.cc<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-xray/xray-stacks.cc?rev=315635&r1=315634&r2=315635&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/tools/llvm-<wbr>xray/xray-stacks.cc?rev=<wbr>315635&r1=315634&r2=315635&<wbr>view=diff</a><br>
> ==============================<wbr>==============================<wbr>==================<br>
> --- llvm/trunk/tools/llvm-xray/<wbr>xray-stacks.cc (original)<br>
> +++ llvm/trunk/tools/llvm-xray/<wbr>xray-stacks.cc Thu Oct 12 15:47:42 2017<br>
> @@ -66,8 +66,52 @@ static cl::opt<bool><br>
> cl::desc("Aggregate stack times across threads"),<br>
> cl::sub(Stack), cl::init(false));<br>
><br>
> -/// A helper struct to work with formatv and XRayRecords. Makes it easier to use<br>
> -/// instrumentation map names or addresses in formatted output.<br>
> +static cl::opt<bool><br>
> + DumpAllStacks("all-stacks",<br>
> + cl::desc("Dump sum of timings for all stacks. "<br>
> + "By default separates stacks per-thread."),<br>
> + cl::sub(Stack), cl::init(false));<br>
> +static cl::alias DumpAllStacksShort("all", cl::aliasopt(DumpAllStacks),<br>
> + cl::desc("Alias for -all-stacks"),<br>
> + cl::sub(Stack));<br>
> +<br>
> +// TODO(kpw): Add other interesting formats. Perhaps chrome trace viewer format<br>
> +// possibly with aggregations or just a linear trace of timings.<br>
> +enum StackOutputFormat { HUMAN, FLAMETOOL };<br>
> +<br>
> +static cl::opt<StackOutputFormat> StacksOutputFormat(<br>
> + "stack-format",<br>
> + cl::desc("The format that output stacks should be "<br>
> + "output in. Only applies with all-stacks."),<br>
> + cl::values(<br>
> + clEnumValN(HUMAN, "human",<br>
> + "Human readable output. Only valid without -all-stacks."),<br>
> + clEnumValN(FLAMETOOL, "flame",<br>
> + "Format consumable by Brendan Gregg's FlameGraph tool. "<br>
> + "Only valid with -all-stacks.")),<br>
> + cl::sub(Stack), cl::init(HUMAN));<br>
> +<br>
> +// Types of values for each stack in a CallTrie.<br>
> +enum class AggregationType {<br>
> + TOTAL_TIME, // The total time spent in a stack and its callees.<br>
> + INVOCATION_COUNT // The number of times the stack was invoked.<br>
> +};<br>
> +<br>
> +static cl::opt<AggregationType> RequestedAggregation(<br>
> + "aggregation-type",<br>
> + cl::desc("The type of aggregation to do on call stacks."),<br>
> + cl::values(<br>
> + clEnumValN(<br>
> + AggregationType::TOTAL_TIME, "time",<br>
> + "Capture the total time spent in an all invocations of a stack."),<br>
> + clEnumValN(AggregationType::<wbr>INVOCATION_COUNT, "count",<br>
> + "Capture the number of times a stack was invoked. "<br>
> + "In flamegraph mode, this count also includes invocations "<br>
> + "of all callees.")),<br>
> + cl::sub(Stack), cl::init(AggregationType::<wbr>TOTAL_TIME));<br>
> +<br>
> +/// A helper struct to work with formatv and XRayRecords. Makes it easier to<br>
> +/// use instrumentation map names or addresses in formatted output.<br>
> struct format_xray_record : public FormatAdapter<XRayRecord> {<br>
> explicit format_xray_record(XRayRecord record,<br>
> const FuncIdConversionHelper &conv)<br>
> @@ -274,10 +318,45 @@ TrieNode *mergeTrieNodes(const TrieNode<br>
> return Node;<br>
> }<br>
><br>
> +template <AggregationType AggType><br>
> +std::size_t GetValueForStack(const TrieNode *Node);<br>
> +<br>
> +// When computing total time spent in a stack, we're adding the timings from<br>
> +// its callees and the timings from when it was a leaf.<br>
> +template <><br>
> +std::size_t<br>
> +GetValueForStack<<wbr>AggregationType::TOTAL_TIME>(<wbr>const TrieNode *Node) {<br>
> + auto TopSum = std::accumulate(Node-><wbr>TerminalDurations.begin(),<br>
> + Node->TerminalDurations.end(), 0uLL);<br>
> + return std::accumulate(Node-><wbr>IntermediateDurations.begin(),<br>
> + Node->IntermediateDurations.<wbr>end(), TopSum);<br>
> +}<br>
> +<br>
> +// Calculates how many times a function was invoked.<br>
> +// TODO: Hook up option to produce stacks<br>
> +template <><br>
> +std::size_t<br>
> +GetValueForStack<<wbr>AggregationType::INVOCATION_<wbr>COUNT>(const TrieNode *Node) {<br>
> + return Node->TerminalDurations.size() + Node->IntermediateDurations.<wbr>size();<br>
> +}<br>
> +<br>
> +// Make sure there are implementations for each enum value.<br>
> +template <AggregationType T> struct DependentFalseType : std::false_type {};<br>
> +<br>
> +template <AggregationType AggType><br>
> +std::size_t GetValueForStack(const TrieNode *Node) {<br>
> + static_assert(<wbr>DependentFalseType<AggType>::<wbr>value,<br>
> + "No implementation found for aggregation type provided.");<br>
> + return 0;<br>
> +}<br>
> +<br>
> class StackTrie {<br>
> + // Avoid the magic number of 4 propagated through the code with an alias.<br>
> + // We use this SmallVector to track the root nodes in a call graph.<br>
> + using RootVector = SmallVector<TrieNode *, 4>;<br>
><br>
> // We maintain pointers to the roots of the tries we see.<br>
> - DenseMap<uint32_t, SmallVector<TrieNode *, 4>> Roots;<br>
> + DenseMap<uint32_t, RootVector> Roots;<br>
><br>
> // We make sure all the nodes are accounted for in this list.<br>
> std::forward_list<TrieNode> NodeStore;<br>
> @@ -439,11 +518,23 @@ public:<br>
> }<br>
> }<br>
><br>
> + /// Prints timing sums for each stack in each threads.<br>
> + template <AggregationType AggType><br>
> + void printAllPerThread(raw_ostream &OS, FuncIdConversionHelper &FN,<br>
> + StackOutputFormat format) {<br>
> + for (auto iter : Roots) {<br>
> + uint32_t threadId = iter.first;<br>
> + RootVector &perThreadRoots = iter.second;<br>
> + bool reportThreadId = true;<br>
> + printAll<AggType>(OS, FN, perThreadRoots, threadId, reportThreadId);<br>
> + }<br>
> + }<br>
> +<br>
> /// Prints top stacks from looking at all the leaves and ignoring thread IDs.<br>
> /// Stacks that consist of the same function IDs but were called in different<br>
> /// thread IDs are not considered unique in this printout.<br>
> void printIgnoringThreads(raw_<wbr>ostream &OS, FuncIdConversionHelper &FN) {<br>
> - SmallVector<TrieNode *, 4> RootValues;<br>
> + RootVector RootValues;<br>
><br>
> // Function to pull the values out of a map iterator.<br>
> using RootsType = decltype(Roots.begin())::<wbr>value_type;<br>
> @@ -459,30 +550,88 @@ public:<br>
> print(OS, FN, RootValues);<br>
> }<br>
><br>
> - /// Merges the trie by thread id before printing top stacks.<br>
> - void printAggregatingThreads(raw_<wbr>ostream &OS, FuncIdConversionHelper &FN) {<br>
> - std::forward_list<TrieNode> AggregatedNodeStore;<br>
> - SmallVector<TrieNode *, 4> RootValues;<br>
> + /// Creates a merged list of Tries for unique stacks that disregards their<br>
> + /// thread IDs.<br>
> + RootVector mergeAcrossThreads(std::<wbr>forward_list<TrieNode> &NodeStore) {<br>
> + RootVector MergedByThreadRoots;<br>
> for (auto MapIter : Roots) {<br>
> const auto &RootNodeVector = MapIter.second;<br>
> for (auto *Node : RootNodeVector) {<br>
> - auto MaybeFoundIter = find_if(RootValues, [Node](TrieNode *elem) {<br>
> - return Node->FuncId == elem->FuncId;<br>
> - });<br>
> - if (MaybeFoundIter == RootValues.end()) {<br>
> - RootValues.push_back(Node);<br>
> + auto MaybeFoundIter =<br>
> + find_if(MergedByThreadRoots, [Node](TrieNode *elem) {<br>
> + return Node->FuncId == elem->FuncId;<br>
> + });<br>
> + if (MaybeFoundIter == MergedByThreadRoots.end()) {<br>
> + MergedByThreadRoots.push_back(<wbr>Node);<br>
> } else {<br>
> - RootValues.push_back(<wbr>mergeTrieNodes(**<wbr>MaybeFoundIter, *Node, nullptr,<br>
> - AggregatedNodeStore));<br>
> - RootValues.erase(<wbr>MaybeFoundIter);<br>
> + MergedByThreadRoots.push_back(<br>
> + mergeTrieNodes(**<wbr>MaybeFoundIter, *Node, nullptr, NodeStore));<br>
> + MergedByThreadRoots.erase(<wbr>MaybeFoundIter);<br>
> }<br>
> }<br>
> }<br>
> - print(OS, FN, RootValues);<br>
> + return MergedByThreadRoots;<br>
> + }<br>
> +<br>
> + /// Print timing sums for all stacks merged by Thread ID.<br>
> + template <AggregationType AggType><br>
> + void printAllAggregatingThreads(<wbr>raw_ostream &OS, FuncIdConversionHelper &FN,<br>
> + StackOutputFormat format) {<br>
> + std::forward_list<TrieNode> AggregatedNodeStore;<br>
> + RootVector MergedByThreadRoots = mergeAcrossThreads(<wbr>AggregatedNodeStore);<br>
> + bool reportThreadId = false;<br>
> + printAll<AggType>(OS, FN, MergedByThreadRoots,<br>
> + /*threadId*/ 0, reportThreadId);<br>
> + }<br>
> +<br>
> + /// Merges the trie by thread id before printing top stacks.<br>
> + void printAggregatingThreads(raw_<wbr>ostream &OS, FuncIdConversionHelper &FN) {<br>
> + std::forward_list<TrieNode> AggregatedNodeStore;<br>
> + RootVector MergedByThreadRoots = mergeAcrossThreads(<wbr>AggregatedNodeStore);<br>
> + print(OS, FN, MergedByThreadRoots);<br>
> + }<br>
> +<br>
> + // TODO: Add a format option when more than one are supported.<br>
> + template <AggregationType AggType><br>
> + void printAll(raw_ostream &OS, FuncIdConversionHelper &FN,<br>
> + RootVector RootValues, uint32_t ThreadId, bool ReportThread) {<br>
> + SmallVector<const TrieNode *, 16> S;<br>
> + for (const auto *N : RootValues) {<br>
> + S.clear();<br>
> + S.push_back(N);<br>
> + while (!S.empty()) {<br>
> + auto *Top = S.pop_back_val();<br>
> + printSingleStack<AggType>(OS, FN, ReportThread, ThreadId, Top);<br>
> + for (const auto *C : Top->Callees)<br>
> + S.push_back(C);<br>
> + }<br>
> + }<br>
> + }<br>
> +<br>
> + /// Prints values for stacks in a format consumable for the <a href="http://flamegraph.pl" rel="noreferrer" target="_blank">flamegraph.pl</a><br>
> + /// tool. This is a line based format that lists each level in the stack<br>
> + /// hierarchy in a semicolon delimited form followed by a space and a numeric<br>
> + /// value. If breaking down by thread, the thread ID will be added as the<br>
> + /// root level of the stack.<br>
> + template <AggregationType AggType><br>
> + void printSingleStack(raw_ostream &OS, FuncIdConversionHelper &Converter,<br>
> + bool ReportThread, uint32_t ThreadId,<br>
> + const TrieNode *Node) {<br>
> + if (ReportThread)<br>
> + OS << "thread_" << ThreadId << ";";<br>
> + SmallVector<const TrieNode *, 5> lineage{};<br>
> + lineage.push_back(Node);<br>
> + while (lineage.back()->Parent != nullptr)<br>
> + lineage.push_back(lineage.<wbr>back()->Parent);<br>
> + while (!lineage.empty()) {<br>
> + OS << Converter.SymbolOrNumber(<wbr>lineage.back()->FuncId) << ";";<br>
> + lineage.pop_back();<br>
> + }<br>
> + OS << " " << GetValueForStack<AggType>(<wbr>Node) << "\n";<br>
> }<br>
><br>
> void print(raw_ostream &OS, FuncIdConversionHelper &FN,<br>
> - SmallVector<TrieNode *, 4> RootValues) {<br>
> + RootVector RootValues) {<br>
> // Go through each of the roots, and traverse the call stack, producing the<br>
> // aggregates as you go along. Remember these aggregates and stacks, and<br>
> // show summary statistics about:<br>
> @@ -502,7 +651,7 @@ public:<br>
> S.emplace_back(N);<br>
><br>
> while (!S.empty()) {<br>
> - auto Top = S.pop_back_val();<br>
> + auto *Top = S.pop_back_val();<br>
><br>
> // We only start printing the stack (by walking up the parent pointers)<br>
> // when we get to a leaf function.<br>
> @@ -587,6 +736,17 @@ static CommandRegistration Unused(&Stack<br>
> "that aggregates threads."),<br>
> std::make_error_code(std::<wbr>errc::invalid_argument));<br>
><br>
> + if (!DumpAllStacks && StacksOutputFormat != HUMAN)<br>
> + return make_error<StringError>(<br>
> + Twine("Can't specify a non-human format without -all-stacks."),<br>
> + std::make_error_code(std::<wbr>errc::invalid_argument));<br>
> +<br>
> + if (DumpAllStacks && StacksOutputFormat == HUMAN)<br>
> + return make_error<StringError>(<br>
> + Twine("You must specify a non-human format when reporting with "<br>
> + "-all-stacks."),<br>
> + std::make_error_code(std::<wbr>errc::invalid_argument));<br>
> +<br>
> symbolize::LLVMSymbolizer::<wbr>Options Opts(<br>
> symbolize::FunctionNameKind::<wbr>LinkageName, true, true, false, "");<br>
> symbolize::LLVMSymbolizer Symbolizer(Opts);<br>
> @@ -625,6 +785,44 @@ static CommandRegistration Unused(&Stack<br>
> "No instrumented calls were accounted in the input file.",<br>
> make_error_code(errc::result_<wbr>out_of_range));<br>
> }<br>
> +<br>
> + // Report the stacks in a long form mode for another tool to analyze.<br>
> + if (DumpAllStacks) {<br>
> + if (AggregateThreads) {<br>
> + switch (RequestedAggregation) {<br>
> + case AggregationType::TOTAL_TIME:<br>
> + ST.printAllAggregatingThreads<<wbr>AggregationType::TOTAL_TIME>(<br>
> + outs(), FuncIdHelper, StacksOutputFormat);<br>
> + break;<br>
> + case AggregationType::INVOCATION_<wbr>COUNT:<br>
> + ST.printAllAggregatingThreads<<wbr>AggregationType::INVOCATION_<wbr>COUNT>(<br>
> + outs(), FuncIdHelper, StacksOutputFormat);<br>
> + break;<br>
> + default:<br>
> + return make_error<StringError>(<br>
> + "Illegal value for aggregation-type.",<br>
> + make_error_code(errc::result_<wbr>out_of_range));<br>
> + }<br>
> + } else {<br>
> + switch (RequestedAggregation) {<br>
> + case AggregationType::TOTAL_TIME:<br>
> + ST.printAllPerThread<<wbr>AggregationType::TOTAL_TIME>(<wbr>outs(), FuncIdHelper,<br>
> + StacksOutputFormat);<br>
> + break;<br>
> + case AggregationType::INVOCATION_<wbr>COUNT:<br>
> + ST.printAllPerThread<<wbr>AggregationType::INVOCATION_<wbr>COUNT>(<br>
> + outs(), FuncIdHelper, StacksOutputFormat);<br>
> + break;<br>
> + default:<br>
> + return make_error<StringError>(<br>
> + "Illegal value for aggregation-type.",<br>
> + make_error_code(errc::result_<wbr>out_of_range));<br>
> + }<br>
> + }<br>
> + return Error::success();<br>
> + }<br>
> +<br>
> + // We're only outputting top stacks.<br>
> if (AggregateThreads) {<br>
> ST.printAggregatingThreads(<wbr>outs(), FuncIdHelper);<br>
> } else if (SeparateThreadStacks) {<br>
><br>
><br>
> ______________________________<wbr>_________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>