public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
From: "amker at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug rtl-optimization/62151] [5 Regression] wrong code at -O2 and -O3 on x86_64-linux-gnu
Date: Thu, 11 Dec 2014 08:52:00 -0000	[thread overview]
Message-ID: <bug-62151-4-apLA8bjY5d@http.gcc.gnu.org/bugzilla/> (raw)
In-Reply-To: <bug-62151-4@http.gcc.gnu.org/bugzilla/>

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62151

--- Comment #16 from amker at gcc dot gnu.org ---
For calls of distribute_notes with from_insn != NULL, I kind of understand why
it is vulnerable, at least when handling REG_DEAD notes.

When we distribute REG_DEAD note of one register from FROM_INSN, generally it
means live range shrink of that register.  For example:
  i1: r1 <- const_0
  i2: r2 <- rx & const_0
            REG_DEAD (rx)
  i3: r3 <- i3src (using r2)
In this case, we need to search backward in instruction flow, find previous
reference of rx and put a REG_DEAD note there, or delete the definition of rx
if there is no other reference.

But in combine, thing is complicate when use of rx in r1 is combined into i2. 
It means live range extend of register rx.  In this case, we need to search
forward and decide whether we need to put a REG_DEAD for rx at i2.  Sometimes
we are lucky when i2 is the predecessor of i3, which means we can infer the
insert point directly.  This is why the code handles this specially (as
quoting):

          /* ...
         If the register is used as an input in I3, it dies there.
         Similarly for I2, if it is nonzero and adjacent to I3.
             ...  */

          else if (reg_referenced_p (XEXP (note, 0), PATTERN (i3)))
        place = i3;
          else if (i2 != 0 && next_nonnote_nondebug_insn (i2) == i3
               && reg_referenced_p (XEXP (note, 0), PATTERN (i2)))
        place = i2;

For other cases, GCC just start from i3 and search backward in flow.  This
causes this PR.

Thing becomes even complicated when we support four insn combination because
it's possible that i1dest is re-set in i2, like below example:
  i0: r0 <- const_0
  i1: r1 <- i1src (using r0)
            REG_DEAD (r0)
  i2: r0 <- i2src (using r1)
  i3: r3 <- i3src (using r0)

Since GCC wrongly handles live range shrink of r0 as live range extend, it
searches from r3 backward in flow, resulting in i2 deleted.

So distribute_notes is complicated and vulnerable because it tries to
understand live range change by guessing based on information like
elim_ix/i2/i3, etc., rather than real live range information.

>From my point of view, all distribute_notes needs to do when "FROM_INSN!=NULL"
are three live range changes:
1) live range shrink, like the first example.
2) live range shrink, but the instruction has already deleted by combine, like
the last example.
3) live range extend, like below example:
      before combine       after combine
  i0: r0 <- const        i0: INSN_DELETED
  i1: r1 <- rx & r0      i1: INSN_DELETED
            REG_DEAD(rx)
  i2: r2 <- r1           i2: r2 <- rx & const
                                   REG_DEAD(rx)
  i3: r3 <- i3src        i3: r3 <- i3src'

Problem is if we can infer live range change from order of i0/i1/i2/i3, using
INSN_LUID maybe?


  parent reply	other threads:[~2014-12-11  8:52 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-15  6:14 [Bug tree-optimization/62151] New: " su at cs dot ucdavis.edu
2014-08-15  7:33 ` [Bug tree-optimization/62151] [5 Regression] " mpolacek at gcc dot gnu.org
2014-08-15  7:34 ` mpolacek at gcc dot gnu.org
2014-08-15  7:42 ` [Bug rtl-optimization/62151] " mpolacek at gcc dot gnu.org
2014-08-15  7:55 ` jakub at gcc dot gnu.org
2014-08-15  8:04 ` amker at gcc dot gnu.org
2014-08-15  9:53 ` rguenth at gcc dot gnu.org
2014-08-15  9:53 ` rguenth at gcc dot gnu.org
2014-08-15 15:39 ` amker at gcc dot gnu.org
2014-08-16 12:11 ` amker at gcc dot gnu.org
2014-08-19 18:55 ` ebotcazou at gcc dot gnu.org
2014-08-20  2:32 ` amker at gcc dot gnu.org
2014-10-02  2:18 ` segher at gcc dot gnu.org
2014-10-02  2:30 ` segher at gcc dot gnu.org
2014-12-09  8:48 ` amker.cheng at gmail dot com
2014-12-09  9:27 ` amker.cheng at gmail dot com
2014-12-10  8:14 ` amker at gcc dot gnu.org
2014-12-11  3:10 ` amker.cheng at gmail dot com
2014-12-11  8:52 ` amker at gcc dot gnu.org [this message]
2014-12-22 10:25 ` amker at gcc dot gnu.org
2014-12-22 10:29 ` amker at gcc dot gnu.org

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=bug-62151-4-apLA8bjY5d@http.gcc.gnu.org/bugzilla/ \
    --to=gcc-bugzilla@gcc.gnu.org \
    --cc=gcc-bugs@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).