<div dir="ltr"><div dir="ltr">In case anyone in this thread is interested, I have proposed a fix to the layering issue in <a href="https://reviews.llvm.org/D58406" target="_blank">https://reviews.llvm.org/D58406</a></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, May 21, 2018 at 6:29 PM David Blaikie via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Ping - did this end up progressing? (I might've missed or forgotten about anything coming out on the list)</div><br><div class="gmail_quote"><div dir="ltr">On Thu, May 3, 2018 at 7:28 AM Fedor Sergeev <<a href="mailto:fedor.sergeev@azul.com" target="_blank">fedor.sergeev@azul.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
  
    
  
  <div bgcolor="#FFFFFF">
    David,<br>
    <br>
    we definitely need to address this issue and I did not forget about
    it.<br>
    <br>
    I'm still fleshing out a proposal on a generic solution for "pass
    execution control points",<br>
    which was inspired by the new pass manager needs but then it appears<br>
    we can largerly reuse the implementation for both managers.<br>
    <br>
    Since the implementation should be based on a special Analysis,<br>
    it definitely will be able to address this layering issue (as a
    side-effect :).<br>
    <br>
    I very much hope to be able send something real to the list in less
    than a week.<br>
    <br>
    regards,<br>
      Fedor.</div><div bgcolor="#FFFFFF"><br>
    <br>
    <div class="gmail-m_3869374156007302401gmail-m_-5147066076508663193m_3437023337864919201moz-cite-prefix">On 04/24/2018 01:42 AM, David Blaikie
      wrote:<br>
    </div>
    <blockquote type="cite">
      
      <div dir="ltr">Ping on this - any chance we can look at fixing the
        OptBisect layering here/now?<br>
        <br>
        Could we move the implementation into Analysis & require
        users to set it, rather than having it as a default value in IR?</div>
      <br>
      <div class="gmail_quote">
        <div dir="ltr">On Tue, Apr 3, 2018 at 9:25 AM David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>>
          wrote:<br>
        </div>
        <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
          <div dir="ltr">
            <div class="gmail_quote">
              <div dir="ltr">On Tue, Apr 3, 2018 at 8:50 AM Fedor
                Sergeev <<a href="mailto:fedor.sergeev@azul.com" target="_blank">fedor.sergeev@azul.com</a>>
                wrote:<br>
              </div>
              <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> >
                Pass Manager (PassManager.h) itself, does not - it's
                only templates,<br>
                none of it depends on Region, Loop, etc.<br>
                Well, true but the problem happens when you try to
                instantiate the thing.<br>
                And for generic features like opt-bisection,
                ir-print-after-all etc that<br>
                want:<br>
                   - to have a say before/on/after every execution of
                every pass<br>
                   - have a shared implementation of the main logic<br>
                <br>
                you have to do most of the following:<br>
                   1. instantiate your interfaces for all the IRUnits<br>
                   2. have pass manager doing the job for you directly<br>
                   3. extend pass interface with specific helpers for
                your job<br>
                (skipFunction)<br>
                <br>
                neither of those helps perfect layering...<br>
                And with new pass manager having no common Pass
                hierarchy this gets even<br>
                more clumsy.<br>
              </blockquote>
            </div>
          </div>
          <div dir="ltr">
            <div class="gmail_quote">
              <div><br>
                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.<br>
                 </div>
            </div>
          </div>
          <div dir="ltr">
            <div class="gmail_quote">
              <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
                <br>
                 > I'm happy to discuss/help design this area as well
                if you'd like :)<br>
                Yeah, I'm interested to continue this design discussion,
                although my<br>
                interests<br>
                are primarily in the area of new pass manager currently.<br>
                I'm going to post a separate RFC on that topic.<br>
              </blockquote>
            </div>
          </div>
          <div dir="ltr">
            <div class="gmail_quote">
              <div><br>
                Sounds good.<br>
                <br>
                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?<br>
                 </div>
            </div>
          </div>
          <div dir="ltr">
            <div class="gmail_quote">
              <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
                <br>
                regards,<br>
                   Fedor.<br>
                <br>
                On 04/03/2018 06:16 PM, David Blaikie wrote:<br>
                 ><br>
                 ><br>
                 > On Mon, Apr 2, 2018 at 11:32 PM Fedor Sergeev via
                llvm-dev<br>
                <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>>
                wrote:<br>
                 ><br>
                 >     On 03/30/2018 12:05 AM, David Blaikie via
                llvm-dev wrote:<br>
                 >      > & now looking back at the
                patch-in-progress, I see it allows<br>
                setting<br>
                 >     the OptBisector/OptPassGate as suggested in
                (2).<br>
                 >     Well, the patch currently discussed does not
                attempt to solve the<br>
                 >     passgate object management issue.<br>
                 >     It is left for the discretion of passgate
                object provider.<br>
                 ><br>
                 >      ><br>
                 >      > If that becomes the /only/ option (ie:
                LLVMContext has no default<br>
                 >     OptPassGate) then the virtual interface could
                be kept down in IR<br>
                (though<br>
                 >     it's still a bit questionable to have those
                Analysis types (Loop,<br>
                 >     Region, CallGraphSCC) even declared in IR).
                Then the<br>
                implementation of<br>
                 >     OptBisector could be moved into Analysis -
                freely able to depend<br>
                on the<br>
                 >     concrete Analysis types.<br>
                 ><br>
                 >     To me this is a "Pass Manager catch" - entity
                that attempts to<br>
                control<br>
                 >     all the passes needs to be part of (or tightly
                cooperate with)<br>
                pass manager.<br>
                 >     Pass manager is currently in IR, and perhaps
                rightfully so.<br>
                 >     Yet passes that it controls work on "IR units"
                which are either IR or<br>
                 >     Analysis, thus Analysis leaks into the
                interfaces inevitably.<br>
                 >     Kinda logical conflict it is...<br>
                 ><br>
                 ><br>
                 > This is in response to my "it's still a bit
                questionable" comment?<br>
                That's not too important - I'm not pushing to change
                that if we can get<br>
                the mechanical layering functional regardless, by only
                having forward<br>
                declarations of those different Analysis entities in
                llvm/IR, but not<br>
                need their definitions except in the implementation of
                this virtual<br>
                interface which could live in llvm/Analysis.<br>
                 ><br>
                 > But to discuss it anyway: It seems a bit different
                that the "Pass<br>
                Manager catch" depends on the concrete types but the
                Pass Manager<br>
                (PassManager.h) itself, does not - it's only templates,
                none of it<br>
                depends on Region, Loop, etc. If the catch could be
                implemented<br>
                similarly to the manager itself, then it'd have the same
                layering<br>
                requirements & no problem. But I haven't looked
                closely enough at the<br>
                APIs to figure out if/how that might be done - the
                current<br>
                implementation/mechanisms are at odds because of the
                incompatibility of<br>
                templates and virtual dispatch (can't have a virtual
                function template -<br>
                it'd have an unbounded/unknowable number of vtable
                entries, etc). Some<br>
                sort of visitor-y thing might be needed/useful, I'm not
                sure. But again,<br>
                not sure this is necessary to address/fix for the issues
                I'm<br>
                seeing/pushing to deal with - but I'm happy to
                discuss/help design this<br>
                area as well if you'd like :)<br>
                 ><br>
                 > - Dave<br>
                 ><br>
                 ><br>
                 ><br>
                 >     regards,<br>
                 >        Fedor.<br>
                 ><br>
                 >      ><br>
                 >      > - Dave<br>
                 >      ><br>
                 >      > On Thu, Mar 29, 2018 at 2:01 PM David
                Blaikie<br>
                <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>>
                wrote:<br>
                 >      ><br>
                 >      >     So... looking at OptBisect, I have a
                few thoughts:<br>
                 >      ><br>
                 >      >     1) what's the purpose of the virtual<br>
                interface/OptPassGate? I'm<br>
                 >     guessing maybe that worked around the circular
                referencing in these<br>
                 >     APIs? hmm, no, I suppose that wouldn't work/be
                relevant here.<br>
                 >      ><br>
                 >      >     2) Why is OptBisector a
                ManagedStatic? That seems pretty<br>
                 >     antithetical to the role of LLVMContext.
                When/why would a user be<br>
                 >     bisecting over multiple LLVMContexts? &
                even then, maybe it'd be more<br>
                 >     suitable for that grouping (the scope for the
                bisection) to be API<br>
                 >     driven - passing the bisector into the
                LLVMContext ctor to define the<br>
                 >     set of contexts that share a bisector?<br>
                 >      ><br>
                 >      >     On Wed, Mar 21, 2018 at 10:20 PM
                Yevgeny Rouban via llvm-dev<br>
                 >     <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>>
                wrote:<br>
                 >      ><br>
                 >      >         Andrew,<br>
                 >      ><br>
                 >      >         I would not make the caller pass
                the description of the IR<br>
                 >     unit. That is because it would result in the
                description<br>
                generated every<br>
                 >     time even if OptBisect is disabled.
                Description generation is not<br>
                very chip.<br>
                 >      >         Thinking on the OptBisect
                extension, I believe passing the<br>
                 >     units are the right choice because
                OptPassGates may use them to make<br>
                 >     pass skipping decisions.<br>
                 >      ><br>
                 >      >         -Yevgeny Rouban<br>
                 >      >
                -----------------------------------------------------------<br>
                 >      ><br>
                 >      >         From: llvm-dev [mailto:<a href="mailto:llvm-dev-bounces@lists.llvm.org" target="_blank">llvm-dev-bounces@lists.llvm.org</a>]
                On<br>
                 >     Behalf Of Kaylor, Andrew via llvm-dev<br>
                 >      >         Sent: Thursday, March 22, 2018
                3:52 AM<br>
                 >      >         To: David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>>;
                llvm-dev<br>
                 >     <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>>;
                Friedman, Eli <<a href="mailto:efriedma@codeaurora.org" target="_blank">efriedma@codeaurora.org</a>><br>
                 >      >         Subject: Re: [llvm-dev] Opt
                Bisect layering<br>
                 >      ><br>
                 >      ><br>
                 >      ><br>
                 >      >         There is a patch under review
                right now from someone who<br>
                 >     wants to provide a mechanism to replace
                OptBisect as the pass gate<br>
                 >     keeping mechanism.<br>
                 >      ><br>
                 >      ><br>
                 >      ><br>
                 >      >         <a href="https://reviews.llvm.org/D44464" rel="noreferrer" target="_blank">https://reviews.llvm.org/D44464</a><br>
                 >      ><br>
                 >      ><br>
                 >      ><br>
                 >      >         Possibly we could take this
                opportunity to move<br>
                OptBisect to<br>
                 >     a different layer, though I don’t know where
                else it would belong.<br>
                 >      ><br>
                 >      ><br>
                 >      ><br>
                 >      >         The other possibility is that we
                could have the caller<br>
                pass<br>
                 >     in a description instead of a pointer to the
                pass and the IR unit.<br>
                 >     OptBisect isn’t doing anything with them other
                than building a string<br>
                 >     for output.<br>
                 >      ><br>
                 >      ><br>
                 >      ><br>
                 >      >         -Andy<br>
                 >      ><br>
                 >      >
                _______________________________________________<br>
                 >      >         LLVM Developers mailing list<br>
                 >      >         <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
                 >      > <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
                 >      ><br>
                 >      ><br>
                 >      ><br>
                 >      >
                _______________________________________________<br>
                 >      > LLVM Developers mailing list<br>
                 >      > <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
                 >      > <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
                 ><br>
                 ><br>
                 >    
                _______________________________________________<br>
                 >     LLVM Developers mailing list<br>
                 >     <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
                 >     <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
                 ><br>
                <br>
              </blockquote>
            </div>
          </div>
        </blockquote>
      </div>
    </blockquote>
    <br>
  </div>

</blockquote></div>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div>