public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Aldy Hernandez <aldyh@redhat.com>
To: Richard Biener <richard.guenther@gmail.com>,
	"MacLeod, Andrew" <amacleod@redhat.com>
Cc: "Martin Liška" <mliska@suse.cz>, "GCC Patches" <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH] Loop unswitching: support gswitch statements.
Date: Thu, 25 Nov 2021 11:38:10 +0100	[thread overview]
Message-ID: <CAGm3qMXZmVteec6nk+5Lkq9w5Wa+6oWHLa3VZYmRGzyfFO6J2Q@mail.gmail.com> (raw)
In-Reply-To: <CAFiYyc1ReTNwTgbOqMX8t0x7j8jYgXxyWek2c1NGY=_bQymt8A@mail.gmail.com>

On Wed, Nov 24, 2021 at 9:00 AM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Tue, Nov 23, 2021 at 5:36 PM Martin Liška <mliska@suse.cz> wrote:
> >
> > On 11/23/21 16:20, Martin Liška wrote:
> > > Sure, so for e.g. case 1 ... 5 we would need to create a new unswitch_predicate
> > > with 1 <= index && index <= 5 tree predicate (and the corresponding irange range).
> > > Later once we unswitch on it, we should use a special unreachable_flag that will
> > > be used for marking of dead edges (similarly how we fold gconds to boolean_{false/true}_node.
> > > Does it make sense?
> >
> > I have thought about it more and it's not enough. What we really want is having a irange
> > for *each edge* (2 for gconds and multiple for gswitchs). Once we select a unswitch_predicate,
> > then we need to fold_range in true/false loop all these iranges. Doing that we can handle situations like:
> >
> > if (index < 1)
> >     do_something1
> >
> > if (index > 2)
> >     do_something2
> >
> > switch (index)
> >     case 1 ... 2:
> >       do_something;
> > ...
> >
> > as seen the once we unswitch on 'index < 1' and 'index > 2', then the first case will be taken in the false_edge
> > of 'index > 2' loop unswitching.
>
> Hmm.  I'm not sure it needs to be this complicated.  We're basically
> evaluating ranges/predicates based
> on a fixed set of versioning predicates.  Your implementation created
> "predicates" for the to be simplified
> conditions but in the end we like to evaluate the actual stmt to
> figure the taken/not taken edges.  IIRC
> elsewhere Andrew showed a snipped on how to evaluate a stmt with a
> given range - not sure if that
> was useful enough.  So what I think would be nice if we could somehow
> use rangers path query
> without an actual CFG.  So we virtuall have
>
>   if (versioning-predicate1)
>     if (versioning-predicate2)
>        ;
>    else
>       for (;;) // out current loop
>         {
>           ...
>               if (condition)
>                 ;
>          ...
>               switch (var)
>                  {
> ...
>                   }
>         }
>
> and versioning-predicate1 and versioning-predicate2 are not in the IL.
> What we'd like
> to do is seed the path query with a "virtual" path through the two
> predicates to the
> entry of the loop and compute_ranges based on those.  Then we like to
> use range_of_stmt on 'if (condition)' and 'switch (var)' to determine
> not taken edges.

Huh, that's an interesting idea.  We could definitely adapt
path_range_query to work with an artificial sequence of blocks, but it
would need some surgery.  Off the top of my head:

a) The phi handling code looks for specific edges in the path (both
for intra path ranges and for relations inherent in PHIs).
b) The exported ranges between blocks in the path, probably needs some
massaging.
c) compute_outgoing_relations would need some work as you mention below...

> Looking somewhat at the sources it seems like we "simply" need to do what
> compute_outgoing_relations does - unfortunately the code lacks comments
> so I have no idea what jt_fur_source src (...).register_outgoing_edges does ...

fur_source is an abstraction for operands to the folding mechanism:

// Source of all operands for fold_using_range and gori_compute.
// It abstracts out the source of an operand so it can come from a stmt or
// and edge or anywhere a derived class of fur_source wants.
// The default simply picks up ranges from the current range_query.

class fur_source
{
}

When passed to register_outgoing_edges, it registers outgoing
relations out of a conditional.  I pass it the known outgoing edge out
of the conditional, so only the relational on that edge is recorded.
I have overloaded fur_source into a path specific jt_fur_source that
uses a path_oracle to register relations as they would occur along a
path.  Once register_outgoing_edges is called on each outgoing edge
between blocks in a path, the relations will have been set, and can be
seen by the range_of_stmt:

path_range_query::range_of_stmt (irange &r, gimple *stmt, tree)
{
...
  // If resolving unknowns, fold the statement making use of any
  // relations along the path.
  if (m_resolve)
    {
      fold_using_range f;
      jt_fur_source src (stmt, this, &m_ranger->gori (), m_path);
      if (!f.fold_stmt (r, stmt, src))
    r.set_varying (type);
    }
...
}

