From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5569 invoked by alias); 20 Oct 2011 13:49:28 -0000 Received: (qmail 5547 invoked by uid 22791); 20 Oct 2011 13:49:25 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL,BAYES_00,TW_TM X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 20 Oct 2011 13:49:00 +0000 Received: from svr-orw-fem-01.mgc.mentorg.com ([147.34.98.93]) by relay1.mentorg.com with esmtp id 1RGszT-0005GZ-G7 from Tom_deVries@mentor.com ; Thu, 20 Oct 2011 06:48:59 -0700 Received: from SVR-IES-FEM-01.mgc.mentorg.com ([137.202.0.104]) by svr-orw-fem-01.mgc.mentorg.com over TLS secured channel with Microsoft SMTPSVC(6.0.3790.4675); Thu, 20 Oct 2011 06:48:59 -0700 Received: from [127.0.0.1] (137.202.0.76) by SVR-IES-FEM-01.mgc.mentorg.com (137.202.0.104) with Microsoft SMTP Server id 14.1.289.1; Thu, 20 Oct 2011 14:48:56 +0100 Message-ID: <4EA026CB.2060309@mentor.com> Date: Thu, 20 Oct 2011 14:45:00 -0000 From: Tom de Vries User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.23) Gecko/20110922 Lightning/1.0b2 Thunderbird/3.1.15 MIME-Version: 1.0 To: Richard Guenther , "gcc-patches@gcc.gnu.org" Subject: [PATCH, PR50763] Fix for ICE in verify_gimple Content-Type: multipart/mixed; boundary="------------020003040505040507080900" 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-10/txt/msg01872.txt.bz2 --------------020003040505040507080900 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Content-length: 4462 Richard, I have a fix for PR50763. The second example from the PR looks like this: ... int bar (int i); void foo (int c, int d) { if (bar (c)) bar (c); d = 33; while (c == d); } ... When compiled with -O2 -fno-dominator-opt, the gimple representation before ftree-tail-merge looks like this: ... foo (intD.6 cD.1606, intD.6 dD.1607) { intD.6 D.2730; # BLOCK 2 freq:900 # PRED: ENTRY [100.0%] (fallthru,exec) # .MEMD.2733_6 = VDEF <.MEMD.2733_5(D)> # USE = nonlocal # CLB = nonlocal D.2730_2 = barD.1605 (cD.1606_1(D)); if (D.2730_2 != 0) goto ; else goto ; # SUCC: 3 [29.0%] (true,exec) 7 [71.0%] (false,exec) # BLOCK 7 freq:639 # PRED: 2 [71.0%] (false,exec) goto ; # SUCC: 4 [100.0%] (fallthru) # BLOCK 3 freq:261 # PRED: 2 [29.0%] (true,exec) # .MEMD.2733_7 = VDEF <.MEMD.2733_6> # USE = nonlocal # CLB = nonlocal barD.1605 (cD.1606_1(D)); # SUCC: 4 [100.0%] (fallthru,exec) # BLOCK 4 freq:900 # PRED: 7 [100.0%] (fallthru) 3 [100.0%] (fallthru,exec) # .MEMD.2733_4 = PHI <.MEMD.2733_6(7), .MEMD.2733_7(3)> if (cD.1606_1(D) == 33) goto ; else goto ; # SUCC: 8 [91.0%] (true,exec) 9 [9.0%] (false,exec) # BLOCK 9 freq:81 # PRED: 4 [9.0%] (false,exec) goto ; # SUCC: 6 [100.0%] (fallthru) # BLOCK 8 freq:819 # PRED: 4 [91.0%] (true,exec) # SUCC: 5 [100.0%] (fallthru) # BLOCK 5 freq:9100 # PRED: 8 [100.0%] (fallthru) 10 [100.0%] (fallthru) if (cD.1606_1(D) == 33) goto ; else goto ; # SUCC: 10 [91.0%] (true,exec) 11 [9.0%] (false,exec) # BLOCK 10 freq:8281 # PRED: 5 [91.0%] (true,exec) goto ; # SUCC: 5 [100.0%] (fallthru) # BLOCK 11 freq:819 # PRED: 5 [9.0%] (false,exec) # SUCC: 6 [100.0%] (fallthru) # BLOCK 6 freq:900 # PRED: 11 [100.0%] (fallthru) 9 [100.0%] (fallthru) # VUSE <.MEMD.2733_4> return; # SUCC: EXIT [100.0%] } ... During the first iteration, tail_merge_optimize finds that block 9 and 11, and block 8 and 10 are equal, and removes block 11 and 10. During the second iteration it finds that block 4 and block 5 are equal, and it removes block 5. Since pre had no effect, the responsibility for updating the vops lies with tail_merge_optimize. Block 4 starts with a virtual PHI which needs updating, but replace_block_by decides that an update is not necessary, because vop_at_entry returns NULL_TREE for block 5 (the vop_at_entry for block 4 is .MEMD.2733_4). What is different from normal is that block 4 dominates block 5. The patch makes sure that the vops are also updated if vop_at_entry is defined for only one of bb1 and bb2. This also forced me to rewrite the code that updates the uses, which uses dominator info now. This forced me to keep the dominator info up-to-date. Which forced me to move the actual deletion of the basic block and some additional bookkeeping related to that from purge_bbs to replace_block_by. Additionally, I fixed the case that update_vuses leaves virtual phis with only one argument (see unlink_virtual_phi). bootstrapped and reg-tested on x86_64. The tested patch had one addition to the attached patch: calling verify_dominators at the end of replace_block_by. OK for trunk? Thanks, - Tom 2011-10-20 Tom de Vries PR tree-optimization/50763 * tree-ssa-tail-merge.c (same_succ_flush_bb): New function, factored out of ... (same_succ_flush_bbs): Use same_succ_flush_bb. (purge_bbs): Remove argument. Remove calls to same_succ_flush_bbs, release_last_vdef and delete_basic_block. (unlink_virtual_phi): New function. (update_vuses): Add and use vuse1_phi_args argument. Set var to SSA_NAME_VAR of vuse1 or vuse2, and use var. Handle case that def_stmt2 is NULL. Use phi result as phi arg in case vuse1 or vuse2 is NULL_TREE. Replace uses of vuse1 if vuse2 is NULL_TREE. Fix code to limit replacement of uses. Propagate phi argument for phis with a single argument. (replace_block_by): Update vops if phi_vuse1 or phi_vuse2 is NULL_TREE. Set vuse1_phi_args if vuse1 is a phi defined in bb1. Add vuse1_phi_args as argument to call to update_vuses. Call release_last_vdef, same_succ_flush_bb, delete_basic_block. Update CDI_DOMINATORS info. (tail_merge_optimize): Remove argument in call to purge_bbs. Remove call to free_dominance_info. Only call calculate_dominance_info once. * gcc.dg/pr50763.c: New test. --------------020003040505040507080900 Content-Type: text/x-patch; name="pr50763.7.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="pr50763.7.patch" Content-length: 8856 Index: gcc/tree-ssa-tail-merge.c =================================================================== --- gcc/tree-ssa-tail-merge.c (revision 180237) +++ gcc/tree-ssa-tail-merge.c (working copy) @@ -753,6 +753,19 @@ delete_basic_block_same_succ (basic_bloc bitmap_set_bit (deleted_bb_preds, e->src->index); } +/* Removes BB from its corresponding same_succ. */ + +static void +same_succ_flush_bb (basic_block bb) +{ + same_succ same = BB_SAME_SUCC (bb); + BB_SAME_SUCC (bb) = NULL; + if (bitmap_single_bit_set_p (same->bbs)) + htab_remove_elt_with_hash (same_succ_htab, same, same->hashval); + else + bitmap_clear_bit (same->bbs, bb->index); +} + /* Removes all bbs in BBS from their corresponding same_succ. */ static void @@ -762,15 +775,7 @@ same_succ_flush_bbs (bitmap bbs) bitmap_iterator bi; EXECUTE_IF_SET_IN_BITMAP (bbs, 0, i, bi) - { - basic_block bb = BASIC_BLOCK (i); - same_succ same = BB_SAME_SUCC (bb); - BB_SAME_SUCC (bb) = NULL; - if (bitmap_single_bit_set_p (same->bbs)) - htab_remove_elt_with_hash (same_succ_htab, same, same->hashval); - else - bitmap_clear_bit (same->bbs, i); - } + same_succ_flush_bb (BASIC_BLOCK (i)); } /* Release the last vdef in BB, either normal or phi result. */ @@ -807,23 +812,8 @@ release_last_vdef (basic_block bb) /* Delete all deleted_bbs. */ static void -purge_bbs (bool update_vops) +purge_bbs (void) { - unsigned int i; - bitmap_iterator bi; - basic_block bb; - - same_succ_flush_bbs (deleted_bbs); - - EXECUTE_IF_SET_IN_BITMAP (deleted_bbs, 0, i, bi) - { - bb = BASIC_BLOCK (i); - if (!update_vops) - release_last_vdef (bb); - - delete_basic_block (bb); - } - bitmap_and_compl_into (deleted_bb_preds, deleted_bbs); bitmap_clear (deleted_bbs); } @@ -1363,26 +1353,54 @@ find_clusters (void) } } +static void +unlink_virtual_phi (gimple phi, tree name) +{ + use_operand_p use_p; + imm_use_iterator iter; + gimple use_stmt; + tree vdef = gimple_phi_result (phi); + + if (!vdef + || TREE_CODE (vdef) != SSA_NAME) + return; + + FOR_EACH_IMM_USE_STMT (use_stmt, iter, vdef) + { + FOR_EACH_IMM_USE_ON_STMT (use_p, iter) + SET_USE (use_p, name); + } + + if (SSA_NAME_OCCURS_IN_ABNORMAL_PHI (vdef)) + SSA_NAME_OCCURS_IN_ABNORMAL_PHI (name) = 1; +} + /* Create or update a vop phi in BB2. Use VUSE1 arguments for all the REDIRECTED_EDGES, or if VUSE1 is NULL_TREE, use BB_VOP_AT_EXIT. If a new phis is created, use the phi instead of VUSE2 in BB2. */ static void -update_vuses (tree vuse1, tree vuse2, basic_block bb2, +update_vuses (bool vuse1_phi_args, tree vuse1, tree vuse2, basic_block bb2, VEC (edge,heap) *redirected_edges) { gimple stmt, phi = NULL; - tree lhs = NULL_TREE, arg; + tree lhs = NULL_TREE, arg, var; unsigned int i; - gimple def_stmt2; + gimple def_stmt2 = NULL; imm_use_iterator iter; use_operand_p use_p; edge_iterator ei; edge e; - def_stmt2 = SSA_NAME_DEF_STMT (vuse2); + if (vuse2 != NULL_TREE) + { + var = SSA_NAME_VAR (vuse2); + def_stmt2 = SSA_NAME_DEF_STMT (vuse2); + } + else + var = SSA_NAME_VAR (vuse1); - if (gimple_bb (def_stmt2) == bb2) + if (def_stmt2 && gimple_bb (def_stmt2) == bb2) /* Update existing phi. */ phi = def_stmt2; else @@ -1392,38 +1410,41 @@ update_vuses (tree vuse1, tree vuse2, ba return; /* Create a phi. */ - lhs = make_ssa_name (SSA_NAME_VAR (vuse2), NULL); + lhs = make_ssa_name (var, NULL); VN_INFO_GET (lhs); phi = create_phi_node (lhs, bb2); SSA_NAME_DEF_STMT (lhs) = phi; /* Set default argument vuse2 for all preds. */ + arg = vuse2 == NULL_TREE ? gimple_phi_result (phi): vuse2; FOR_EACH_EDGE (e, ei, bb2->preds) - add_phi_arg (phi, vuse2, e, UNKNOWN_LOCATION); + add_phi_arg (phi, arg, e, UNKNOWN_LOCATION); } /* Update phi. */ for (i = 0; i < EDGE_COUNT (redirected_edges); ++i) { e = VEC_index (edge, redirected_edges, i); - if (vuse1 != NULL_TREE) - arg = vuse1; - else + if (vuse1_phi_args) arg = BB_VOP_AT_EXIT (e->src); + else + arg = vuse1 == NULL_TREE ? gimple_phi_result (phi): vuse1; + add_phi_arg (phi, arg, e, UNKNOWN_LOCATION); } /* Return if we updated an existing phi. */ - if (gimple_bb (def_stmt2) == bb2) + if (def_stmt2 && gimple_bb (def_stmt2) == bb2) return; - /* Replace relevant uses of vuse2 with the newly created phi. */ - FOR_EACH_IMM_USE_STMT (stmt, iter, vuse2) + /* Replace relevant uses with the newly created phi. */ + FOR_EACH_IMM_USE_STMT (stmt, iter, vuse2 == NULL_TREE ? vuse1 : vuse2) { if (stmt == phi) continue; - if (gimple_code (stmt) != GIMPLE_PHI) - if (gimple_bb (stmt) != bb2) + + if (gimple_code (stmt) != GIMPLE_PHI && + !dominated_by_p (CDI_DOMINATORS, gimple_bb (stmt), bb2)) continue; FOR_EACH_IMM_USE_ON_STMT (use_p, iter) @@ -1432,8 +1453,16 @@ update_vuses (tree vuse1, tree vuse2, ba { unsigned int pred_index = PHI_ARG_INDEX_FROM_USE (use_p); basic_block pred = EDGE_PRED (gimple_bb (stmt), pred_index)->src; - if (pred != bb2) + if (!dominated_by_p (CDI_DOMINATORS, pred, bb2)) continue; + + if (pred == bb2 && EDGE_COUNT (gimple_bb (stmt)->preds) == 2) + { + gimple_stmt_iterator gsi = gsi_for_stmt (stmt); + unlink_virtual_phi (stmt, lhs); + remove_phi_node (&gsi, true); + break; + } } SET_USE (use_p, lhs); update_stmt (stmt); @@ -1507,6 +1536,8 @@ replace_block_by (basic_block bb1, basic VEC (edge,heap) *redirected_edges = NULL; edge e; edge_iterator ei; + bool vuse1_phi_args = false; + VEC (basic_block,heap) *fix_dom_bb; phi_vuse2 = vop_at_entry (bb2); if (phi_vuse2 != NULL_TREE && TREE_CODE (phi_vuse2) != SSA_NAME) @@ -1517,11 +1548,11 @@ replace_block_by (basic_block bb1, basic /* Find the vops at entry of bb1 and bb2. */ phi_vuse1 = vop_at_entry (bb1); - /* If one of the 2 not found, it means there's no need to update. */ - update_vops = phi_vuse1 != NULL_TREE && phi_vuse2 != NULL_TREE; + /* If both are not found, it means there's no need to update. */ + update_vops = phi_vuse1 != NULL_TREE || phi_vuse2 != NULL_TREE; } - if (update_vops && gimple_bb (SSA_NAME_DEF_STMT (phi_vuse1)) == bb1) + if (phi_vuse1 && gimple_bb (SSA_NAME_DEF_STMT (phi_vuse1)) == bb1) { /* If the vop at entry of bb1 is a phi, save the phi alternatives in BB_VOP_AT_EXIT, before we lose that information by redirecting the @@ -1531,7 +1562,7 @@ replace_block_by (basic_block bb1, basic arg = PHI_ARG_DEF_FROM_EDGE (SSA_NAME_DEF_STMT (phi_vuse1), e); BB_VOP_AT_EXIT (e->src) = arg; } - phi_vuse1 = NULL; + vuse1_phi_args = true; } /* Mark the basic block for later deletion. */ @@ -1556,9 +1587,22 @@ replace_block_by (basic_block bb1, basic /* Update the vops. */ if (update_vops) { - update_vuses (phi_vuse1, phi_vuse2, bb2, redirected_edges); + update_vuses (vuse1_phi_args, phi_vuse1, phi_vuse2, bb2, + redirected_edges); VEC_free (edge, heap, redirected_edges); } + else + release_last_vdef (bb1); + + same_succ_flush_bb (bb1); + delete_basic_block (bb1); + + fix_dom_bb = VEC_alloc (basic_block, heap, 2); + VEC_safe_push (basic_block, heap, fix_dom_bb, bb2); + FOR_EACH_EDGE (e, ei, bb2->succs) + VEC_safe_push (basic_block, heap, fix_dom_bb, e->dest); + iterate_fix_dominators (CDI_DOMINATORS, fix_dom_bb, false); + VEC_free (basic_block, heap, fix_dom_bb); } /* Bbs for which update_debug_stmt need to be called. */ @@ -1708,13 +1752,11 @@ tail_merge_optimize (unsigned int todo) if (nr_bbs_removed == 0) break; - free_dominance_info (CDI_DOMINATORS); - purge_bbs (update_vops); + purge_bbs (); if (iteration_nr == max_iterations) break; - calculate_dominance_info (CDI_DOMINATORS); update_worklist (); } @@ -1724,7 +1766,6 @@ tail_merge_optimize (unsigned int todo) if (nr_bbs_removed_total > 0) { - calculate_dominance_info (CDI_DOMINATORS); update_debug_stmts (); if (dump_file && (dump_flags & TDF_DETAILS)) Index: gcc/testsuite/gcc.dg/pr50763.c =================================================================== --- /dev/null (new file) +++ gcc/testsuite/gcc.dg/pr50763.c (revision 0) @@ -0,0 +1,16 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fno-tree-dominator-opts -fdump-tree-pre" } */ + +int bar (int i); + +void +foo (int c, int d) +{ + if (bar (c)) + bar (c); + d = 33; + while (c == d); +} + +/* { dg-final { scan-tree-dump-times "== 33" 1 "pre"} } */ +/* { dg-final { cleanup-tree-dump "pre" } } */ --------------020003040505040507080900--