<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Mon, Apr 2, 2018 at 11:32 PM Fedor Sergeev via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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 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 (though<br>
it's still a bit questionable to have those Analysis types (Loop,<br>
Region, CallGraphSCC) even declared in IR). Then the implementation of<br>
OptBisector could be moved into Analysis - freely able to depend on the<br>
concrete Analysis types.<br>
<br>
To me this is a "Pass Manager catch" - entity that attempts to control<br>
all the passes needs to be part of (or tightly cooperate with) 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></blockquote><div><br>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.<br><br>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 :)<br><br>- Dave<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
regards,<br>
   Fedor.<br>
<br>
 ><br>
 > - Dave<br>
 ><br>
 > On Thu, Mar 29, 2018 at 2:01 PM David Blaikie <<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 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 generated every<br>
time even if OptBisect is disabled. Description generation is not 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 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 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>
</blockquote></div></div>