[PATCH] D36311: [ThinLTO] Add GraphTraits for FunctionSummaries

Teresa Johnson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 13 08:38:03 PDT 2017


tejohnson added a comment.

In https://reviews.llvm.org/D36311#896969, @ncharlie wrote:

> Any updates on this @tejohnson @davide ?


Sorry - didn't realize this was ready for re-review! I had a question about the tests earlier (see below). The comment wasn't marked as done. Although I see now that the test has changed a bit - did you resolve this? I will take a look this morning.



================
Comment at: test/ThinLTO/X86/module_summary_graph_traits.ll:32
+define void @l2() {
+  call void @l1()
+  ret void
----------------
ncharlie wrote:
> tejohnson wrote:
> > ncharlie wrote:
> > > Despite the indirect recursion in l1/l2, the SCCs only have 1 node in each. The loop is still detected (with the scc_iterator::hasLoop function), but there's an SCC for each function.
> > Hmm, that seems wrong? Wouldn't we incorrectly think these were NoRecurse then? Wondering also why there are 3 functions marked as "has loop". I assume that is root() - but we haven't done any inlining that would cause this right?
> I'll look into this.
Did you figure this out?


https://reviews.llvm.org/D36311





More information about the llvm-commits mailing list