[llvm-dev] possible inconsistency in PassManagerInternal.h

[Fedor Sergeev via llvm-dev]

On 09/03/2018 03:51 AM, De Azevedo Piovezan, Felipe wrote:
>
> Thanks for the reply, Fedor!
>
> >You dont need to operate with classes defined in PassManagerInternal.h
> >They are, well, internal.
> >Unless you want to hack into the details of PassManager implementation
>
> Apologies, this was a bit of miscommunication on my part: I am indeed 
> trying to “open the box”  of the new PM, not merely use it; it’s a 
> good opportunity to learn about the techniques employed there.
>
> In order to do so, I was trying to understand some of the decisions 
> made in that header, and two of them stood out as odd.
>
Ah, okey.
The main trick, as you might have got already, is using a tiny 
Concept/Model wrapper to provide virtual dispatch to an arbitrary class 
implementing pass/analysis.
>
> The first one is a case where we are using override on a method but 
> allowing a function signature mismatch (PreservedAnalysisT vs 
> PreservedAnalysis).
>
Yes, this one does look like an inconsistency.
No harm done as there is no other PreservedAnalyses.
And on mismatch it will raise an *error*, not a warning due to 
'override' keyword.
>
> The second is the use AnalysisManagerT vs AnalysisManager in a 
> seemingly arbitrary way.
>
And this one is not an issue.
PassConcept/PassModel is an interface for the PassManager template, and 
part of that interface involves operating on a particular
AnalysisManagerT class, passed as a parameter to 'run' method.

At the same time, AnalysisConcept/Analysis is an interface for the 
AnalysisManager template, and part of AnalysisModel implementation
involves dealing with a very specific specialization of 
AnalysisManager<IRUnitT, ExtraArgTs...>, connecting this AnalysisModel 
and this AnalysisManager
specialization through this set of template parameters.
>
> > Again, it is not an inconsistency, just an implementation detail
>
> That’s what I have, so far, failed to see; I will keep exploring 
> though – the PassManager.h file is still being digested.
>
Feel free to ask questions if you happen to get more.

regards,
   Fedor.

> *From:*Fedor Sergeev [mailto:fedor.sergeev at azul.com]
> *Sent:* Sunday, September 2, 2018 7:07 PM
> *To:* De Azevedo Piovezan, Felipe 
> <felipe.de.azevedo.piovezan at intel.com>; llvm-dev at lists.llvm.org
> *Subject:* Re: [llvm-dev] possible inconsistency in PassManagerInternal.h
>
> You dont need to operate with classes defined in PassManagerInternal.h
> They are, well, internal.
>
> Unless you want to hack into the details of PassManager implementation 
> all you need
> is to write a pass/analysis - which means implementing corresponding 
> ::run() method - and register
> it with the PassBuilder one way or another.
>
> There are plenty of real examples of how you can write a pass in llvm 
> source tree.
> Also there was a recent blog post mentioned on this list with an 
> introduction to new pass manager.
>
> On 09/02/2018 11:22 PM, De Azevedo Piovezan, Felipe via llvm-dev wrote:
>
>     Hello,
>
>     (fairly new around here, apologies if the question has an obvious
>     answer)
>
>     I’ve been studying the structure of the new PassManager in the
>     hopes of using it soon. After watching a couple of talks (Chandler
>     / Sergeev), I decided to start with the Concept/Model classes in
>     PassManagerInternal.h
>
>     While I could make sense of almost everything there, two details
>     caught my attention:
>
>     1-  The PassConcept abstract class has a function `run = 0;` which
>     returns an object of type `PreservedAnalysis`. Such object is
>     forward-declared in this header, but it is defined in
>     PassManager.h  This is the only place in the file where we
>     reference this class.
>
>     However, right after, we define the PassModel class which derives
>     from PassConcept, and one of its template parameters is a typename
>     `PreservedAnalysisT`. The implementation of the `run` method uses
>     the keyword override and it returns an object of type
>     `PreservedAnalysisT`.
>
>     This should raise a warning if PreservedAnalysisT !=
>     PreservedAnalysis.   Is this intentional? My naive intuition is
>     that we should have removed PreservedAnalysisT from the template
>     and used PreservedAnalysis. However, everywhere else in the file
>     PreservedAnalysisT is used as template parameter.
>
>     Am I missing something obvious?
>
> PassModel is a generic template helper used deep inside PassManager 
> template.
> You should neither be modifying anything in it nor using it directly.
>
> And yes - PreservedAnalyses is the type that your Pass' run() method 
> should return.
>
>
>     2- A similar issue arises when using AnalysisManager.
>
>     Both PassConcept  and PassModel have a template typename
>     `AnalysisManagerT`.
>
>     However, AnalysisPassConcept and AnalysisPassModel have no such
>     template parameter; instead, they use the forward-declared
>     AnalysisManager type, defined in PassManager.h
>
>     Again, is there a particular reason for this apparent inconsistency?
>
> Again, it is not an inconsistency, just an implementation detail.
> Ignore it and jump straight to using various AnalysisManagers defined 
> in PassManager.h/LoopPassManager.h etc.
>
> regards,
>   Fedor.
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180903/16dd3944/attachment-0001.html>