llvm dev - Jan 2019 - Advice for Porting SafeStack to New Pass Manager

Hello,

I'm in the process of creating a pass for the new PM for SafeStack
which is only available as a part of the legacy PM. The only thing
bugging me is in regards to the TargetPassConfig analysis. Whereas
most other passes/analyses I have seen separate the logic between the
actual pass and anything it does to the IRUnits it runs over are in 2
separate classes, TargetPassConfig has them both in the same class and
many other analysis inherit from this class as well. I also do not
think this analysis has a new PM equivalent.

My question is what would be the correct way to proceed with porting
TargetPassConfig to the new PM? Normally, what I have done before and
what I have seen from other examples is to separate the logic of what
the pass does to an IRUnit and the pass itself into 2 separate
classes, then an instance of the class that contains the logic can be
owned by a pass for the new PM. The only trouble I see with this is
that this will require a lot of refactoring (that I started but
haven't finished), since TargetPassConfig is the parent to many other
passes/analyses and these classes override many virtual methods in
TargetPassConfig, so it may not be the cleanest solution and
potentially prone to a lot of error.

Another potential idea which I am exploring in parallel to the first
is to instead make the existing TargetPassConfig also a pass that
works with the new PM by adding a run() method. This seems like less
work, but feels incorrect since I'm shoving more things into this
class.

Any opinions on this?

Thanks,
Leonard Chan

On 12/28/18 10:09 PM, Leonard Chan via llvm-dev wrote:
> Hello,
>
> I'm in the process of creating a pass for the new PM for SafeStack
> which is only available as a part of the legacy PM. The only thing
> bugging me is in regards to the TargetPassConfig analysis. Whereas
> most other passes/analyses I have seen separate the logic between the
> actual pass and anything it does to the IRUnits it runs over are in 2
> separate classes, TargetPassConfig has them both in the same class and
> many other analysis inherit from this class as well. I also do not
> think this analysis has a new PM equivalent.
My knowledge on Codegen is close to being non-existent, but as far as I see
there is no New-PM equivalent for CodeGen passes/analyses at all.

And as Chandler and others told me, NewPM CodeGen work not yet done 
includes
fleshing out a "new-style" IRUnit support for MachineIR (Pass/Analysis
Managers for those).
You can surely convert some Codegen FunctionPass'es into NewPM,
but you wont be able to create a fully operational CodeGen pipeline w/o 
a MachineIR.

(this is not to discourage your effort on doing the port though :) ...
> My question is what would be the correct way to proceed with porting
> TargetPassConfig to the new PM? Normally, what I have done before and
> what I have seen from other examples is to separate the logic of what
> the pass does to an IRUnit and the pass itself into 2 separate
> classes, then an instance of the class that contains the logic can be
> owned by a pass for the new PM. The only trouble I see with this is
> that this will require a lot of refactoring (that I started but
> haven't finished), since TargetPassConfig is the parent to many other
> passes/analyses and these classes override many virtual methods in
> TargetPassConfig, so it may not be the cleanest solution and
> potentially prone to a lot of error.
When possible separation of IR analysis/transformation logic from the 
pass-manager-specific
pass/analysis housekeeping provides a cleanest way to share code between 
legacy and newpm
while avoiding any ill dependencies.
In some cases this was done by NewPM pass instance reusing the 
LegacyPass class
as an instance that does the actual transformation.

Yet, as I  scan through TargetPassConfig, it looks more like a 
PassBuilder/PassManager rather
than an analysis.
> Another potential idea which I am exploring in parallel to the first
> is to instead make the existing TargetPassConfig also a pass that
> works with the new PM by adding a run() method. This seems like less
> work, but feels incorrect since I'm shoving more things into this
> class.
Lets first decide if it should be a pass/analysis at all :).
And if we ever decide that way then adding ::run to it would be the 
least comfortable solution to me.

best regards,
   Fedor.
>
> Any opinions on this?
>
> Thanks,
> Leonard Chan
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

> > My question is what would be the correct way to proceed with porting
> > TargetPassConfig to the new PM? Normally, what I have done before and
> > what I have seen from other examples is to separate the logic of what
> > the pass does to an IRUnit and the pass itself into 2 separate
> > classes, then an instance of the class that contains the logic can be
> > owned by a pass for the new PM. The only trouble I see with this is
> > that this will require a lot of refactoring (that I started but
> > haven't finished), since TargetPassConfig is the parent to many
other
> > passes/analyses and these classes override many virtual methods in
> > TargetPassConfig, so it may not be the cleanest solution and
> > potentially prone to a lot of error.
> When possible separation of IR analysis/transformation logic from the
> pass-manager-specific
> pass/analysis housekeeping provides a cleanest way to share code between
> legacy and newpm
> while avoiding any ill dependencies.
> In some cases this was done by NewPM pass instance reusing the
> LegacyPass class
> as an instance that does the actual transformation.
>
> Yet, as I  scan through TargetPassConfig, it looks more like a
> PassBuilder/PassManager rather
> than an analysis.
Do you know of other analyses that also act like pass
builders/managers that were ported to new PM?

Thanks,
Leonard