[PATCH] D42311: [SyntheticCounts] Rewrite the code using only graph traits.

[Easwaran Raman via llvm-commits]

Given that there exists only one algorithm that uses CallGraphTraits (this
one) it is hard for me to argue that every algorithm will require
has_function_def and so I'll remove this.

On Mon, Jan 22, 2018 at 5:20 PM, Daniel Berlin via Phabricator <
reviews at reviews.llvm.org> wrote:

> dberlin added a comment.
>
> Is it a thing that is even different for each callgraph implementation?
> (If not, why is it a trait?)
> Even if it is, if you look, you'll see we don't try to encode random
> computed Node property differences in GraphTraits.



> Only the things that *every* graph algorithm needs (IE it caters to
> anything, not everything).


I don't understand what you've written inside the parenthesis, but I take
issue with the assertion  that "only things that every graph algorithm
needs [are encoded in traits]". As an example, DepthFirstIterator only
needs the get_entry_node, child_begin and child_end. Similarly, once can
think of a graph algorithm like connected components for which
get_entry_node makes no sense.



>   Random node property differences are handled through other trait classes
> or filtering.  This does not, at a glance, seem like a thing that literally
> every algorithm that is going to use a Callgraph needs. It looks like one
> that some may need.  You even prove this yourself.
>
> If you want to do something different here for CallGraphTraits, i'd like
> to understand what advantage it has.  At least in this patch as currently
> written, there is no advantage to has_function_def existing as a
> CallGraphTrait.
> It would work just existing fine outside of CallGraphTraits, and even as a
> templated function  local to a single file.
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D42311
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180123/02c04cdf/attachment.html>