From: Aldy Hernandez <aldyh@redhat.com>
To: Richard Biener <richard.guenther@gmail.com>
Cc: Michael Matz <matz@suse.de>, Jeff Law <jeffreyalaw@gmail.com>,
GCC Mailing List <gcc@gcc.gnu.org>,
Andrew MacLeod <amacleod@redhat.com>
Subject: Re: More aggressive threading causing loop-interchange-9.c regression
Date: Wed, 8 Sep 2021 18:19:47 +0200 [thread overview]
Message-ID: <6d5695e4-e4eb-14a5-46a6-f425d1514008@redhat.com> (raw)
In-Reply-To: <CAFiYyc2KWNEMD31AdYuNJ-dP7ixMsWtTCtokQpcbRrZctTUqzA@mail.gmail.com>
On 9/8/21 3:49 PM, Richard Biener wrote:
> On Wed, Sep 8, 2021 at 3:25 PM Aldy Hernandez <aldyh@redhat.com> wrote:
>>
>>> It would be helpful to have the patch causing the issue to look at the IL.
>>> But as Micha said, there needs to be a perfect loop nest for interchange
>>> to work.
>>>
>>> Richard.
>>
>> Absolutely! I'm attaching the reduced testcase, as well as the patch.
>>
>> The problematic thread shows up in the thread2 dump:
>>
>> Checking profitability of path (backwards): bb:3 (4 insns) bb:9 (0
>> insns) bb:5
>> Control statement insns: 2
>> Overall: 2 insns
>> Registering FSM jump thread: (5, 9) incoming edge; (9, 3) (3, 8)
>> nocopy; (3, 8)
>
> Well, so as Micha said, the threading destroys the outer loop by
> making it have two entries - we actually _do_ recover from this
> but it results in a non-empty latch of the outer loop and thus
> the loop is no longer a perfect nest. The interchange pass has
> special-sauce to recover from the loop store motion applied
> but not to handle the kind of loop rotation the jump threading
> caused.
Understood. The backedge into BB8 and the non-empty latch seem to cause
problems.
>
> The forward threader guards against this by simply disallowing
> threadings that involve different loops. As I see
The thread in question (5->9->3) is all within the same outer loop,
though. BTW, the backward threader also disallows threading across
different loops (see path_crosses_loops variable).
> the threading done here should be 7->3 (outer loop entry) to bb 8
> rather than one involving the backedge. Also note the condition
> is invariant in the loop and in fact subsumed by the condition
> outside of the loop and it should have been simplified by
> VRP after pass_ch but I see there's a jump threading pass
> inbetween pass_ch and the next VRP which is likely the
> problem.
A 7->3->8 thread would cross loops though, because 7 is outside the
outer loop.
>
> Why does jump threading not try to simplify a condition before
> attempting to thread through it? After all ranger should be around?
That's a very good question ;-). The current code does not use the
ranger to solve unknowns. Anything further back than the first block in
a path will return varying. The plan was to add this ranger solving
functionality as a follow-up.
I have a whole slew of pending patches adding precisely this
functionality. We should be able to solve ranges outside the path with
ranger, as well as relationals occurring in a path.
However, even if there are alternate ways of threading this IL,
something like 5->9->3 could still happen. We need a way to disallow
this. I'm having a hard time determining the hammer for this. I would
vote for disabling threading through latches, but it seems the backward
threader is aware of this scenario and allows it anyhow (see
threaded_through_latch variable). Ughh.
Thanks for looking into this.
Aldy
next prev parent reply other threads:[~2021-09-08 16:19 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-07 11:49 Aldy Hernandez
2021-09-07 14:45 ` Michael Matz
2021-09-08 10:44 ` Aldy Hernandez
2021-09-08 13:13 ` Richard Biener
2021-09-08 13:25 ` Aldy Hernandez
2021-09-08 13:49 ` Richard Biener
2021-09-08 16:19 ` Aldy Hernandez [this message]
2021-09-08 16:39 ` Michael Matz
2021-09-08 18:13 ` Michael Matz
2021-09-09 6:57 ` Richard Biener
2021-09-09 7:37 ` Aldy Hernandez
2021-09-09 7:45 ` Richard Biener
2021-09-09 8:36 ` Aldy Hernandez
2021-09-09 8:58 ` Richard Biener
2021-09-09 9:21 ` Aldy Hernandez
2021-09-09 10:15 ` Richard Biener
2021-09-09 11:28 ` Aldy Hernandez
2021-09-10 15:51 ` Jeff Law
2021-09-10 16:11 ` Aldy Hernandez
2021-09-10 15:43 ` Jeff Law
2021-09-10 16:05 ` Aldy Hernandez
2021-09-10 16:21 ` Jeff Law
2021-09-10 16:38 ` Aldy Hernandez
2021-09-09 16:59 ` Jeff Law
2021-09-09 12:47 ` Michael Matz
2021-09-09 8:14 ` Aldy Hernandez
2021-09-09 8:24 ` Richard Biener
2021-09-09 12:52 ` Michael Matz
2021-09-09 13:37 ` Aldy Hernandez
2021-09-09 14:44 ` Michael Matz
2021-09-09 15:07 ` Aldy Hernandez
2021-09-10 7:04 ` Aldy Hernandez
2021-09-09 16:54 ` Jeff Law
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=6d5695e4-e4eb-14a5-46a6-f425d1514008@redhat.com \
--to=aldyh@redhat.com \
--cc=amacleod@redhat.com \
--cc=gcc@gcc.gnu.org \
--cc=jeffreyalaw@gmail.com \
--cc=matz@suse.de \
--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).