register_outgoing_edges would probably have to be adjusted for your
CFGless paths, and maybe the path_oracle (Andrew??).

My apologies.  The jt_fur_source is not only  wrongly named
"jump_thread", but is the least obvious part of the solver.  There are
some comments for jt_fur_source, but its use could benefit from better
comments throughout.  Let's see if I have some time before my leave to
document things better.

Aldy

>
> Anyway, for now manually simplifying things is fine but I probably would still
> stick to a basic interface that marks not taken outgoing edges of a stmt based
> on the set of versioning predicates.
>
> Richard.
>
> >
> > Martin
>


  parent reply	other threads:[~2021-11-25 10:38 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-15  8:46 Martin Liška
2021-09-19 16:50 ` Jeff Law
2021-09-28 11:50 ` Richard Biener
2021-09-28 20:39   ` Andrew MacLeod
2021-09-29  8:43     ` Richard Biener
2021-09-29 15:20       ` Andrew MacLeod
2021-09-29 15:28         ` Jeff Law
2021-09-29 15:59           ` Andrew MacLeod
2021-09-30  7:33           ` Richard Biener
2021-11-08 15:05     ` Martin Liška
2021-11-08 18:34       ` Andrew MacLeod
2021-11-08 19:45       ` Andrew MacLeod
2021-11-09 13:37         ` Richard Biener
2021-11-09 16:41           ` Andrew MacLeod
2021-11-10  7:52             ` Aldy Hernandez
2021-11-10  8:50             ` Richard Biener
2021-11-09 16:44           ` Martin Liška
2021-11-10  8:59             ` Richard Biener
2021-11-10 13:29               ` Martin Liška
2021-11-11  7:15                 ` Richard Biener
2021-11-16 13:53                   ` Martin Liška
2021-11-19  9:49                     ` Richard Biener
2021-11-16 14:40                   ` Martin Liška
2021-11-19 10:00                     ` Richard Biener
2021-11-22 15:06                       ` Martin Liška
2021-11-23 13:58                         ` Richard Biener
2021-11-23 15:20                           ` Martin Liška
2021-11-23 16:36                             ` Martin Liška
2021-11-24  8:00                               ` Richard Biener
2021-11-24 10:48                                 ` Martin Liška
2021-11-24 12:48                                   ` Richard Biener
2021-11-24 14:14                                     ` Martin Liška
2021-11-24 14:32                                       ` Martin Liška
2021-11-26  8:12                                         ` Richard Biener
2021-11-29 12:45                                           ` Martin Liška
2021-11-30 11:17                                             ` Richard Biener
2021-12-01 14:10                                               ` Martin Liška
2021-12-01 14:19                                                 ` Richard Biener
2021-12-01 14:25                                                   ` Martin Liška
2021-12-01 14:34                                                     ` Richard Biener
2021-12-01 14:48                                                       ` Martin Liška
2021-12-01 18:21                                                         ` Andrew MacLeod
2021-12-02 11:45                                                           ` Martin Liška
2021-12-02 12:01                                                             ` Richard Biener
2021-12-02 13:10                                                               ` Martin Liška
2021-12-02 13:46                                                                 ` Richard Biener
2021-12-08 21:06                                                                   ` Andrew MacLeod
2021-12-02 14:27                                                             ` Andrew MacLeod
2021-12-02 16:02                                                               ` Martin Liška
2021-12-03 14:09                                                                 ` Andrew MacLeod
2021-12-09 12:59                                                                   ` Martin Liška
2021-12-09 14:44                                                                     ` Andrew MacLeod
2021-12-09 13:02                                               ` Martin Liška
2022-01-05 12:34                                                 ` Richard Biener
2022-01-06 15:11                                                   ` Andrew MacLeod
2022-01-06 16:02                                                     ` Martin Liška
2022-01-06 16:20                                                       ` Andrew MacLeod
2022-01-06 16:35                                                         ` Martin Liška
2022-01-06 16:42                                                           ` Andrew MacLeod
2022-01-06 16:32                                                       ` Andrew MacLeod
2022-01-06 16:30                                                   ` Martin Liška
2022-01-13 16:01                                                     ` Martin Liška
2022-01-14  7:23                                                       ` Richard Biener
2021-11-25 10:38                                 ` Aldy Hernandez [this message]
2021-11-26  7:45                                   ` Richard Biener
2021-11-24  7:46                             ` Richard Biener
2021-10-05 17:08   ` Andrew MacLeod

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAGm3qMXZmVteec6nk+5Lkq9w5Wa+6oWHLa3VZYmRGzyfFO6J2Q@mail.gmail.com \
    --to=aldyh@redhat.com \
    --cc=amacleod@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=mliska@suse.cz \
    --cc=richard.guenther@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).