llvm dev - Oct 2018 - OptBisect implementation for new pass manager

On 10/01/2018 11:13 PM, David Greene wrote:
> Philip Pfaffe <philip.pfaffe at gmail.com> writes:
>> Sorry, but I strongly oppose to the road you're suggesting here. As
I
>> said before the Pass*Manager* is entirely the wrong place to handle
>> OptNone, the absolutely the wrong design. It makes sense for function
>> *only*, and immediately breaks down everywhere else. Frankly, OptNone
>> should take no role in this particular discussion, and I feel like so
>> far it has because "We've always done it this way".
> I think we're miscommunicating.  I'm not talking about the optnone
> function attribute and I don't think Andy is either.  I'm saying
that
> one option for "skipping" passes that must run is to run them in
a
> degraded mode where they do just enough to generate functional code.
> One way (though maybe not the best way) is to tell the pass to run that
> way via the OptLevel::None value.
We better use "pass execution" terminology rather than
"optimization
level" one.
Lets say that pass "runs in 'skip' mode".
> Even Module passes could run in degraded mode.
>> Second, I don't completely subscribe to the arguments towards
>> not-skippable passes. Fundamentally: Who decides whether a pass is
>> skippable or not? _When_ is a pass skippable, when isn't it?
Neither
>> the Pass nor the PassManager may actually decide that. The only things
>> that do are the pipeline builder and the bisecter. Pipeline builder
>> here means the driver tool that constructs and executes the pipeline
>> (like clang or opt), not PassBuilder. We can achieve that if we put
>> this feature into the instrumentation instead. Here we have the right
>> amount of control, at least cost and minimal complexity. Then, all we
>> need to is get the default treatment right. If we can agree on whether
>> we truly want to not skip some passes 90% of the time, than we can
>> just encode that as an overrideable default, such as by tracking a
>> list of non-skippable passes right there in the implementation.
> I think that's compeletely fine.  You're right that neither the
pass
> manager nor the pass can make those decisions.
Yes, I believe we can state that OptBisect (and OptBisect only) makes 
those decisions.
>    The piece I don't yet
> grasp is how we express that a pass is not skippable (or may be run in
> degraded mode) in various contexts.  How do we provide the information
> so that pass instrumentation may make the correct decision?
As Philip already mentioned, "by tracking a list of non-skippable passes 
right there
in the implementation"   .... of OptBisect, I
assume.
> I do think running a "must run" pass in degraded mode during
bisecting
> is very useful.
And that is the most tricky part - how does pass know that a particular 
pass *invocation*
runs in 'skip' mode?

It appears that the only "proper" way for passes to get
"outside"
information is to ask for the analysis.
So we need special analysis deployed by OptBisect object that tells 
those tricky passes like
scheduler that they need to run in 'skip' mode.

regards,
   FEdor.
>
>                               -David

Some of the issues we're talking about were also considered when I did the
initial implementation of OptBisect. If anyone wants to look at the relevant
reviews they can be found here:

https://reviews.llvm.org/D18576 (Abandoned first attempt)
https://reviews.llvm.org/D19172 (Committed second version)

Then there were a dozen or so revisions to implement the opt-in process.

The highlights were:

- My initial implementation had a hard-coded list of passes that couldn't be
skipped. We agreed that was awful.
- Paul Robinson mentioned that during the initial work on the
"optnone" attribute they want a way for each pass to identify itself
as skippable or not and the pass manager would only run non-skippable passes on
an "optnone" function. He said, "Chandler was adamant that we NOT
do it this way, that the pass manager should remain ignorant of this sort of
trickery and each individual pass would need to decide for itself whether
'optnone' was something to pay attention to." This led to the
current implementation where "optnone" and opt-bisect are both handled
in the passes' run functions.
- At one point I had something (mostly?) working with the new pass manager, but
it broke with Address Sanitizer and since the new pass manager wasn't ready
to go then we decided to strip that part out completely.

-Andy


-----Original Message-----
From: Fedor Sergeev [mailto:fedor.sergeev at azul.com] 
Sent: Monday, October 01, 2018 2:33 PM
To: David Greene <dag at cray.com>; Philip Pfaffe <philip.pfaffe at
gmail.com>
Cc: Kaylor, Andrew <andrew.kaylor at intel.com>; llvm-dev <llvm-dev at
lists.llvm.org>; Zhizhou Yang <zhizhouy at google.com>; David Blaikie
<dblaikie at gmail.com>; Chandler Carruth <chandlerc at gmail.com>
Subject: Re: [llvm-dev] OptBisect implementation for new pass manager

