[PATCH] D60086: [SampleProfile] Check entry count instead of total count to decide if inlined callsite is hot.

[Wei Mi via Phabricator via llvm-commits]

wmi added a comment.

> I can understand the value of context-sensitive profile information, but wouldn't it be only valuable if the callsite is actually worth inlining?

I think an example may be helpful to illustrate why inlining for better profile annotation is important even if the inlining doesn't remove call overhead because of entry being cold.

  foo() {
    for (i = 0; i < N; i++) {
      if (condition = getcond()) {
  
      } else {
  
      }
    }
  }
  
  goo {
    ...
    call foo  // condition is true in 99% iterations of loop in foo.
    ...
  }
  
  hoo {
    ...
    call foo  // condition is false in 99% iterations of loop in foo.
    ...
  }

Suppose foo is only called twice, once in goo and once in hoo, so is very cold at the function entry. But the loop inside of foo is very hot (N is very large). Suppose in non-FDO build, both callsites of foo are inlined although they are cold because size cost is lower than threshold (we can reduce size by inlining foo). So we will have context sensitive profiles for foo in the profile generated from non-FDO binary.

If we inline the two callsites in sample profile loader, after profile annotation, we know the branch probablity of the if statement inlined into goo will be 99% vs 1% while branch probablity in hoo will be 1% vs 99%.

If we don't inline the above two callsites in sample profile loader, it could be the case either we don't get any profile for foo because foo are all inlined in non-FDO binary, or we get the profile but the profile is not context sensitive so the branch probability of if statement in foo will be regarded as 50% vs 50%.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60086