[llvm-commits] Improvements to PassManager

[Kostya Serebryany]

On Mon, Nov 26, 2012 at 9:39 PM, Pedro Artigas <partigas at apple.com> wrote:

>
> 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)
>

I've implemented part of the fix in asan, hope to finish it this week.

--kcc


> - 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/debf3544/attachment.html>