[PATCH] D82270: Added a Printer to the FunctionPropertiesAnalysis

[Mircea Trofin via Phabricator via llvm-commits]

mtrofin added a comment.

In D82270#2111746 <https://reviews.llvm.org/D82270#2111746>, @jdoerfert wrote:

> In D82270#2110007 <https://reviews.llvm.org/D82270#2110007>, @mtrofin wrote:
>
> > In D82270#2109923 <https://reviews.llvm.org/D82270#2109923>, @jdoerfert wrote:
> >
> > > In D82270#2106860 <https://reviews.llvm.org/D82270#2106860>, @mtrofin wrote:
> > >
> > > > What's the value of having the result type have the analyze method? Is there a scenario where we want to get the result and then re-analyze it?
> > >
> > >
> > > In the new-PM if you preserve the analysis it doesn't need to be recomputed, I thought. Or maybe I misunderstand your question. Could you elaborate?
> > >
> > > [EDIT: Do you mean the `...Info` struct? It is used by two passes, right?]
> >
> >
> > What I mean was, why not let the computation happen in the Analysis::run, and the Info is basically an object not meant to be mutated. This is because the PM will call ::run to recompute, and we don't really have (or want to have) a scenario where we want someone to call Info::analyze() (unless I'm missing something)
>
>
> Oh, I see. This is a common pattern, I think. Mostly because it allows the passes to be minimal wrappers. This is good as we might want to have an old-PM pass as well. Then you want the analyze in the info "for sure". Even without an old-PM pass, this way allows old-PM cgscc passes to create the info object and populate it, something that doesn't work in the old-PM through the pass mechanism.
>
> Is there a strong reason or is it more a design preference?


It's a design preference, under the belief that clarity in design helps readability and maintainability.

To unblock this patch, perhaps we could separate that refactoring from the printer part of the patch?

Sounds like we need the Info's self-refreshing ability to support old-PM scenarios, correct? If I understand it correctly, there, the Info object could be retrieved from the analysis (this'd be an old-PM analysis, to be added), and then the client could refresh it without the involvement of the analysis or analysis manager, but updating the state the Analysis manager. Is that even desirable?


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

https://reviews.llvm.org/D82270