[PATCH] D30053: Add function importing info from samplepgo profile to the module summary.

Mehdi AMINI via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 16 16:51:12 PST 2017


mehdi_amini added inline comments.


================
Comment at: lib/Analysis/ModuleSummaryAnalysis.cpp:188
+    CallGraphEdges[I].updateHotness(CalleeInfo::HotnessType::Hot);
+
   bool NonRenamableLocal = isNonRenamableLocal(F);
----------------
danielcdh wrote:
> mehdi_amini wrote:
> > danielcdh wrote:
> > > mehdi_amini wrote:
> > > > I'm still unsure why we need this side channel to communicate the hotness, while we have above already some infrastructure? Why isn't samplePGO integrate in the general PGO infrastructure?
> > > The root cause is the profile annotation mechanism is different.
> > > 
> > > SamplePGO needs to make sure that before profile annotation, the IR resembles the hot portion of the profiling binary. As a result, we need to explicitly inline functions before profile annotation. But in order to inline, we need to first have it imported.
> > > 
> > > I can go into more details with an example if you want. Please let me know.
> > I'm still not fond of this "side channel", and I rather have a solution that annotates hot calls in the IR instead, that would be agnostic to the source of information we derive this "hotness" from.
> I think we already have a side channel for ICP?
> 
> The issue for marking "hot call" is that it only goes down by 1 level, for the cases where foo_in_a_cc()->bar_in_b_cc()->baz_in_c_cc(), we may only go one level to include bar_in_b_cc if we mark the hot call. But in practice, we need both levels to be imported and inlined.
I don't see how having one level at a time is an issue. The two calls can be marked hot and considered independently when building the chain.


https://reviews.llvm.org/D30053





More information about the llvm-commits mailing list