llvm dev - Jun 2020 - optnone/skipping passes in the new pass manager

Looking through some of the remaining test failures under the new pass
manager, I've narrowed down one of the failures in GWPAsan(!) to the fact
that the new pass manager doesn't properly skip passes like the old pass
manager. For example, when a function is marked optnone, or when using
https://llvm.org/docs/OptBisect.html.

Lots of passes (e.g. SROA) will do the following under the legacy pass
manager:

  bool runOnFunction(Function &F) override {
    if (skipFunction(F))
      return false;
    // do pass
  }

What's the right way to proceed with this? There are 50-100 calls to
skipFunction() in legacy passes. This doesn't even account for other types
of IR units, like skipModule(Module&).

I suppose it's possible to manually go in and add in the same check in the
new passes, but that seems tedious (and how do you test that at scale?
clearly there aren't many tests for it right now since check-llvm passes
under the new pass manager). An alternative of skipping passes at the pass
manager level would require marking each pass as necessary/optional (I
think...).
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200607/5c5ec1a2/attachment.html>

When the optnone design was being discussed, Chandler specifically rejected
having the pass manager involved in the decision (which was the original
proposed design).  Assuming he still feels the same way now, if the existing
`skipFunction` calls aren’t being executed under the new pass manager, then each
pass that has that call will need to be modified accordingly (added to the NPM
path or moved to some common point).  It would be best if the `skipFunction`
calls were handled consistently in all passes so that it would become part of
the normal pass boilerplate.

I suspect any `skipFunction` or opt-bisection tests have been written to force
the old pass manager, which is why defaulting to the new pass manager doesn’t
fail anywhere.
--paulr

From: llvm-dev <llvm-dev-bounces at lists.llvm.org> On Behalf Of Arthur
Eubanks via llvm-dev
Sent: Sunday, June 7, 2020 7:59 PM
To: llvm-dev <llvm-dev at lists.llvm.org>
Subject: [llvm-dev] optnone/skipping passes in the new pass manager

Looking through some of the remaining test failures under the new pass manager,
I've narrowed down one of the failures in GWPAsan(!) to the fact that the
new pass manager doesn't properly skip passes like the old pass manager. For
example, when a function is marked optnone, or when using
https://llvm.org/docs/OptBisect.html.

Lots of passes (e.g. SROA) will do the following under the legacy pass manager:

  bool runOnFunction(Function &F) override {
    if (skipFunction(F))
      return false;
    // do pass
  }

What's the right way to proceed with this? There are 50-100 calls to
skipFunction() in legacy passes. This doesn't even account for other types
of IR units, like skipModule(Module&).

I suppose it's possible to manually go in and add in the same check in the
new passes, but that seems tedious (and how do you test that at scale? clearly
there aren't many tests for it right now since check-llvm passes under the
new pass manager). An alternative of skipping passes at the pass manager level
would require marking each pass as necessary/optional (I think...).
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200608/9da207d9/attachment.html>

re: Paul: Sounds about right. (+Chandler just in case he wants to weigh in)

On Mon, Jun 8, 2020 at 7:11 AM Robinson, Paul via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
>
> When the optnone design was being discussed, Chandler specifically rejected
having the pass manager involved in the decision (which was the original
proposed design).  Assuming he still feels the same way now, if the existing
`skipFunction` calls aren’t being executed under the new pass manager, then each
pass that has that call will need to be modified accordingly (added to the NPM
path or moved to some common point).  It would be best if the `skipFunction`
calls were handled consistently in all passes so that it would become part of
the normal pass boilerplate.
>
>
>
> I suspect any `skipFunction` or opt-bisection tests have been written to
force the old pass manager, which is why defaulting to the new pass manager
doesn’t fail anywhere.
>
> --paulr
>
>
>
> From: llvm-dev <llvm-dev-bounces at lists.llvm.org> On Behalf Of
Arthur Eubanks via llvm-dev
> Sent: Sunday, June 7, 2020 7:59 PM
> To: llvm-dev <llvm-dev at lists.llvm.org>
> Subject: [llvm-dev] optnone/skipping passes in the new pass manager
>
>
>
> Looking through some of the remaining test failures under the new pass
manager, I've narrowed down one of the failures in GWPAsan(!) to the fact
that the new pass manager doesn't properly skip passes like the old pass
manager. For example, when a function is marked optnone, or when using
https://llvm.org/docs/OptBisect.html.
>
>
>
> Lots of passes (e.g. SROA) will do the following under the legacy pass
manager:
>
>
>
>   bool runOnFunction(Function &F) override {
>     if (skipFunction(F))
>       return false;
>
>     // do pass
>
>   }
>
>
>
> What's the right way to proceed with this? There are 50-100 calls to
skipFunction() in legacy passes. This doesn't even account for other types
of IR units, like skipModule(Module&).
>
>
>
> I suppose it's possible to manually go in and add in the same check in
the new passes, but that seems tedious (and how do you test that at scale?
clearly there aren't many tests for it right now since check-llvm passes
under the new pass manager). An alternative of skipping passes at the pass
manager level would require marking each pass as necessary/optional (I
think...).
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

