[PATCH] D32773: Update VP prof metadata during inlining.

Teresa Johnson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu May 4 13:22:41 PDT 2017


tejohnson added a comment.

In https://reviews.llvm.org/D32773#746315, @tejohnson wrote:

> In https://reviews.llvm.org/D32773#745551, @tejohnson wrote:
>
> > I found that not all calls with VP metadata were being scaled after this patch. Specifically, this was VP metadata on a memcpy intrinsic (so profiling the memcpy size values not an indirect call), but it still makes sense to scale them the same way. I confirmed that the call was not in the value map iterated in the caller of this function (updateCallProfile), which is what is iterated to update the inlined instructions in the caller. Are we guaranteed that every inlined instruction would be in there?
>
>
> This may be a false alarm. It looks like we might simply not bed doing the update, which would happen if the entry count of the callee was 0. My guess is that this happened because I was only redoing the ThinLTO backend with the patch, and we probably needed to do scaling in the compile step to avoid importing and inlining a callee with 0 entry count and unscaled VP metadata...confirming with a full rebuild, which will take some time.


Confirmed that this was a false alarm. In the source file I was looking at the expected functions are marked cold. So LGTM, but please wait for Easwaran to see if he has any more concerns.


https://reviews.llvm.org/D32773





More information about the llvm-commits mailing list