[PATCH] D97350: [SampleFDO] Another fix to prevent repeated indirect call promotion in sample loader pass

[Wei Mi via Phabricator via llvm-commits]

wmi created this revision.
wmi added reviewers: davidxl, wenlei, hoy.
Herald added a subscriber: hiraditya.
wmi requested review of this revision.
Herald added a project: LLVM.

In https://reviews.llvm.org/rG5fb65c02ca5e91e7e1a00e0efdb8edc899f3e4b9, to prevent repeated indirect call promotion for the same indirect call and the same target, we use zero-count value profile to indicate an indirect call has been promoted for a certain target. We removed PromotedInsns cache in the same patch. However, there was a problem in that patch described below, and that problem led me to add PromotedInsns back as a mitigation in https://reviews.llvm.org/rG4ffad1fb489f691825d6c7d78e1626de142f26cf.

When we get value profile from metadata by calling getValueProfDataFromInst, we need to specify the maximum possible number of values we expect to read. We uses MaxNumPromotions in the last patch so the maximum number of value information extracted from metadata is MaxNumPromotions. If we have many values including zero-count values when we write the metadata, some of them will be dropped when we read them because we only read MaxNumPromotions values. It will allow repeated indirect call promotion again. We need to make sure if there are zero-count values, those values need to be saved into or extracted from metadata with higher priority than other non-zero count values.

The patch fixed that problem. When we prepare to update the metadata in updateIDTMetaData, we will sort the values first and extract only MaxNumPromotions values to write into metadata. We will select values in such order: first try to extract zero-count values, then extract the values with the highest profile count, until we have MaxNumPromotions values to write. In this way, if we have equal to or less than MaxNumPromotions of zero-count values, they will all be kept in metadata and will be read without problem. If we have more than MaxNumPromotions of zero-count values, we will only save MaxNumPromotions zero-count values maximally. In such case, we have logic in place in doesHistoryAllowICP to guarantee no more indirect call promotion in sample loader pass will happen for the indirect call, because it has been promoted enough.

With this change, now we can remove PromotedInsns without problem.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97350

Files:
  llvm/lib/Analysis/IndirectCallPromotionAnalysis.cpp
  llvm/lib/Transforms/IPO/SampleProfile.cpp
  llvm/test/Transforms/SampleProfile/Inputs/norepeated-icp-2.prof
  llvm/test/Transforms/SampleProfile/norepeated-icp-2.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D97350.325942.patch
Type: text/x-patch
Size: 18689 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210224/f6aa18e0/attachment.bin>