On Mon, Jun 8, 2020 at 7:11 AM Robinson, Paul <paul.robinson at sony.com>
wrote:
> When the optnone design was being discussed, Chandler specifically
> rejected having the pass manager involved in the decision (which was the
> original proposed design).  Assuming he still feels the same way now, if
> the existing `skipFunction` calls aren’t being executed under the new pass
> manager, then each pass that has that call will need to be modified
> accordingly (added to the NPM path or moved to some common point).  It
> would be best if the `skipFunction` calls were handled consistently in all
> passes so that it would become part of the normal pass boilerplate.
>
Makes sense, thanks for the background. I'll see if there's a clean way
to
make this functionality easy to use in the NPM.
>
>
> I suspect any `skipFunction` or opt-bisection tests have been written to
> force the old pass manager, which is why defaulting to the new pass manager
> doesn’t fail anywhere.
>
I took a closer look at some optnone tests and they use opt instead of
clang. opt still defaults to the legacy pass manager regardless of clang's
default.
> --paulr
>
>
>
> *From:* llvm-dev <llvm-dev-bounces at lists.llvm.org> *On Behalf Of
*Arthur
> Eubanks via llvm-dev
> *Sent:* Sunday, June 7, 2020 7:59 PM
> *To:* llvm-dev <llvm-dev at lists.llvm.org>
> *Subject:* [llvm-dev] optnone/skipping passes in the new pass manager
>
>
>
> Looking through some of the remaining test failures under the new pass
> manager, I've narrowed down one of the failures in GWPAsan(!) to the
fact
> that the new pass manager doesn't properly skip passes like the old
pass
> manager. For example, when a function is marked optnone, or when using
> https://llvm.org/docs/OptBisect.html.
>
>
>
> Lots of passes (e.g. SROA) will do the following under the legacy pass
> manager:
>
>
>
>   bool runOnFunction(Function &F) override {
>     if (skipFunction(F))
>       return false;
>
>     // do pass
>
>   }
>
>
>
> What's the right way to proceed with this? There are 50-100 calls to
> skipFunction() in legacy passes. This doesn't even account for other
types
> of IR units, like skipModule(Module&).
>
>
>
> I suppose it's possible to manually go in and add in the same check in
the
> new passes, but that seems tedious (and how do you test that at scale?
> clearly there aren't many tests for it right now since check-llvm
passes
> under the new pass manager). An alternative of skipping passes at the pass
> manager level would require marking each pass as necessary/optional (I
> think...).
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200608/19f6e81f/attachment.html>

I think adding a before-pass callback checking optnone to PassInstrumentation of
NPM should suffice for the `skipFunction` alone. However, it is closely related
to opt-bisect which makes it more complicated.

This was the previous discussion.
http://lists.llvm.org/pipermail/llvm-dev/2018-September/126477.html


________________________________________
From: llvm-dev <llvm-dev-bounces at lists.llvm.org> on behalf of Arthur
Eubanks via llvm-dev <llvm-dev at lists.llvm.org>
Sent: Sunday, June 7, 2020 4:58 PM
To: llvm-dev
Subject: [llvm-dev] optnone/skipping passes in the new pass manager

Looking through some of the remaining test failures under the new pass manager,
I've narrowed down one of the failures in GWPAsan(!) to the fact that the
new pass manager doesn't properly skip passes like the old pass manager. For
example, when a function is marked optnone, or when using
https://llvm.org/docs/OptBisect.html.

Lots of passes (e.g. SROA) will do the following under the legacy pass manager:

  bool runOnFunction(Function &F) override {
    if (skipFunction(F))
      return false;
    // do pass
  }

What's the right way to proceed with this? There are 50-100 calls to
skipFunction() in legacy passes. This doesn't even account for other types
of IR units, like skipModule(Module&).

I suppose it's possible to manually go in and add in the same check in the
new passes, but that seems tedious (and how do you test that at scale? clearly
there aren't many tests for it right now since check-llvm passes under the
new pass manager). An alternative of skipping passes at the pass manager level
would require marking each pass as necessary/optional (I think...).

Thanks for the link to the previous discussion, that's super helpful!
I'll
have to read it over more closely again.

That does remind me of another item on the NPM TODO list: codegen is still
using the legacy pass manager. I don't think it's a blocker as long as
things like opt-bisect work with it (as mentioned in the previous
discussion).

On Mon, Jun 8, 2020 at 5:56 PM Chen, Yuanfang <Yuanfang.Chen at sony.com>
wrote:
> I think adding a before-pass callback checking optnone to
> PassInstrumentation of NPM should suffice for the `skipFunction` alone.
> However, it is closely related to opt-bisect which makes it more
> complicated.
>
> This was the previous discussion.
> http://lists.llvm.org/pipermail/llvm-dev/2018-September/126477.html
>
>
> ________________________________________
> From: llvm-dev <llvm-dev-bounces at lists.llvm.org> on behalf of
Arthur
> Eubanks via llvm-dev <llvm-dev at lists.llvm.org>
> Sent: Sunday, June 7, 2020 4:58 PM
> To: llvm-dev
> Subject: [llvm-dev] optnone/skipping passes in the new pass manager
>
> Looking through some of the remaining test failures under the new pass
> manager, I've narrowed down one of the failures in GWPAsan(!) to the
fact
> that the new pass manager doesn't properly skip passes like the old
pass
> manager. For example, when a function is marked optnone, or when using
> https://llvm.org/docs/OptBisect.html.
>
> Lots of passes (e.g. SROA) will do the following under the legacy pass
> manager:
>
>   bool runOnFunction(Function &F) override {
>     if (skipFunction(F))
>       return false;
>     // do pass
>   }
>
> What's the right way to proceed with this? There are 50-100 calls to
> skipFunction() in legacy passes. This doesn't even account for other
types
> of IR units, like skipModule(Module&).
>
> I suppose it's possible to manually go in and add in the same check in
the
> new passes, but that seems tedious (and how do you test that at scale?
> clearly there aren't many tests for it right now since check-llvm
passes
> under the new pass manager). An alternative of skipping passes at the pass
> manager level would require marking each pass as necessary/optional (I
> think...).
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200609/289d2aa4/attachment.html>