On 10/01/2018 11:13 PM, David Greene wrote:
> Philip Pfaffe <philip.pfaffe at gmail.com> writes:
>> Sorry, but I strongly oppose to the road you're suggesting here. As
I
>> said before the Pass*Manager* is entirely the wrong place to handle 
>> OptNone, the absolutely the wrong design. It makes sense for function 
>> *only*, and immediately breaks down everywhere else. Frankly, OptNone 
>> should take no role in this particular discussion, and I feel like so 
>> far it has because "We've always done it this way".
> I think we're miscommunicating.  I'm not talking about the optnone 
> function attribute and I don't think Andy is either.  I'm saying
that
> one option for "skipping" passes that must run is to run them in
a
> degraded mode where they do just enough to generate functional code.
> One way (though maybe not the best way) is to tell the pass to run 
> that way via the OptLevel::None value.
We better use "pass execution" terminology rather than
"optimization level" one.
Lets say that pass "runs in 'skip' mode".
> Even Module passes could run in degraded mode.
>> Second, I don't completely subscribe to the arguments towards 
>> not-skippable passes. Fundamentally: Who decides whether a pass is 
>> skippable or not? _When_ is a pass skippable, when isn't it?
Neither
>> the Pass nor the PassManager may actually decide that. The only 
>> things that do are the pipeline builder and the bisecter. Pipeline 
>> builder here means the driver tool that constructs and executes the 
>> pipeline (like clang or opt), not PassBuilder. We can achieve that if 
>> we put this feature into the instrumentation instead. Here we have 
>> the right amount of control, at least cost and minimal complexity. 
>> Then, all we need to is get the default treatment right. If we can 
>> agree on whether we truly want to not skip some passes 90% of the 
>> time, than we can just encode that as an overrideable default, such 
>> as by tracking a list of non-skippable passes right there in the
implementation.
> I think that's compeletely fine.  You're right that neither the
pass
> manager nor the pass can make those decisions.
Yes, I believe we can state that OptBisect (and OptBisect only) makes those
decisions.
>    The piece I don't yet
> grasp is how we express that a pass is not skippable (or may be run in 
> degraded mode) in various contexts.  How do we provide the information 
> so that pass instrumentation may make the correct decision?
As Philip already mentioned, "by tracking a list of non-skippable passes
right there in the implementation"   .... of OptBisect, I
assume.
> I do think running a "must run" pass in degraded mode during
bisecting
> is very useful.
And that is the most tricky part - how does pass know that a particular pass
*invocation* runs in 'skip' mode?

It appears that the only "proper" way for passes to get
"outside"
information is to ask for the analysis.
So we need special analysis deployed by OptBisect object that tells those tricky
passes like scheduler that they need to run in 'skip' mode.

regards,
   FEdor.
>
>                               -David

