[PATCH] D23885: [ThinLTO] add constArgumentsBitmask to caller summary

[Teresa Johnson via llvm-commits]

tejohnson added inline comments.

================
Comment at: include/llvm/IR/ModuleSummaryIndex.h:244
@@ +243,3 @@
+/// Specific info about the call site, like the info about the arguments.
+struct CallsiteInfo {
+  /// if n_th bit is set, it means that n_th argument is constant.
----------------
Prazek wrote:
> tejohnson wrote:
> > Prazek wrote:
> > > tejohnson wrote:
> > > > Since we are accumulating the info for all callsites, there isn't a reason to keep this in a separate structure from CalleeInfo. I.e. the profile counts are accumulated in CalleeInfo, even though that is essentially also callsite info.  If we decide to keep a separate edge per callsite eventually, the CalleeInfo struct will simply change to a CallsiteInfo struct and we would remove CallsiteCount. So better to have this info in CalleeInfo directly.
> > > oh I see, it doesn't matter what caller calls with constants as long as the info is accumulated only in one module.
> > Sorry I don't understand your comment? Each caller function summary contains a single edge to each unique callee, and the info is accumulated for all calls from that function to a given callee function. I don't understand the "as long as the info is accumulated only in one module"?
> So my I firstly thought that I should make a separate Callsite info because this info would be different depending on the caller in one module, so I would probably want to 
> keep it somewhere else than the callee info. But the Callee info is combined only in one module, so it really doesn't matter if I will decide to import one function only based on the one caller or on combined all callers.
Well the CalleeInfo is only combined within each function within each module. If there are calls from multiple functions within a module, it will not be combined across the different callers.

Or are you talking about functions with weak linkage where there can be a multiple callers with the same name in different modules? In that case, right, we do not combine across modules.


https://reviews.llvm.org/D23885