[PATCH] D94001: [CSSPGO] Call site prioritized inlining for sample PGO

[Wenlei He via Phabricator via llvm-commits]

wenlei marked an inline comment as done.
wenlei added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:1535
+        // For indirect call, we don't run CallAnalyzer through InlineCost
+        // before actual inlining to work around PR18962. However, that means we
+        // may do ICP first and later decided not to inline, which is mostly ok
----------------
wmi wrote:
> wenlei wrote:
> > wmi wrote:
> > > According to https://bugs.llvm.org/show_bug.cgi?id=18962, PR18962 has been fixed  in 2014. Is it the right bug to refer to?
> > The bug was fixed, but we generate different types for the same definition for the case in that bug, and call analyzer can't handle that now. Updated the comment to include more details.
> > 
> > For code below, whether fn1 is present affects the type name of A. Then we could see two different types from the same definition, which makes CallAnalyzer choke as it's expecting matching parameter type on both caller and callee side.
> > 
> > ```
> > class A {
> >   // append has to have the same prototype as fn1 to tickle the bug.
> >   void (*append)(A *);
> > };
> > void fn1(A *p1) {
> > }
> > ```
> > 
> I see, thanks for clarifying it. It is better to add a TODO here.
Updated comment with TODO.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94001/new/

https://reviews.llvm.org/D94001