[PATCH] D107878: [SampleFDO] Flow Sensitive Sample FDO (FSAFDO) profile loader

[Rong Xu via llvm-commits]

Yes. Resetting them will not cause correctness problems.

Note that SampleProfileLoader is a module scope object -- we initialize
it in a module pass. LoopInfo, DOM etc are a module level analysis obj.
After inline, we need to invalidate and recompute.

This is different for MIRProfileLoader (which is a function level object).
The loopInfo and Dom etc are specific to this function. Since we do not do
any transformation (except changing the BranchProb). I don't think we need
to invalidate them either.

On Wed, Aug 18, 2021 at 1:36 PM Wenlei He via Phabricator <
reviews at reviews.llvm.org> wrote:

> wenlei accepted this revision.
> wenlei added inline comments.
>
>
> ================
> Comment at: llvm/lib/CodeGen/FlowSensitiveSampleProfile.cpp:293
> +  Function &Func = F.getFunction();
> +  clearFunctionData(false);
> +  Samples = Reader->getSamplesFor(Func);
> ----------------
> xur wrote:
> > wenlei wrote:
> > > xur wrote:
> > > > wenlei wrote:
> > > > > Why do we keep dominator tree and loop info?
> > > > Profile loader does not invalid these infos (this is different in
> SampleLoader, where inline will change it)
> > > This is still all per-function info, so it needs to be cleared before
> we use the profile loader to process the next function, right? or did i
> miss anything?
> > We initialize these data structures per function in setInitVals(). Each
> function will have its own value and no need to clear.
> Ok, but resetting it isn't going to cause problem either, right? Just
> trying to understand if there's a correctness issue that requires not
> resetting these fields. If not, might as well not adding the special case
> and always reset/clear. Seem unnecessary to skip reset, but not a big deal
> though.
>
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D107878/new/
>
> https://reviews.llvm.org/D107878
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210818/f2dd3bfe/attachment.html>