[llvm-commits] Improvements to PassManager

[Kostya Serebryany]

On Mon, Nov 19, 2012 at 4:08 PM, Chandler Carruth <chandlerc at google.com>wrote:

> FWIW, similar to Evan's comments, I would be interested to see what
> variation of the patch would satisfy the immediate needs of ASan,
> mostly because I don't want to block this on the fixing the much
> deeper issues w/ the pass manager.
>

I can move asan back to ModulePass if that helps.
It was a ModulePass until recently, I changed it to FunctionPass because it
simplified using ScalarEvolution, which is a FunctionPass too.
Apparently, this chage was wrong, although I still not see why :(
(We still did not apply the patch with ScalarEvolution, so moving asan back
to FunctionPass should not break anything)

--kcc



>
> However, having looked at the *specific* case of ASan, I think it is
> likely playing fast and loose with the finalization step, and that
> step should be moved to a module pass. Are the issues you're seeing
> strictly a consequence of the finalization step? If so, I can work on
> splitting it into a module pass to help unblock you.
>
>
> Everything below is trying to get a better understanding of the
> underlying issues, and so may not be relevant in the short-term...
>
> On Sun, Nov 18, 2012 at 3:37 PM, Nick Lewycky <nicholas at mxc.ca> wrote:
> > Pedro Artigas wrote:
> >>
> >> Hello evan,
> >>
> >> I believe that there should be no ordering assumption between any
> >> doFinalization/doInitialization calls, that is what got us in trouble
> in the
> >> first place. The current assumption is that doFinalization of
> >> AddressSanitizer is run before runOnModule of the printer which is,
> AFAIK,
> >> an unreasonable assumption and a current implementation detail.
> >
> >
> > Hi Pedro, I think I'm responsible for the way that ASAN uses
> > doInitialization/doFinalization, by suggesting it in review when the pass
> > was added.
> >
> > My understanding is that function passes are expected to do things to the
> > module in their init/finalize functions that they can't do in
> runOnFunction
> > -- in this case, creating and then cleaning up the function declarations.
> > Trying to do that in runOnFunction would break parallel runs of function
> > passes.
>
> My perspective on the init/finalize functions is somewhat different:
> 0) They should have a common interface across all passes: function,
> module, and BB. I think the current differences should be fixed.
> 1) They should get a handle to the module being processed.
> 2) They should be called exactly once per module, no matter how many
> times the pass appears in the optimization pipeline.
> 3) The init variant should be called before any pass's 'runOnFoo'
> method is called, and the finalize after every runOnFoo.
>
> The goal here is to allow once-per-module setup and teardown of common
> infrastructure used in a pass. It is *not* for the purpose of doing
> transformations of entities within the pass, however merely adding
> declarations of functions (or even adding entirely new functions)
> seems an excellent use of the init.
>
> I think ASan's finalize is actually misbehaving here, as it is really
> doing a transformation that should be its own module pass, run after
> globalopt and friends.
>
> > I didn't think of what would happen if you put the asm printer in the
> same
> > passmanager. You're right that it doesn't work, but it's not immediately
> > obvious to me that this is asan's fault, or the user's fault for putting
> a
> > per-function asm printer inside the same pass manager.
>
> Forgive my ignorance, but what problem are you envisioning here, and
> how can I observe it? I'd like to understand it fully to see in what
> ways my thoughts above would or wouldn't interact with it, and I'm
> just not sufficiently familiar w/ the asm printer infrastructure...
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121119/aa662da7/attachment.html>