[PATCH] D28331: Improve PGO support for the new inliner

[David Li via Phabricator via llvm-commits]

davidxl added inline comments.


================
Comment at: test/Transforms/Inline/function-count-update-2.ll:12
+define i32 @caller1() !prof !2 {
+  %i = call i32 @callee()
+  ret i32 %i
----------------
Check no calls remaining.


================
Comment at: test/Transforms/Inline/function-count-update-2.ll:17
+define i32 @caller2() !prof !3 {
+  %i = call i32 @callee()
+  ret i32 %i
----------------
same here


================
Comment at: test/Transforms/Inline/function-count-update-3.ll:13
+; known and hence the cost gets reduced.
+; Estimated count of a->e callsite = C2 * (C1 / C3)
+; Estimated count of a->e callsite = 250 * (200 / 500) = 100
----------------
Should it be C2 * (C1/C4) where C4 is the entry count of function C? which happens to be the same to C3 here. C3 is irrelevant


================
Comment at: test/Transforms/Inline/function-count-update-3.ll:31
+
+define i32 @c(i32 %c1, i32 %c100) !prof !3 {
+  call void @ext()
----------------
Also check remaining count of C


================
Comment at: test/Transforms/Inline/inline-cold-callee.ll:31
 ; CHECK: ret i32 %x3.i
   %y2 = call i32 @callee2(i32 %y1)
   %y3 = call i32 @callee1(i32 %y2)
----------------
Wrap callee2 call in conditional -- otherwise its entry count should be  at least 50.


================
Comment at: test/Transforms/Inline/inline-cold-callee.ll:32
   %y2 = call i32 @callee2(i32 %y1)
   %y3 = call i32 @callee1(i32 %y2)
   ret i32 %y3
----------------
This test case is flawed.  callee1's entry count should be a least 50 given new change.


================
Comment at: test/Transforms/Inline/inline-cold-callsite.ll:15
+
+define i32 @callee2(i32 %x) !prof !21 {
+; CHECK-LABEL: @callee2(
----------------
Is there a need to introduce callee2? Just calling callee1 twice should also illustrate the point.


================
Comment at: test/Transforms/Inline/inline-hot-callsite-2.ll:17
+
+define i32 @callee2(i32 %x) !prof !21 {
+; CHECK-LABEL: @callee2(
----------------
just reuse callee1?


https://reviews.llvm.org/D28331