[llvm-commits] Improvements to PassManager

[Pedro Artigas]

Hello All,


To summarize the thread and make sure everybody is on the same page I will repeat some information below.

- doInitialization and doFinalization are expected to be called before/after runOn???? for each pass on a given module, if more than one module exist in the compilation it will be called multiple times. No other assumption exists. (that is, for example, doFinalization may be called before or after runOn??? of other, unrelated, passes)

- ASAN violates this model and it is currently being fixed to split the part that does transformations on the module in doFinalization into a separate module pass (where the code to transform will be called in the run function on the module and, therefore, has proper ordering with other passes)

- the short term plan is to workaround the limitation in ASAN by imposing a more restrictive ordering in the doFinalization for function passes (must be run after runOnFunction on each function) so that ASAN works for now, but this limitation should be lifted in the future.

If the above is the conclusion I am happy to implement item #3 and defer the fixing of ASAN until time permits. It will unblock me.

Thanks everyone,

Pedro Artigas


On Nov 21, 2012, at 7:22 PM, Kostya Serebryany <kcc at google.com> wrote:

> 
> 
> On Tue, Nov 20, 2012 at 4:12 PM, 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? 
>  
> 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?)
>  
> 
> 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. 
> 
> 
> Some progress: I've ripped of the creation of global redzones into a separate class.
> Now I'll need to actually create a separate ModulePass.
> 
> --kcc 
>  
> 
> 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). 
> 
> --kcc 
> 
> _______________________________________________
> 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/20121126/75cc5bba/attachment.html>