public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jeff Law <law@redhat.com>
To: gcc-patches <gcc-patches@gcc.gnu.org>
Subject: [RFA] [PATCH] [PR tree-optimization/68619] Avoid direct cfg cleanups in tree-ssa-dom.c [0/3]
Date: Tue, 08 Dec 2015 06:15:00 -0000	[thread overview]
Message-ID: <5666757D.5060606@redhat.com> (raw)

Richi and I have been discussing revamping slightly how DOM handles 
conditionals which it detects are always true or always false.

During gcc6 stage1 I added code to allow DOM to clean them up 
immediately, primarily to avoid the waste of having the threader handle 
those cases.  It was also believed that by cleaning things up during the 
DOM walk we could realize some secondary benefits (certain PHIs become 
more likely to collapse down to a const/copy which can then be propagated).

That code causes an interesting problem as shown by 68619.  Essentially 
the CFG has 3 loops, one is a natural loop, the other two are irreducible.

DOM finds conditionals which it can optimize to true/false.  It removes 
the unreachable edges and everything seems perfect.  Except that removal 
of those edges causes the irreducible loops become reducible.  This is a 
good thing, except....

Now we have two new natural loops, which triggers a checking failure 
because we haven't set up loop structures for the newly exposed natural 
loops.

Richi's suggestion (before this problem was reported) was to have DOM 
leave the CFG alone, but otherwise optimize as-if the edges had been 
removed.  Final removal of the edges would be left to cfg_cleanup.  He 
also pointed me at SCCVN which does something similar.

This change essentially has DOM working in the same was as SCCVN.  The 
change is broken into 3 parts.

1. Refactor the code from tree-ssa-sccvn.c into domwalk.c  Essentially 
it's 4 new member functions that a dominator walker can optionally use 
to improve it's behaviour when the pass might make certain edges 
unexecutable.  I need someone to review these changes.  If you've got a 
better name for the member functions, certainly pass them along.  I'm 
not particularly happy with maybe_clear_unreachable_dom.  It feels like 
an internal implementation detail has leaked out, but I'm not really 
sure how to fix it, so any suggestions there are certainly welcome

2. Use the new member functions in tree-ssa-dom.c.  It's pretty simple 
stuff.

3. New tests.  One is the actual 68619 testcase.  Two ICEs for minor 
bugs found during development/testing, one case where we optimize better 
now than before, one for a missed optimization during development.

The patchset as a whole has been bootstrapped and regression tested on 
x86_64-linux-gnu.

Jeff

                 reply	other threads:[~2015-12-08  6:15 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=5666757D.5060606@redhat.com \
    --to=law@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    /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).