[llvm-commits] Improvements to PassManager

[Kostya Serebryany]

On Tue, Nov 20, 2012 at 4:17 PM, Chandler Carruth <chandlerc at google.com>wrote:

> On Tue, Nov 20, 2012 at 4:12 AM, Kostya Serebryany <kcc at google.com> wrote:
> >
> >
> > On Tue, Nov 20, 2012 at 3:05 PM, Chandler Carruth <chandlerc at google.com>
> > wrote:
> >>
> >> On Mon, Nov 19, 2012 at 4:46 AM, Kostya Serebryany <kcc at google.com>
> wrote:
> >> >
> >> >
> >> > On Mon, Nov 19, 2012 at 4:33 PM, Chandler Carruth <
> chandlerc at google.com>
> >> > wrote:
> >> >>
> >> >> FYI, it shouldn't be moved back to a module pass. I'm just suggesting
> >> >> the
> >> >> logic in doFinalization should be lifted into a module pass.
> >> >
> >> > You mean split the AddressSanitizer class into two? This will cause
> some
> >> > pain.
> >> > There are things computed at doInitialization or at runOnFunction, and
> >> > then
> >> > used in doFinalization.
> >> > E.g. DynamicallyInitializedGlobals will need to be computed twice.
> >> > GlobalsCreatedByAsan will have to be replaced with some other way to
> >> > mark
> >> > globals created by asan (use metadata nodes? match name strings?)
> >>
> >> This is what I meant... I realize it's not trivial, but I think it's
> >> the correct thing to do.
> >>
> >> Essentially, transforms on globals themselves should be done as a
> >> module pass (much like globalopt is today a module pass), and
> >> transforms on the functions as a function pass.The global transforms
> >> are also much simpler / more constrained, and so it seems less likely
> >> for them to benefit from being a function pass.
> >
> >
> > Makes sense. (The downside, of course, is increased code complexity.
> > Hopefully not too much).
> >
> >
> >>
> >>
> >>
> >> To address your specific points:
> >>
> >> DynamicallyInitializedGlobals appears to only be created inside the
> >> finalization step, even though it is referenced from outside of it...
> >> I've no idea how the outside references can work currently.
> >
> >
> > hah! This is a performance bug in asan-initialization-order mode!
> > (The global transform phase was done in doInitialization until recently).
> > I'll fix this with a unit test this time ;)
> >
> >
> >>
> >> That said,
> >> computing this twice (once in the initialization step of the function
> >> pass, once in the module pass) doesn't seem terribly costly.
> >
> >
> > Not too bad, indeed.
> >
> >>
> >>
> >> GlobalsCreatedByAsan should indeed be handled in some other way. I'm
> >> hopeful that the new attribute system will be generalized to support
> >> globals as well as functions, which would allow us to just not mark
> >> the ASan globals as instrumented.
> >
> >
> > But that won't happen for at lest a couple of months, right?
>
> The attribute stuff is going in presently, but still may not want to wait.
>
> >
> >>
> >> I'd also be fine using a particular
> >> naming convention, there are already some naming convention stuff done
> >> in ASan, so it wouldn't be too weird.
> >
> >
> > I will probably have to use names for now (e.g. __asan_gen_1234?)
>
> FWIW, this seems fine...
>
> >
> >>
> >>
> >> Are there any other bits that would be made particularly awkward by this
> >> split?
> >
> >
> > Nothing awkward, just quite a few duplicated lines.
> >
> >>
> >>
> >>
> >> Is this something you folks have bandwidth for? I can look at it if not.
> >
> >
> > Depends on what is the ETA. I'll do it, but don't promise to finish it
> > immediately.
> >
> > This all is a bit sad.
> > Mainly because the behavior of doInitialization is a) not what I
> expected it
> > to be and b) not at all understood (by me).
>
> One point -- doFinalization is the real problem here. I think ASan's
> doInitialization is fine as-is.
>

I meant doFinalization.


>
> So what would you want the behavior of doFinalization to be? That is,
> what are you looking for in these routines? Just a way to share common
> state between a FunctionPass and a ModulePass?
>

First of all, I want to understand how it works now :)
I expected that it is
  a) doInitialization
  b) for each f: runOnFunction(f)
  c) doFinalization
Apparently, there is something else in between (other functions passes are
added to the mix).


--kcc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121120/deb01e36/attachment.html>