[PATCH] D82213: [Remarks] Add callsite locations to inline remarks

2020-06-24 Thread Wenlei He via Phabricator via cfe-commits
wenlei added a comment. In D82213#2110941 , @fhahn wrote: > That's interesting. We are also using something similar for the matrix > lowering remarks [1]: we traverse the inlining chain bottom up and emit a > remark at each step which contains the

[PATCH] D82213: [Remarks] Add callsite locations to inline remarks

2020-06-24 Thread Florian Hahn via Phabricator via cfe-commits
fhahn added a comment. That's interesting. We are also using something similar for the matrix lowering remarks [1]: we traverse the inlining chain bottom up and emit a remark at each step which contains the expression available at that level. I think those approaches could be useful in general

[PATCH] D82213: [Remarks] Add callsite locations to inline remarks

2020-06-21 Thread Wenlei He via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG7c8a6936bf6b: [Remarks] Add callsite locations to inline remarks (authored by wenlei). Changed prior to commit: https://reviews.llvm.org/D82213?vs=272263=272289#toc Repository: rG LLVM Github

[PATCH] D82213: [Remarks] Add callsite locations to inline remarks

2020-06-20 Thread David Li via Phabricator via cfe-commits
davidxl accepted this revision. davidxl added a comment. This revision is now accepted and ready to land. lgtm Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:383 + const Function , const InlineCost , + bool

[PATCH] D82213: [Remarks] Add callsite locations to inline remarks

2020-06-20 Thread Wenlei He via Phabricator via cfe-commits
wenlei marked an inline comment as done. wenlei added inline comments. Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:392 +if (ProfileGuidedInline) + Remark << " by profile guided inliner"; +Remark << " with " << IC; davidxl wrote: > Perhaps reword

[PATCH] D82213: [Remarks] Add callsite locations to inline remarks

2020-06-20 Thread Wenlei He via Phabricator via cfe-commits
wenlei updated this revision to Diff 272263. wenlei added a comment. Update remark message. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82213/new/ https://reviews.llvm.org/D82213 Files:

[PATCH] D82213: [Remarks] Add callsite locations to inline remarks

2020-06-20 Thread David Li via Phabricator via cfe-commits
davidxl added inline comments. Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:392 +if (ProfileGuidedInline) + Remark << " by profile guided inliner"; +Remark << " with " << IC; Perhaps reword it to " to match profiling context" .. Repository:

[PATCH] D82213: [Remarks] Add callsite locations to inline remarks

2020-06-20 Thread Wenlei He via Phabricator via cfe-commits
wenlei updated this revision to Diff 272240. wenlei added a comment. Address David's comments, add test for nested inlinining. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82213/new/ https://reviews.llvm.org/D82213 Files:

[PATCH] D82213: [Remarks] Add callsite locations to inline remarks

2020-06-19 Thread Wenlei He via Phabricator via cfe-commits
wenlei marked an inline comment as done. wenlei added inline comments. Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:391 +Remark << ore::NV("Caller", ); +if (ProfileGuidedInline) + Remark << " by profile guided inliner"; davidxl wrote: > is this

[PATCH] D82213: [Remarks] Add callsite locations to inline remarks

2020-06-19 Thread David Li via Phabricator via cfe-commits
davidxl added a comment. Can you add a test case where there is more than one level of inline contexts for the callsite? Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:391 +Remark << ore::NV("Caller", ); +if (ProfileGuidedInline) + Remark << " by profile guided

[PATCH] D82213: [Remarks] Add callsite locations to inline remarks

2020-06-19 Thread Francis Visoiu Mistrih via Phabricator via cfe-commits
thegameg added subscribers: fhahn, anemet, thegameg. thegameg added a comment. This sounds useful indeed. @fhahn, @anemet might want to take a look. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82213/new/ https://reviews.llvm.org/D82213

[PATCH] D82213: [Remarks] Add callsite locations to inline remarks

2020-06-19 Thread Wenlei He via Phabricator via cfe-commits
wenlei created this revision. wenlei added reviewers: wmi, davidxl, hoy. Herald added subscribers: llvm-commits, cfe-commits, hiraditya. Herald added projects: clang, LLVM. Add call site location info into inline remarks so we can differentiate inline sites. This can be useful for inliner