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

[Xinliang David Li via llvm-commits]

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?)
>


My understanding is that it is different -- at least between regular
callgraph and summary based callgraph used in ThinLTO (thinlink) phase.


> 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).
>   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.
>
>
I see it differently -- this property does not look like some random
property, but something native to callgraph nodes.


> 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.
>
>
This is not limited to single file, but shared across module callgraph and
thinLink CG.

David



>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D42311
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180122/a9eb4e1d/attachment.html>