> -----Original Message-----
> From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org] On Behalf Of
Fedor
> Sergeev via llvm-dev
> Sent: Monday, October 01, 2018 5:33 PM
> To: David Greene; Philip Pfaffe
> Cc: llvm-dev
> Subject: Re: [llvm-dev] OptBisect implementation for new pass manager
> 
> On 10/01/2018 11:13 PM, David Greene wrote:
> > Philip Pfaffe <philip.pfaffe at gmail.com> writes:
> >> Sorry, but I strongly oppose to the road you're suggesting
here. As I
> >> said before the Pass*Manager* is entirely the wrong place to
handle
> >> OptNone, the absolutely the wrong design. It makes sense for
function
> >> *only*, and immediately breaks down everywhere else. Frankly,
OptNone
> >> should take no role in this particular discussion, and I feel like
so
> >> far it has because "We've always done it this way".
> > I think we're miscommunicating.  I'm not talking about the
optnone
> > function attribute and I don't think Andy is either.  I'm
saying that
> > one option for "skipping" passes that must run is to run
them in a
> > degraded mode where they do just enough to generate functional code.
> > One way (though maybe not the best way) is to tell the pass to run
that
> > way via the OptLevel::None value.
> We better use "pass execution" terminology rather than
"optimization
> level" one.
> Lets say that pass "runs in 'skip' mode".
> 
> > Even Module passes could run in degraded mode.
> >> Second, I don't completely subscribe to the arguments towards
> >> not-skippable passes. Fundamentally: Who decides whether a pass is
> >> skippable or not? _When_ is a pass skippable, when isn't it?
Neither
> >> the Pass nor the PassManager may actually decide that. The only
things
> >> that do are the pipeline builder and the bisecter. Pipeline
builder
> >> here means the driver tool that constructs and executes the
pipeline
> >> (like clang or opt), not PassBuilder. We can achieve that if we
put
> >> this feature into the instrumentation instead. Here we have the
right
> >> amount of control, at least cost and minimal complexity. Then, all
we
> >> need to is get the default treatment right. If we can agree on
whether
> >> we truly want to not skip some passes 90% of the time, than we can
> >> just encode that as an overrideable default, such as by tracking a
> >> list of non-skippable passes right there in the implementation.
> > I think that's compeletely fine.  You're right that neither
the pass
> > manager nor the pass can make those decisions.
> Yes, I believe we can state that OptBisect (and OptBisect only) makes
> those decisions.
> >    The piece I don't yet
> > grasp is how we express that a pass is not skippable (or may be run in
> > degraded mode) in various contexts.  How do we provide the information
> > so that pass instrumentation may make the correct decision?
> As Philip already mentioned, "by tracking a list of non-skippable
passes
> right there
> in the implementation"   .... of OptBisect, I assume.
> > I do think running a "must run" pass in degraded mode during
bisecting
> > is very useful.
> And that is the most tricky part - how does pass know that a particular
> pass *invocation*
> runs in 'skip' mode?
> 
> It appears that the only "proper" way for passes to get
"outside"
> information is to ask for the analysis.
> So we need special analysis deployed by OptBisect object that tells
> those tricky passes like
> scheduler that they need to run in 'skip' mode.
There's a sort of architectural purity to this, but to make it properly
pervasive it would want to be very little work on the pass author's part
to query and act on it.  At which point we are basically doing the same
helper-in-base-class deal that we have now.  Maybe making it into an
always-available analysis is helpful or simpler, I don't know, I don't
do passes normally. :-)
--paulr
> 
> regards,
>    FEdor.
> 
> >
> >                               -David
> 
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

On 10/02/2018 01:37 AM, paul.robinson at sony.com wrote:
>> And that is the most tricky part - how does pass know that a particular
>> pass *invocation*
>> runs in 'skip' mode?
>>
>> It appears that the only "proper" way for passes to get
"outside"
>> information is to ask for the analysis.
>> So we need special analysis deployed by OptBisect object that tells
>> those tricky passes like
>> scheduler that they need to run in 'skip' mode.
> There's a sort of architectural purity to this, but to make it properly
> pervasive it would want to be very little work on the pass author's
part
> to query and act on it.
Majority of passes do not need to worry about it, since they are just run,
not trying to determine its optimization mode through a dark magic 
interaction
with OptBisect. As soon as they get run they "just run".
Thus I dont really worry about this being very little work for passes.

regards,
   Fedor.
>    At which point we are basically doing the same
> helper-in-base-class deal that we have now.  Maybe making it into an
> always-available analysis is helpful or simpler, I don't know, I
don't
> do passes normally. :-)
> --paulr
>
>> regards,
>>     FEdor.
>>
>>>                                -David
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

On 10/02/2018 01:17 AM, Kaylor, Andrew wrote:
> Some of the issues we're talking about were also considered when I did
the initial implementation of OptBisect. If anyone wants to look at the relevant
reviews they can be found here:
Thanks for sharing these, very useful!
> https://reviews.llvm.org/D18576 (Abandoned first attempt)
> https://reviews.llvm.org/D19172 (Committed second version)
>
> Then there were a dozen or so revisions to implement the opt-in process.
>
> The highlights were:
>
> - My initial implementation had a hard-coded list of passes that
couldn't be skipped. We agreed that was awful.
> - Paul Robinson mentioned that during the initial work on the
"optnone" attribute they want a way for each pass to identify itself
as skippable or not and the pass manager would only run non-skippable passes on
an "optnone" function. He said, "Chandler was adamant that we NOT
do it this way, that the pass manager should remain ignorant of this sort of
trickery and each individual pass would need to decide for itself whether
'optnone' was something to pay attention to." This led to the
current implementation where "optnone" and opt-bisect are both handled
in the passes' run functions.
If I interpret Chandler right, generic nature of pass instrumentation 
framework solves this objection by abstracting everything out of pass 
manager itself.
There is now a good alternative to both coding everything in all the 
passes and polluting pass manager with non-pass-managing details.

regards,
   Fedor.
> - At one point I had something (mostly?) working with the new pass manager,
but it broke with Address Sanitizer and since the new pass manager wasn't
ready to go then we decided to strip that part out completely.
>
> -Andy
>
>