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

[David Li via Phabricator via llvm-commits]

davidxl added inline comments.


================
Comment at: llvm/lib/ProfileData/InstrProf.cpp:1012
   // Get total count
   ConstantInt *TotalCInt = mdconst::dyn_extract<ConstantInt>(MD->getOperand(2));
+  if (!TotalCInt)
----------------
Does total count include the magic num? It should not.


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:712
+  unsigned NumPromoted = 0;
   if (Valid) {
     for (uint32_t I = 0; I < NumVals; I++) {
----------------
early return if not valid


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:732
                   const SmallVectorImpl<InstrProfValueData> &CallTargets,
-                  uint64_t Total = 0) {
+                  uint64_t Sum = 0) {
   DenseMap<uint64_t, uint64_t> ValueCountMap;
----------------
document 'Sum'  What is the difference to TotalCount?


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:759
+      TotalCount -= Pair.first->second;
+      Pair.first->second = Data.Count;
+    } else if (Pair.first->second == NOMORE_ICP_MAGICNUM &&
----------------
`=  NOMORE_ICP_MAGICNUM`


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:762
+               Data.Count != NOMORE_ICP_MAGICNUM) {
+      Sum -= Data.Count;
+    } else if (Pair.first->second != NOMORE_ICP_MAGICNUM &&
----------------
can it become negative?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97350