From: Jakub Jelinek <jakub@redhat.com>
To: Eric Botcazou <ebotcazou@adacore.com>
Cc: gcc-patches@gcc.gnu.org
Subject: [PATCH] Fix combiner ICEs after my recent patch (PR rtl-optimization/48549)
Date: Mon, 11 Apr 2011 10:50:00 -0000 [thread overview]
Message-ID: <20110411104958.GY17079@tyan-ft48-01.lab.bos.redhat.com> (raw)
[-- Attachment #1: Type: text/plain, Size: 5189 bytes --]
Hi!
On the testcase below we ICE since my PR48343 patch.
The problem is that update_cfg_for_uncondjump delete_insns the noop move,
if the insn to be deleted is last_combined_insn (which can be both
if it is i3 or when retrying and undobuf.other_insn happens to be
last_combined_insn), next propagate_for_debug won't find the deleted
insn in the stream and thus will search through the whole rest of the
function and crash on dereferencing NULL NEXT_INSN.
The following patch fixes it, bootstrapped/regtested on x86_64-linux
and i686-linux (also with the patch attached after it), but it is
mainly for discussion what to do.
The propagate_for_debug change alone could fix it, we should never
fall through into next basic block. We are unforuntately not deleting
just jumps (which ought to appear at the end of bbs), but also
any other noop moves, which I think is unintentional, we have
delete_noop_moves that should clean that up instead (see the second patch).
With the second patch, bootstrap succeeded even with gcc_assert (at_end);
in update_cfg_for_uncondjump, so either it is always at the end, or at
least very often, thus perhaps with the second patch being applied
in combine_instructions we could just for INSN_DELETED_P clear
last_combined_insn right away, meaning we want to propagate till end of
current bb.
2011-04-11 Jakub Jelinek <jakub@redhat.com>
PR rtl-optimization/48549
* combine.c (propagate_for_debug): Also stop after BB_END of
this_basic_block.
(combine_instructions): If last_combined_insn has been deleted,
set last_combined_insn to the next nondebug insn, or NULL
if after the end of BB.
(try_combine): Handle last_combined_insn being NULL.
* g++.dg/opt/pr48549.C: New test.
--- gcc/combine.c.jj 2011-04-10 19:05:25.000000000 +0200
+++ gcc/combine.c 2011-04-11 09:13:04.000000000 +0200
@@ -1179,7 +1179,7 @@ combine_instructions (rtx f, unsigned in
FOR_EACH_BB (this_basic_block)
{
- rtx last_combined_insn = NULL_RTX;
+ rtx last_combined_insn = pc_rtx;
optimize_this_for_speed_p = optimize_bb_for_speed_p (this_basic_block);
last_call_luid = 0;
mem_last_set = -1;
@@ -1198,9 +1198,26 @@ combine_instructions (rtx f, unsigned in
next = 0;
if (NONDEBUG_INSN_P (insn))
{
- if (last_combined_insn == NULL_RTX
- || DF_INSN_LUID (last_combined_insn) < DF_INSN_LUID (insn))
+ if (last_combined_insn == pc_rtx)
last_combined_insn = insn;
+ else if (last_combined_insn)
+ {
+ if (INSN_DELETED_P (last_combined_insn))
+ {
+ while (last_combined_insn
+ != NEXT_INSN (BB_END (this_basic_block))
+ && (!NONDEBUG_INSN_P (last_combined_insn)
+ || INSN_DELETED_P (last_combined_insn)))
+ last_combined_insn = NEXT_INSN (last_combined_insn);
+ if (last_combined_insn
+ == NEXT_INSN (BB_END (this_basic_block)))
+ last_combined_insn = NULL_RTX;
+ }
+ if (last_combined_insn
+ && DF_INSN_LUID (last_combined_insn)
+ <= DF_INSN_LUID (insn))
+ last_combined_insn = insn;
+ }
/* See if we know about function return values before this
insn based upon SUBREG flags. */
@@ -2451,14 +2468,14 @@ propagate_for_debug_subst (rtx from, con
static void
propagate_for_debug (rtx insn, rtx last, rtx dest, rtx src)
{
- rtx next, loc;
+ rtx next, loc, end = NEXT_INSN (BB_END (this_basic_block));
struct rtx_subst_pair p;
p.to = src;
p.adjusted = false;
next = NEXT_INSN (insn);
- while (next != last)
+ while (next != last && next != end)
{
insn = next;
next = NEXT_INSN (insn);
@@ -3904,8 +3921,9 @@ try_combine (rtx i3, rtx i2, rtx i1, rtx
first = i3;
last = undobuf.other_insn;
gcc_assert (last);
- if (DF_INSN_LUID (last)
- < DF_INSN_LUID (last_combined_insn))
+ if (last_combined_insn == NULL_RTX
+ || (DF_INSN_LUID (last)
+ < DF_INSN_LUID (last_combined_insn)))
last = last_combined_insn;
}
--- gcc/testsuite/g++.dg/opt/pr48549.C.jj 2011-04-11 08:35:03.000000000 +0200
+++ gcc/testsuite/g++.dg/opt/pr48549.C 2011-04-11 08:34:19.000000000 +0200
@@ -0,0 +1,63 @@
+// PR rtl-optimization/48549
+// { dg-do compile }
+// { dg-options "-fcompare-debug -O2" }
+
+void
+foo (void *from, void *to)
+{
+ long offset = reinterpret_cast <long>(to) - reinterpret_cast <long>(from);
+ if (offset != static_cast <int>(offset))
+ *(int *) 0xC0DE = 0;
+ reinterpret_cast <int *>(from)[1] = offset;
+}
+struct A
+{
+ A () : a () {}
+ A (void *x) : a (x) {}
+ void *bar () { return a; }
+ void *a;
+};
+struct C;
+struct D;
+struct E : public A
+{
+ C m1 (int);
+ D m2 ();
+ E () {}
+ E (A x) : A (x) {}
+};
+struct C : public E
+{
+ C () {}
+ C (void *x) : E (x) {}
+};
+struct D : public E
+{
+ D (void *x) : E (x) {}
+};
+C
+E::m1 (int x)
+{
+ return (reinterpret_cast <char *>(bar ()) + x);
+}
+D
+E::m2 ()
+{
+ return reinterpret_cast <char *>(bar ());
+}
+struct B
+{
+ E a;
+ unsigned b : 16;
+ unsigned c : 1;
+};
+void
+baz (B *x)
+{
+ for (unsigned i = 0; i < 64; i++)
+ {
+ D d = x[i].a.m2 ();
+ C c = x[i].a.m1 (x[i].c);
+ foo (d.bar (), c.bar ());
+ }
+}
Jakub
[-- Attachment #2: 1 --]
[-- Type: text/plain, Size: 915 bytes --]
2011-04-11 Jakub Jelinek <jakub@redhat.com>
* combine.c (try_combine): Don't call update_cfg_for_uncondjump
for noop non-jump moves.
--- gcc/combine.c.jj 2011-04-09 19:29:19.083421147 +0200
+++ gcc/combine.c 2011-04-11 10:27:41.769671151 +0200
@@ -4402,7 +4421,8 @@ try_combine (rtx i3, rtx i2, rtx i1, rtx
/* A noop might also need cleaning up of CFG, if it comes from the
simplification of a jump. */
- if (GET_CODE (newpat) == SET
+ if (JUMP_P (i3)
+ && GET_CODE (newpat) == SET
&& SET_SRC (newpat) == pc_rtx
&& SET_DEST (newpat) == pc_rtx)
{
@@ -4411,6 +4431,7 @@ try_combine (rtx i3, rtx i2, rtx i1, rtx
}
if (undobuf.other_insn != NULL_RTX
+ && JUMP_P (undobuf.other_insn)
&& GET_CODE (PATTERN (undobuf.other_insn)) == SET
&& SET_SRC (PATTERN (undobuf.other_insn)) == pc_rtx
&& SET_DEST (PATTERN (undobuf.other_insn)) == pc_rtx)
next reply other threads:[~2011-04-11 10:50 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-04-11 10:50 Jakub Jelinek [this message]
2011-04-12 7:23 ` Eric Botcazou
2011-04-12 8:07 ` Jakub Jelinek
2011-04-12 8:35 ` Eric Botcazou
2011-04-12 10:55 ` Jakub Jelinek
2011-04-12 17:09 ` [PATCH] Don't update_cfg_for_uncondjump for noop non-jump moves Jakub Jelinek
2011-04-12 17:12 ` Jeff Law
2011-04-12 17:31 ` Eric Botcazou
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=20110411104958.GY17079@tyan-ft48-01.lab.bos.redhat.com \
--to=jakub@redhat.com \
--cc=ebotcazou@adacore.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).