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

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

It looks like https://reviews.llvm.org/D36850 got accidentally merged into this version of the patch -make sure to remove that.



================
Comment at: include/llvm/IR/ModuleSummaryIndex.h:754
   FunctionSummary calculateCallGraphRoot() {
+    // Functinos that have a parent will be mark in FunctionHasParent pair.
+    // Once we've marked all functions, the functions in the map that are false
----------------
s/Functinos/Functions
s/mark/marked/


================
Comment at: include/llvm/IR/ModuleSummaryIndex.h:1015
+/// Gets the first available function summary from a ValueInfo
+static FunctionSummary *getFrontFunctionSummary(ValueInfo V) {
+  FunctionSummary *F;
----------------
ncharlie wrote:
> @tejohnson 
> 
> I just wanted to double check that this functionality for getting a FunctionSummary from a ValueInfo is sane. It expects the first value in the SummaryList to be either a FunctionSummary (in which case it returns it) or an AliasSummary (in which case it gets its aliasee, expecting a FunctionSummary).
You can just invoke GlobalValueSummary::getBaseObject which will return the aliasee if it is an alias,  otherwise the this pointer. It looks like you should just assert then that the resulting object is a FunctionSummary, since the caller expects it to be one (and looks like this is just called when walking call edges, so should always be the case).


https://reviews.llvm.org/D36311