[llvm-dev] Opt Bisect layering
David Blaikie via llvm-dev
llvm-dev at lists.llvm.org
Mon Apr 23 15:42:11 PDT 2018
Ping on this - any chance we can look at fixing the OptBisect layering
here/now?
Could we move the implementation into Analysis & require users to set it,
rather than having it as a default value in IR?
On Tue, Apr 3, 2018 at 9:25 AM David Blaikie <dblaikie at gmail.com> wrote:
> On Tue, Apr 3, 2018 at 8:50 AM Fedor Sergeev <fedor.sergeev at azul.com>
> wrote:
>
>> > Pass Manager (PassManager.h) itself, does not - it's only templates,
>> none of it depends on Region, Loop, etc.
>> Well, true but the problem happens when you try to instantiate the thing.
>> And for generic features like opt-bisection, ir-print-after-all etc that
>> want:
>> - to have a say before/on/after every execution of every pass
>> - have a shared implementation of the main logic
>>
>> you have to do most of the following:
>> 1. instantiate your interfaces for all the IRUnits
>> 2. have pass manager doing the job for you directly
>> 3. extend pass interface with specific helpers for your job
>> (skipFunction)
>>
>> neither of those helps perfect layering...
>> And with new pass manager having no common Pass hierarchy this gets even
>> more clumsy.
>>
>
> Not sure I follow, sorry - when you go to instantiate the pass manager &
> the catch system - at that point there's a concrete set of passes (you have
> a dependence on Analysis and Transforms) and entities (regions, loops, etc)
> so the dependencies seem like they make sense.
>
>
>>
>> > I'm happy to discuss/help design this area as well if you'd like :)
>> Yeah, I'm interested to continue this design discussion, although my
>> interests
>> are primarily in the area of new pass manager currently.
>> I'm going to post a separate RFC on that topic.
>>
>
> Sounds good.
>
> Looping back for this thing - would it be reasonable to remove the default
> OptBisect from IR, move it into Transform or some other leafier dependency?
> Leaving only the basic interface (that can get away with forward
> declarations of Region, Loop, etc) in IR? Is that likely to be done in the
> patch under review, or shortly after it?
>
>
>>
>> regards,
>> Fedor.
>>
>> On 04/03/2018 06:16 PM, David Blaikie wrote:
>> >
>> >
>> > On Mon, Apr 2, 2018 at 11:32 PM Fedor Sergeev via llvm-dev
>> <llvm-dev at lists.llvm.org> wrote:
>> >
>> > On 03/30/2018 12:05 AM, David Blaikie via llvm-dev wrote:
>> > > & now looking back at the patch-in-progress, I see it allows
>> setting
>> > the OptBisector/OptPassGate as suggested in (2).
>> > Well, the patch currently discussed does not attempt to solve the
>> > passgate object management issue.
>> > It is left for the discretion of passgate object provider.
>> >
>> > >
>> > > If that becomes the /only/ option (ie: LLVMContext has no
>> default
>> > OptPassGate) then the virtual interface could be kept down in IR
>> (though
>> > it's still a bit questionable to have those Analysis types (Loop,
>> > Region, CallGraphSCC) even declared in IR). Then the
>> implementation of
>> > OptBisector could be moved into Analysis - freely able to depend
>> on the
>> > concrete Analysis types.
>> >
>> > To me this is a "Pass Manager catch" - entity that attempts to
>> control
>> > all the passes needs to be part of (or tightly cooperate with)
>> pass manager.
>> > Pass manager is currently in IR, and perhaps rightfully so.
>> > Yet passes that it controls work on "IR units" which are either IR
>> or
>> > Analysis, thus Analysis leaks into the interfaces inevitably.
>> > Kinda logical conflict it is...
>> >
>> >
>> > This is in response to my "it's still a bit questionable" comment?
>> That's not too important - I'm not pushing to change that if we can get
>> the mechanical layering functional regardless, by only having forward
>> declarations of those different Analysis entities in llvm/IR, but not
>> need their definitions except in the implementation of this virtual
>> interface which could live in llvm/Analysis.
>> >
>> > But to discuss it anyway: It seems a bit different that the "Pass
>> Manager catch" depends on the concrete types but the Pass Manager
>> (PassManager.h) itself, does not - it's only templates, none of it
>> depends on Region, Loop, etc. If the catch could be implemented
>> similarly to the manager itself, then it'd have the same layering
>> requirements & no problem. But I haven't looked closely enough at the
>> APIs to figure out if/how that might be done - the current
>> implementation/mechanisms are at odds because of the incompatibility of
>> templates and virtual dispatch (can't have a virtual function template -
>> it'd have an unbounded/unknowable number of vtable entries, etc). Some
>> sort of visitor-y thing might be needed/useful, I'm not sure. But again,
>> not sure this is necessary to address/fix for the issues I'm
>> seeing/pushing to deal with - but I'm happy to discuss/help design this
>> area as well if you'd like :)
>> >
>> > - Dave
>> >
>> >
>> >
>> > regards,
>> > Fedor.
>> >
>> > >
>> > > - Dave
>> > >
>> > > On Thu, Mar 29, 2018 at 2:01 PM David Blaikie
>> <dblaikie at gmail.com> wrote:
>> > >
>> > > So... looking at OptBisect, I have a few thoughts:
>> > >
>> > > 1) what's the purpose of the virtual
>> interface/OptPassGate? I'm
>> > guessing maybe that worked around the circular referencing in these
>> > APIs? hmm, no, I suppose that wouldn't work/be relevant here.
>> > >
>> > > 2) Why is OptBisector a ManagedStatic? That seems pretty
>> > antithetical to the role of LLVMContext. When/why would a user be
>> > bisecting over multiple LLVMContexts? & even then, maybe it'd be
>> more
>> > suitable for that grouping (the scope for the bisection) to be API
>> > driven - passing the bisector into the LLVMContext ctor to define
>> the
>> > set of contexts that share a bisector?
>> > >
>> > > On Wed, Mar 21, 2018 at 10:20 PM Yevgeny Rouban via llvm-dev
>> > <llvm-dev at lists.llvm.org> wrote:
>> > >
>> > > Andrew,
>> > >
>> > > I would not make the caller pass the description of the
>> IR
>> > unit. That is because it would result in the description
>> generated every
>> > time even if OptBisect is disabled. Description generation is not
>> very chip.
>> > > Thinking on the OptBisect extension, I believe passing
>> the
>> > units are the right choice because OptPassGates may use them to
>> make
>> > pass skipping decisions.
>> > >
>> > > -Yevgeny Rouban
>> > > -----------------------------------------------------------
>> > >
>> > > From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org]
>> On
>> > Behalf Of Kaylor, Andrew via llvm-dev
>> > > Sent: Thursday, March 22, 2018 3:52 AM
>> > > To: David Blaikie <dblaikie at gmail.com>; llvm-dev
>> > <llvm-dev at lists.llvm.org>; Friedman, Eli <efriedma at codeaurora.org>
>> > > Subject: Re: [llvm-dev] Opt Bisect layering
>> > >
>> > >
>> > >
>> > > There is a patch under review right now from someone who
>> > wants to provide a mechanism to replace OptBisect as the pass gate
>> > keeping mechanism.
>> > >
>> > >
>> > >
>> > > https://reviews.llvm.org/D44464
>> > >
>> > >
>> > >
>> > > Possibly we could take this opportunity to move
>> OptBisect to
>> > a different layer, though I don’t know where else it would belong.
>> > >
>> > >
>> > >
>> > > The other possibility is that we could have the caller
>> pass
>> > in a description instead of a pointer to the pass and the IR unit.
>> > OptBisect isn’t doing anything with them other than building a
>> string
>> > for output.
>> > >
>> > >
>> > >
>> > > -Andy
>> > >
>> > > _______________________________________________
>> > > LLVM Developers mailing list
>> > > llvm-dev at lists.llvm.org
>> > > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>> > >
>> > >
>> > >
>> > > _______________________________________________
>> > > LLVM Developers mailing list
>> > > llvm-dev at lists.llvm.org
>> > > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>> >
>> >
>> > _______________________________________________
>> > LLVM Developers mailing list
>> > llvm-dev at lists.llvm.org
>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>> >
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180423/37968b0e/attachment.html>
More information about the llvm-dev
mailing list