[PATCH] D24940: [thinlto] Add cold-callsite import heuristic

[Mehdi AMINI via llvm-commits]

mehdi_amini added inline comments.

================
Comment at: lib/Transforms/IPO/FunctionImport.cpp:305
@@ -294,3 +304,3 @@
     if (!CalleeSummary) {
       DEBUG(dbgs() << "ignored! No qualifying callee with summary found.\n");
       continue;
----------------
Prazek wrote:
> mehdi_amini wrote:
> > I agree... when there is an actual value to read these lines! Otherwise apply this principles means inlining the whole program as nested lambda into the main() :p
> > So it is always a signal/noise ratio evaluation. Having a lambda means the code is available, but it means the body of the current function is longer.
> > (Here I felt I can read and understand the code in the loop without the need to see the body of a function with a name GetBonusMultiplier)
> The boilerplate is actually not that big, there is actually one line more (because I would have to declare a variable to store result ). 
> 
> I guess the real question is, should I hide implementation or not, because wrapping in the function/lambda is clearly win.
> 
> So if you think that function will be better then I will change it because both works for me, and I want to push it upstream before I will finish internship (Friday)
I don't think it makes much of a difference here, this is why I mentioned before either way is fine, pick your favorite.
I'm fine continuing bike-shedding this though :)

I can't believe your internship is already done, time flies...


https://reviews.llvm.org/D24940