From patchwork Mon Apr 11 10:49:58 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 90585 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 84202B6F10 for ; Mon, 11 Apr 2011 20:50:36 +1000 (EST) Received: (qmail 20931 invoked by alias); 11 Apr 2011 10:50:34 -0000 Received: (qmail 20922 invoked by uid 22791); 11 Apr 2011 10:50:33 -0000 X-SWARE-Spam-Status: No, hits=-6.4 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_HI, SPF_HELO_PASS, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 11 Apr 2011 10:50:23 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p3BAo0Mx006434 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 11 Apr 2011 06:50:00 -0400 Received: from tyan-ft48-01.lab.bos.redhat.com (tyan-ft48-01.lab.bos.redhat.com [10.16.42.4]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p3BAnxIf022075 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 11 Apr 2011 06:50:00 -0400 Received: from tyan-ft48-01.lab.bos.redhat.com (localhost.localdomain [127.0.0.1]) by tyan-ft48-01.lab.bos.redhat.com (8.14.4/8.14.4) with ESMTP id p3BAnxoW024429; Mon, 11 Apr 2011 12:49:59 +0200 Received: (from jakub@localhost) by tyan-ft48-01.lab.bos.redhat.com (8.14.4/8.14.4/Submit) id p3BAnw29024428; Mon, 11 Apr 2011 12:49:58 +0200 Date: Mon, 11 Apr 2011 12:49:58 +0200 From: Jakub Jelinek To: Eric Botcazou Cc: gcc-patches@gcc.gnu.org Subject: [PATCH] Fix combiner ICEs after my recent patch (PR rtl-optimization/48549) Message-ID: <20110411104958.GY17079@tyan-ft48-01.lab.bos.redhat.com> Reply-To: Jakub Jelinek MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org 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 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. Jakub 2011-04-11 Jakub Jelinek * 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) --- 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 (to) - reinterpret_cast (from); + if (offset != static_cast (offset)) + *(int *) 0xC0DE = 0; + reinterpret_cast (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 (bar ()) + x); +} +D +E::m2 () +{ + return reinterpret_cast (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 ()); + } +}