[PATCH] D96806: [SampleFDO] Stop repeated indirect call promotion for the same target

[Wei Mi via Phabricator via llvm-commits]

wmi added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:1072
+      // means the candidate has been promoted for this indirect call.
+      if (ValueData[I].Value == Function::getGUID(Candidate) &&
+          ValueData[I].Count == 0)
----------------
davidxl wrote:
> Why do we need to keep the zero count data in the first place?
I added the zero count data in updateIDTMetaData to indicate that a target has been promoted so it won't be promoted again. Once an indirect call is promoted for a target, we hope it won't be promoted for the same target again in any case, no matter where it is inlined into, so I think metadata is the best place to save the information. Currently I use zero count to represent that a target cannot be promoted anymore, this is consistent with how regular ICP handles zero count profile. 




================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:1108
+    if (Pair.first->second != 0)
+      Pair.first->second = Data.Count;
+  }
----------------
davidxl wrote:
> Why updating here? Is the meta data more up to date?
Yes, SampleFDO is different from PGO. PGO initialize the value profiling data once and only substract count of a target when the target is promoted. For SampleFDO, it needs to update metadata every time the indirect call is reannotated with more precise profile after inlining. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96806