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

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

> 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.
That may well be the case. I certainly wouldn't want to speak for Chandler,
and I have to admit the comment I made earlier sounds dangerously like a game of
telephone with all the misunderstandings inherent in that.  My understanding at
the time was that not having the Pass Manager make any decision based on the
contents of the IR was an important part of the issue. I think you have that
covered.

From there, I would argue that it's a matter of extending this sort of
principle through and making sure that at each level the component involved is
doing what it should be doing and not borrowing any part of another
component's domain. I think we're all in agreement about that too.

At this point it feels to me kind of like we're getting a bit of churn.
Concerns have been raised and discussed in circles. It doesn't seem like new
concerns are coming up. Maybe you could now take this input and put together a
concrete proposal for the parts that can be implemented now. I like what
you've done so far and am confident that you're going in the right
direction.

-Andy

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

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

> -----Original Message-----
> From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org] On Behalf Of
> Kaylor, Andrew via llvm-dev
> Sent: Monday, October 01, 2018 7:55 PM
> To: Fedor Sergeev; David Greene; Philip Pfaffe
> Cc: llvm-dev
> Subject: Re: [llvm-dev] OptBisect implementation for new pass manager
> 
> > 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.
> 
> That may well be the case. I certainly wouldn't want to speak for
> Chandler, and I have to admit the comment I made earlier sounds
> dangerously like a game of telephone with all the misunderstandings
> inherent in that.  My understanding at the time was that not having the
> Pass Manager make any decision based on the contents of the IR was an
> important part of the issue. I think you have that covered.
> 
> From there, I would argue that it's a matter of extending this sort of
> principle through and making sure that at each level the component
> involved is doing what it should be doing and not borrowing any part of
> another component's domain. I think we're all in agreement about
that too.
> 
> At this point it feels to me kind of like we're getting a bit of churn.
> Concerns have been raised and discussed in circles. It doesn't seem
like
> new concerns are coming up. Maybe you could now take this input and put
> together a concrete proposal for the parts that can be implemented now. I
> like what you've done so far and am confident that you're going in
the
> right direction.
> 
> -Andy
+1.  I think Fedor has had reasonable answers for everything and it's time
to make it happen.
--paulr
> 
> -----Original Message-----
> From: Fedor Sergeev [mailto:fedor.sergeev at azul.com]
> Sent: Monday, October 01, 2018 4:42 PM
> To: Kaylor, Andrew <andrew.kaylor at intel.com>; David Greene <dag
at cray.com>;
> Philip Pfaffe <philip.pfaffe at gmail.com>
> Cc: llvm-dev <llvm-dev at lists.llvm.org>; Chandler Carruth
> <chandlerc at gmail.com>
> Subject: Re: [llvm-dev] OptBisect implementation for new pass manager
> 
> 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
> >
> >
> 
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev