[PATCH] D22694: [Inliner, OptDiag] Add hotness attribute to opt diagnostics

[Easwaran Raman via llvm-commits]

eraman added a comment.

In https://reviews.llvm.org/D22694#500141, @anemet wrote:

> In https://reviews.llvm.org/D22694#500037, @eraman wrote:
>
> > This computes DominatorTree, LoopInfo, BPI and BFI for *every callsite* that is considered for inlining. Even though this is (I assume) turned off by default, it's still very expensive.
>
>
> Certainly it is off by default and it's meant as a performance analysis tool so the budget is different.


Yes, I understand that this is a reporting tool and so the compilation time is not a major constraint. I was wondering if this makes it practical for use in real applications. Perhaps it is. Have you run this on anything large (SPEC, for example) and measured the overhead? I don't have any objections to this patch if you have measured the overhead and found them reasonable.

>   That said, we can I guess cache ORE per caller and only invalidate it once we inline.  Does that sound reasonable to you?


That'll certainly reduce the overhead in the cases where inlining is considered, but does not happen. I am not sure if it is worth the additional complexity though. If you go this route, please isolate the changes to within ORE and invalidate this in emitOptimizationRemark.

> 

> 

> > Even in the new PM, this can be avoided only by incrementally updating the BFI of the caller after inlining a callee into it.

> 

> 

> Sure but I thought that was required for PGO-based inlining anyway.  I was hoping to piggyback on that.


Yes, that's true. I didn't mean to say that ORE has to specifically do this.


================
Comment at: test/Transforms/Inline/optimization-remarks-with-hotness.ll:1
@@ +1,2 @@
+; RUN: opt < %s -inline -pass-remarks=inline -pass-remarks-missed=inline -pass-remarks-analysis=inline -pass-remarks-with-hotness -S 2>&1 | FileCheck %s
+
----------------
The test case could be vastly simplified. The contents of the functions do not affect the test case.


https://reviews.llvm.org/D22694