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

[Piotr Padlewski via llvm-commits]

Prazek marked 4 inline comments as done.

================
Comment at: lib/Transforms/IPO/FunctionImport.cpp:296
@@ +295,3 @@
+      BonusMultiplier = ImportHotMultiplier;
+    else if (Edge.second.Hotness == CalleeInfo::HotnessType::Cold)
+      BonusMultiplier = ImportColdMultiplier;
----------------
mehdi_amini wrote:
> Prazek wrote:
> > mehdi_amini wrote:
> > > Oh I see why you did it. Maybe a static function close to the cl::opt?
> > > 
> > > `const float BonusMultiplier = getBonusMultiplierForHotness(Edge.second.Hotness);`
> > > 
> > > (Straw man naming)
> > what about
> >   auto GetBonusMultiplier = [](CalleeInfo::HotnessType Hotness) {
> >       if (Hotness == CalleeInfo::HotnessType::Hot)
> >         return ImportHotMultiplier;
> >       if (Hotness == CalleeInfo::HotnessType::Cold)
> >         return ImportColdMultiplier;
> >       return 1.0;
> >     };
> >     
> >     const auto NewThreshold = Threshold * GetBonusMultiplier(Edge.second.Hotness);
> That's fine with me.
> 
> (I wouldn't use a lambda as there is not much value seeing this boilerplate when reading this loop body, but I don't mind if you prefer it).
I prefere lambda because it has smaller scope as function, and you don't have to jump in the file to read 5 simple lines.
So I will stay with it.


https://reviews.llvm.org/D24940