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

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:1201
+      Pair.first->second = Data.Count;
+    else if (Total > Data.Count)
+      Total -= Data.Count;
----------------
wmi wrote:
> wmi wrote:
> > wenlei wrote:
> > > In what case do we have Total < Data.Count? And Total == Data.Count should be fine for subtraction too?
> > Currently we won't have such case. It is just some extra protect in case of future change. I think assertion will be better. Will change it.
> > 
> > Right, Total == Data.Count should be added. will change it.
> The assertion catches a case where we have Total < Data.Count when running the test test/Transforms/SampleProfile/csspgo-inline-icall.ll.
> 
> For CS profile, the sum computed in findIndirectCallFunctionSamples can be smaller than the sum of the target counts returned by findCallTargetMapAt. Is it expected?
For CS profile, we use callsite count (including both inlined and non-inlined targets) instead of using entry counts, since there's yet a way to tell if an indirect call target is inlined or not from a CS profile. 

We have a fix for this to review. Haven't sent it out yet. It basically replaces this code

```
         uint64_t Sum = 0;
          findIndirectCallFunctionSamples(I, Sum);
```

with
          
```
if (FunctionSamples::ProfileIsCS) {
            // With CSSPGO all indirect call targets are counted towards the
            // original indirect call site in the profile, including both
            // inlined and non-inlined targets.
            Sum = 0;
            for (const auto &T_C : T.get())
              Sum += T_C.second;
          } else {
            findIndirectCallFunctionSamples(I, Sum);
          }
```


 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96806