<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jan 22, 2018 at 5:20 PM, Daniel Berlin via Phabricator <span dir="ltr"><<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">dberlin added a comment. <br></blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Is it a thing that is even different for each callgraph implementation? (If not, why is it a trait?)<br></blockquote><div><br></div><div><br></div><div>My understanding is that it is different -- at least between regular callgraph and summary based callgraph used in ThinLTO (thinlink) phase.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
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.<br>
<br></blockquote><div><br></div><div>I see it differently -- this property does not look like some random property, but something native to callgraph nodes.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
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.<br>
It would work just existing fine outside of CallGraphTraits, and even as a templated function  local to a single file.<br>
<div class="HOEnZb"><div class="h5"><br></div></div></blockquote><div><br></div><div>This is not limited to single file, but shared across module callgraph and thinLink CG.</div><div><br></div><div>David</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">
<br>
Repository:<br>
  rL LLVM<br>
<br>
<a href="https://reviews.llvm.org/D42311" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D42311</a><br>
<br>
<br>
<br>
</div></div></blockquote></div><br></div></div>