[PATCH] D36722: [InlineCost] Simplify the cold attribute handling in inline-cost.

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added inline comments.


================
Comment at: lib/Analysis/InlineCost.cpp:659
+  // should *always* be considered cold.
+  if (Callee.hasFnAttribute(Attribute::Cold))
+    return true;
----------------
davidxl wrote:
> chandlerc wrote:
> > davidxl wrote:
> > > IIRC, static branch prediction already handles this case. Do you see missing cases?
> > Sorry, I tried to explain in my top-level description -- there are cases where despite being cold, the call site won't be cold relative to the function entry. For example, when the call site is *in* the function entry block.
> > 
> > Maybe this still isn't making sense? I can add an example bit of code.
> There is an inconsistency between cold callsite relative freq and the cold branch probability --- it is 6% vs 2%. I think we should fix one of them -- IMO, it is more reasonable to fix the static branch prediction.
> 
> 
Sure? I don't have strong feelings about that. But it seems like that is orthogonal? (Or am I missing something?)

Before this patch, if the branch probability check below failed, we would continue onward and ask PSI and it would still say the call is cold based on this attribute. (I think...)


https://reviews.llvm.org/D36722