From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4182 invoked by alias); 26 Mar 2011 13:04:11 -0000 Received: (qmail 4170 invoked by uid 22791); 26 Mar 2011 13:04:09 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,TW_CF X-Spam-Check-By: sourceware.org Received: from mel.act-europe.fr (HELO mel.act-europe.fr) (194.98.77.210) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 26 Mar 2011 13:04:04 +0000 Received: from localhost (localhost [127.0.0.1]) by filtered-smtp.eu.adacore.com (Postfix) with ESMTP id 83FAD2900C4 for ; Sat, 26 Mar 2011 14:04:02 +0100 (CET) Received: from mel.act-europe.fr ([127.0.0.1]) by localhost (smtp.eu.adacore.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id TW7lO6yk2ucs for ; Sat, 26 Mar 2011 14:03:59 +0100 (CET) Received: from [192.168.1.2] (bon31-9-83-155-120-49.fbx.proxad.net [83.155.120.49]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mel.act-europe.fr (Postfix) with ESMTP id 63DD02900C1 for ; Sat, 26 Mar 2011 14:03:59 +0100 (CET) From: Eric Botcazou To: gcc-patches@gcc.gnu.org Subject: Fix nasty bug in reg-stack.c Date: Sat, 26 Mar 2011 14:56:00 -0000 User-Agent: KMail/1.9.9 MIME-Version: 1.0 Content-Type: Multipart/Mixed; boundary="Boundary-00=_tMejNHIq+bIS8Yk" Message-Id: <201103261359.25674.ebotcazou@adacore.com> Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2011-03/txt/msg01824.txt.bz2 --Boundary-00=_tMejNHIq+bIS8Yk Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline Content-length: 1955 The bug was introduced 6 years ago but its occurrences apparently are quite rare since we detected it only very recently in our 4.3-based Ada compiler. The end of convert_regs reads: inserted |= compensate_edges (); clear_aux_for_blocks (); fixup_abnormal_edges (); if (inserted) commit_edge_insertions (); There is a nasty ordering problem in these lines. fixup_abnormal_edges is intended to fix up the CFG by moving insns that were inserted past the last insn in a BB before compensate_edges is run onto the fallthrough edge. And compensate_edges also inserts insns onto (fallthrough) edges. Now inserting insns on edges is a FIFO process so, if an insn was inserted past the last insn in a BB and a compensation insn is inserted afterward on the fallthrough edge, then they end up being swapped after the call to fixup_abnormal_edges. A possible fix would be to use in the earlier phases the same mechanism used in compensate_edges: inserting insns on edges directly (except for the case of a single successor). This seems inappropriate at this point of the lifetime of the reg-stack.c code. The attached fix moves the call to fixup_abnormal_edges up to before the call to compensate_edges instead; this in turn requires some changes to fixup_abnormal_edges itself, mostly the uncoupling of the fixup and the finalization codes. Tested on i586-suse-linux, applied on the mainline. 2011-03-26 Eric Botcazou * basic-block.h (fixup_abnormal_edges): Adjust prototype. * reload1.c (reload): Adjust call to fixup_abnormal_edges. Rediscover basic blocks and call commit_edge_insertions directly. (fixup_abnormal_edges): Move from here to... * cfgrtl.c (fixup_abnormal_edges): ...here. Only insert instructions on the edges and return whether some have actually been inserted. * reg-stack.c (convert_regs): Fix up abnormal edges before inserting compensation code. -- Eric Botcazou --Boundary-00=_tMejNHIq+bIS8Yk Content-Type: text/x-diff; charset="iso 8859-15"; name="p.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="p.diff" Content-length: 8945 Index: basic-block.h =================================================================== --- basic-block.h (revision 171404) +++ basic-block.h (working copy) @@ -798,6 +798,7 @@ extern basic_block force_nonfallthru (ed extern rtx block_label (basic_block); extern bool purge_all_dead_edges (void); extern bool purge_dead_edges (basic_block); +extern bool fixup_abnormal_edges (void); /* In cfgbuild.c. */ extern void find_many_sub_basic_blocks (sbitmap); @@ -814,7 +815,6 @@ extern bool delete_unreachable_blocks (v extern bool mark_dfs_back_edges (void); extern void set_edge_can_fallthru_flag (void); extern void update_br_prob_note (basic_block); -extern void fixup_abnormal_edges (void); extern bool inside_basic_block_p (const_rtx); extern bool control_flow_insn_p (const_rtx); extern rtx get_last_bb_insn (basic_block); Index: reload1.c =================================================================== --- reload1.c (revision 171404) +++ reload1.c (working copy) @@ -721,6 +721,7 @@ reload (rtx first, int global) rtx insn; struct elim_table *ep; basic_block bb; + bool inserted; /* Make sure even insns with volatile mem refs are recognizable. */ init_recog (); @@ -1299,7 +1300,21 @@ reload (rtx first, int global) /* Free all the insn_chain structures at once. */ obstack_free (&reload_obstack, reload_startobj); unused_insn_chains = 0; - fixup_abnormal_edges (); + + inserted = fixup_abnormal_edges (); + + /* We've possibly turned single trapping insn into multiple ones. */ + if (cfun->can_throw_non_call_exceptions) + { + sbitmap blocks; + blocks = sbitmap_alloc (last_basic_block); + sbitmap_ones (blocks); + find_many_sub_basic_blocks (blocks); + sbitmap_free (blocks); + } + + if (inserted) + commit_edge_insertions (); /* Replacing pseudos with their memory equivalents might have created shared rtx. Subsequent passes would get confused @@ -9112,112 +9127,3 @@ add_auto_inc_notes (rtx insn, rtx x) } } #endif - -/* This is used by reload pass, that does emit some instructions after - abnormal calls moving basic block end, but in fact it wants to emit - them on the edge. Looks for abnormal call edges, find backward the - proper call and fix the damage. - - Similar handle instructions throwing exceptions internally. */ -void -fixup_abnormal_edges (void) -{ - bool inserted = false; - basic_block bb; - - FOR_EACH_BB (bb) - { - edge e; - edge_iterator ei; - - /* Look for cases we are interested in - calls or instructions causing - exceptions. */ - FOR_EACH_EDGE (e, ei, bb->succs) - { - if (e->flags & EDGE_ABNORMAL_CALL) - break; - if ((e->flags & (EDGE_ABNORMAL | EDGE_EH)) - == (EDGE_ABNORMAL | EDGE_EH)) - break; - } - if (e && !CALL_P (BB_END (bb)) - && !can_throw_internal (BB_END (bb))) - { - rtx insn; - - /* Get past the new insns generated. Allow notes, as the insns - may be already deleted. */ - insn = BB_END (bb); - while ((NONJUMP_INSN_P (insn) || NOTE_P (insn)) - && !can_throw_internal (insn) - && insn != BB_HEAD (bb)) - insn = PREV_INSN (insn); - - if (CALL_P (insn) || can_throw_internal (insn)) - { - rtx stop, next; - - stop = NEXT_INSN (BB_END (bb)); - BB_END (bb) = insn; - insn = NEXT_INSN (insn); - - e = find_fallthru_edge (bb->succs); - - while (insn && insn != stop) - { - next = NEXT_INSN (insn); - if (INSN_P (insn)) - { - delete_insn (insn); - - /* Sometimes there's still the return value USE. - If it's placed after a trapping call (i.e. that - call is the last insn anyway), we have no fallthru - edge. Simply delete this use and don't try to insert - on the non-existent edge. */ - if (GET_CODE (PATTERN (insn)) != USE) - { - /* We're not deleting it, we're moving it. */ - INSN_DELETED_P (insn) = 0; - PREV_INSN (insn) = NULL_RTX; - NEXT_INSN (insn) = NULL_RTX; - - insert_insn_on_edge (insn, e); - inserted = true; - } - } - else if (!BARRIER_P (insn)) - set_block_for_insn (insn, NULL); - insn = next; - } - } - - /* It may be that we don't find any such trapping insn. In this - case we discovered quite late that the insn that had been - marked as can_throw_internal in fact couldn't trap at all. - So we should in fact delete the EH edges out of the block. */ - else - purge_dead_edges (bb); - } - } - - /* We've possibly turned single trapping insn into multiple ones. */ - if (cfun->can_throw_non_call_exceptions) - { - sbitmap blocks; - blocks = sbitmap_alloc (last_basic_block); - sbitmap_ones (blocks); - find_many_sub_basic_blocks (blocks); - sbitmap_free (blocks); - } - - if (inserted) - commit_edge_insertions (); - -#ifdef ENABLE_CHECKING - /* Verify that we didn't turn one trapping insn into many, and that - we found and corrected all of the problems wrt fixups on the - fallthru edge. */ - verify_flow_info (); -#endif -} Index: cfgrtl.c =================================================================== --- cfgrtl.c (revision 171404) +++ cfgrtl.c (working copy) @@ -30,6 +30,8 @@ along with GCC; see the file COPYING3. insert_insn_on_edge, commit_edge_insertions - CFG updating after insn simplification purge_dead_edges, purge_all_dead_edges + - CFG fixing after coarse manipulation + fixup_abnormal_edges Functions not supposed for generic use: - Infrastructure to determine quickly basic block for insn @@ -2471,6 +2473,95 @@ purge_all_dead_edges (void) return purged; } +/* This is used by a few passes that emit some instructions after abnormal + calls, moving the basic block's end, while they in fact do want to emit + them on the fallthru edge. Look for abnormal call edges, find backward + the call in the block and insert the instructions on the edge instead. + + Similarly, handle instructions throwing exceptions internally. + + Return true when instructions have been found and inserted on edges. */ + +bool +fixup_abnormal_edges (void) +{ + bool inserted = false; + basic_block bb; + + FOR_EACH_BB (bb) + { + edge e; + edge_iterator ei; + + /* Look for cases we are interested in - calls or instructions causing + exceptions. */ + FOR_EACH_EDGE (e, ei, bb->succs) + if ((e->flags & EDGE_ABNORMAL_CALL) + || ((e->flags & (EDGE_ABNORMAL | EDGE_EH)) + == (EDGE_ABNORMAL | EDGE_EH))) + break; + + if (e && !CALL_P (BB_END (bb)) && !can_throw_internal (BB_END (bb))) + { + rtx insn; + + /* Get past the new insns generated. Allow notes, as the insns + may be already deleted. */ + insn = BB_END (bb); + while ((NONJUMP_INSN_P (insn) || NOTE_P (insn)) + && !can_throw_internal (insn) + && insn != BB_HEAD (bb)) + insn = PREV_INSN (insn); + + if (CALL_P (insn) || can_throw_internal (insn)) + { + rtx stop, next; + + e = find_fallthru_edge (bb->succs); + + stop = NEXT_INSN (BB_END (bb)); + BB_END (bb) = insn; + + for (insn = NEXT_INSN (insn); insn != stop; insn = next) + { + next = NEXT_INSN (insn); + if (INSN_P (insn)) + { + delete_insn (insn); + + /* Sometimes there's still the return value USE. + If it's placed after a trapping call (i.e. that + call is the last insn anyway), we have no fallthru + edge. Simply delete this use and don't try to insert + on the non-existent edge. */ + if (GET_CODE (PATTERN (insn)) != USE) + { + /* We're not deleting it, we're moving it. */ + INSN_DELETED_P (insn) = 0; + PREV_INSN (insn) = NULL_RTX; + NEXT_INSN (insn) = NULL_RTX; + + insert_insn_on_edge (insn, e); + inserted = true; + } + } + else if (!BARRIER_P (insn)) + set_block_for_insn (insn, NULL); + } + } + + /* It may be that we don't find any trapping insn. In this + case we discovered quite late that the insn that had been + marked as can_throw_internal in fact couldn't trap at all. + So we should in fact delete the EH edges out of the block. */ + else + purge_dead_edges (bb); + } + } + + return inserted; +} + /* Same as split_block but update cfg_layout structures. */ static basic_block Index: reg-stack.c =================================================================== --- reg-stack.c (revision 171404) +++ reg-stack.c (working copy) @@ -3150,11 +3150,14 @@ convert_regs (void) cfg_altered |= convert_regs_2 (b); } + /* We must fix up abnormal edges before inserting compensation code + because both mechanisms insert insns on edges. */ + inserted |= fixup_abnormal_edges (); + inserted |= compensate_edges (); clear_aux_for_blocks (); - fixup_abnormal_edges (); if (inserted) commit_edge_insertions (); --Boundary-00=_tMejNHIq+bIS8Yk--