[PATCH] D94435: [SampleFDO] Add the support to split the function profiles with context into separate sections.

[Wei Mi via Phabricator via llvm-commits]

wmi added a comment.

> I guess there will be a separate patch to turn on this work, i.e, setting CtxSplitLayout?

I will use the resetSecLayout interface to select CtxSplitLayout. That will be done in our own branch of autofdo tool. I think for now most of the cases will still want to use the DefaultLayout.



================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:462
+  /// postlink phase, only profiles with context will be read.
+  bool IsThinLTOPostLink;
 
----------------
hoy wrote:
> I'm wondering if we can just use one field for the phase it's currently are at. We may also want to check it for fullLTO in the future.
Good point. Will change it.


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:2126
+  if (!F.getEntryCount().hasValue())
+    F.setEntryCount(ProfileCount(initialEntryCount, Function::PCT_Real));
   std::unique_ptr<OptimizationRemarkEmitter> OwnedORE;
----------------
hoy wrote:
> `F` may get a chance to update its entry count with a post-inline count during top-down processing with the default layout. Maybe put this under the check against `SkipNoContextProf`?
I think for any case we don't want to reinitiate F's entrycount to initialEntryCount if it has already had a valid value.

Even if SkipNoContextProf is false, in LTO/ThinLTO postlink phase, it is possible that the emitAnnotations function doesn't change anything which could affect profile annotation (i.e., the variable Changed in emitAnnotations is false), so if a function getting a valid entry count in prelink phase is reinitialized to -1 before entering emitAnnotations in postlink,  emitAnnotations may not be able to update it with a valid entry count again.

In addition, if F has a valid entrycount entering emitAnnotations, emitAnnotations can still update it without problem.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D94435