[Bug middle-end/68870] [6 Regression] ICE on valid code at -O1, -O2 and -O3 on x86_64-linux-gnu
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68870 Richard Biener changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #11 from Richard Biener --- Fixed.
[Bug middle-end/68870] [6 Regression] ICE on valid code at -O1, -O2 and -O3 on x86_64-linux-gnu
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68870 --- Comment #10 from Richard Biener --- Author: rguenth Date: Wed Dec 16 14:56:10 2015 New Revision: 231695 URL: https://gcc.gnu.org/viewcvs?rev=231695&root=gcc&view=rev Log: 2015-12-16 Richard Biener PR tree-optimization/68870 * tree-cfgcleanup.c (cleanup_control_expr_graph): Add first_p parameter, if set only perform trivial constant folding. Queue other blocks with conditions for later processing. (cleanup_control_flow_bb): Add first_p parameter and pass it through. (cleanup_tree_cfg_1): Pass true for the first iteration cleanup_control_expr_graph. * gcc.dg/torture/pr68870.c: New testcase. Added: trunk/gcc/testsuite/gcc.dg/torture/pr68870.c Modified: trunk/gcc/ChangeLog trunk/gcc/testsuite/ChangeLog trunk/gcc/tree-cfgcleanup.c
[Bug middle-end/68870] [6 Regression] ICE on valid code at -O1, -O2 and -O3 on x86_64-linux-gnu
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68870 Richard Biener changed: What|Removed |Added Status|NEW |ASSIGNED Assignee|unassigned at gcc dot gnu.org |rguenth at gcc dot gnu.org --- Comment #9 from Richard Biener --- So to complete my earlier fix we need to do sth like Index: gcc/tree-cfgcleanup.c === --- gcc/tree-cfgcleanup.c (revision 231673) +++ gcc/tree-cfgcleanup.c (working copy) @@ -78,7 +78,8 @@ remove_fallthru_edge (vec * at block BB. */ static bool -cleanup_control_expr_graph (basic_block bb, gimple_stmt_iterator gsi) +cleanup_control_expr_graph (basic_block bb, gimple_stmt_iterator gsi, + bool first_p) { edge taken_edge; bool retval = false; @@ -95,15 +96,26 @@ cleanup_control_expr_graph (basic_block switch (gimple_code (stmt)) { case GIMPLE_COND: - { - code_helper rcode; - tree ops[3] = {}; - if (gimple_simplify (stmt, &rcode, ops, NULL, no_follow_ssa_edges, -no_follow_ssa_edges) - && rcode == INTEGER_CST) - val = ops[0]; - break; - } + /* During a first iteration on the CFG only remove trivially +dead edges but mark other conditions for re-evaluation. */ + if (first_p) + { + val = const_binop (gimple_cond_code (stmt), boolean_type_node, +gimple_cond_lhs (stmt), +gimple_cond_rhs (stmt)); + if (! val) + bitmap_set_bit (cfgcleanup_altered_bbs, bb->index); + } + else + { + code_helper rcode; + tree ops[3] = {}; + if (gimple_simplify (stmt, &rcode, ops, NULL, no_follow_ssa_edges, + no_follow_ssa_edges) + && rcode == INTEGER_CST) + val = ops[0]; + } + break; case GIMPLE_SWITCH: val = gimple_switch_index (as_a (stmt)); @@ -176,7 +188,7 @@ cleanup_call_ctrl_altering_flag (gimple true if anything changes. */ static bool -cleanup_control_flow_bb (basic_block bb) +cleanup_control_flow_bb (basic_block bb, bool first_p) { gimple_stmt_iterator gsi; bool retval = false; @@ -199,7 +211,7 @@ cleanup_control_flow_bb (basic_block bb) || gimple_code (stmt) == GIMPLE_SWITCH) { gcc_checking_assert (gsi_stmt (gsi_last_bb (bb)) == stmt); - retval |= cleanup_control_expr_graph (bb, gsi); + retval |= cleanup_control_expr_graph (bb, gsi, first_p); } else if (gimple_code (stmt) == GIMPLE_GOTO && TREE_CODE (gimple_goto_dest (stmt)) == ADDR_EXPR @@ -680,7 +692,7 @@ cleanup_tree_cfg_1 (void) { bb = BASIC_BLOCK_FOR_FN (cfun, i); if (bb) - retval |= cleanup_control_flow_bb (bb); + retval |= cleanup_control_flow_bb (bb, true); } /* After doing the above SSA form should be valid (or an update SSA @@ -708,7 +720,7 @@ cleanup_tree_cfg_1 (void) if (!bb) continue; - retval |= cleanup_control_flow_bb (bb); + retval |= cleanup_control_flow_bb (bb, false); retval |= cleanup_tree_cfg_bb (bb); }
[Bug middle-end/68870] [6 Regression] ICE on valid code at -O1, -O2 and -O3 on x86_64-linux-gnu
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68870 --- Comment #8 from Arseny Solokha --- I believe the following reproducer is for the issue reported here. Its further minimization yields backtrace listed in #c0. The only difference is that w/ the following not minimized snippet gcc ICEs in tree_nop_conversion_p(tree_node const*, tree_node const*) when compiling it at -O1: int w3, ao, k9, nl; static int oy(void) { static int pe; int ht; for (ao = 0; ao < 1; ++ao) for (ht = 0; ht < 1; ++ht) for (w3 = 0; w3 < 1; ++w3) for (k9 = 0; k9 < 1; ++k9) if (pe < 1) return 0; return (ht > 1) || nl; } int ct(void) { return oy(); }
[Bug middle-end/68870] [6 Regression] ICE on valid code at -O1, -O2 and -O3 on x86_64-linux-gnu
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68870 --- Comment #7 from rguenther at suse dot de --- On December 15, 2015 3:15:13 AM GMT+01:00, law at redhat dot com wrote: >https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68870 > >--- Comment #6 from Jeffrey A. Law --- >I'd say that changing genmatch (or any code) to know about the freelist >is a >horrid hack and just papering over the problem. > >I haven't put this case under a debugger, but presumably we've got a >reference >to a released SSA_NAME in the IL, right? We have invalid SSA, a use not dominated by its def. It happens to be in a trivially dead region but CFG cleanup deleting the def first which is in another trivially dead region and then folding the conditional is causing this. Note the conditional happens to be folded before it is deleted as part of the other trivial dead region. So it's the old CFG cleanup ordering issue I tried to fix with the last rev. But forgot that folding code... Presumably in some code that >isn't >trivially proved unreachable? It actually is.
[Bug middle-end/68870] [6 Regression] ICE on valid code at -O1, -O2 and -O3 on x86_64-linux-gnu
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68870 --- Comment #6 from Jeffrey A. Law --- I'd say that changing genmatch (or any code) to know about the freelist is a horrid hack and just papering over the problem. I haven't put this case under a debugger, but presumably we've got a reference to a released SSA_NAME in the IL, right? Presumably in some code that isn't trivially proved unreachable?
[Bug middle-end/68870] [6 Regression] ICE on valid code at -O1, -O2 and -O3 on x86_64-linux-gnu
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68870 --- Comment #5 from Jakub Jelinek --- Or change genmatch so that it stops looking if it sees SSA_NAMEs in free list.
[Bug middle-end/68870] [6 Regression] ICE on valid code at -O1, -O2 and -O3 on x86_64-linux-gnu
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68870 Jeffrey A. Law changed: What|Removed |Added CC||law at redhat dot com --- Comment #4 from Jeffrey A. Law --- A post-pass over the CFG doing trivial simplifications on the conditions wouldn't be terribly hard. It would have to limit itself to not looking at anything in the SSA graph since it's inconsistent until after update_ssa. My concern is that doing so just papers over the deeper problems we're having with the interactions with SSA_NAME recycling and the folders. Would it make sense to define objects that are on the pending free list as still being referencable by the IL? So they still have a valid type, def_stmt, etc. It feels like that's the direction we're already going anyway.
[Bug middle-end/68870] [6 Regression] ICE on valid code at -O1, -O2 and -O3 on x86_64-linux-gnu
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68870 --- Comment #3 from Richard Biener --- The right (TM) fix: Index: gcc/tree-cfgcleanup.c === --- gcc/tree-cfgcleanup.c (revision 231617) +++ gcc/tree-cfgcleanup.c (working copy) @@ -95,15 +95,9 @@ cleanup_control_expr_graph (basic_block switch (gimple_code (stmt)) { case GIMPLE_COND: - { - code_helper rcode; - tree ops[3] = {}; - if (gimple_simplify (stmt, &rcode, ops, NULL, no_follow_ssa_edges, -no_follow_ssa_edges) - && rcode == INTEGER_CST) - val = ops[0]; - break; - } + val = const_binop (gimple_cond_code (stmt), boolean_type_node, +gimple_cond_lhs (stmt), gimple_cond_rhs (stmt)); + break; case GIMPLE_SWITCH: val = gimple_switch_index (as_a (stmt)); and the following shows the regressions: Index: gcc/tree-cfgcleanup.c === --- gcc/tree-cfgcleanup.c (revision 231617) +++ gcc/tree-cfgcleanup.c (working copy) @@ -95,15 +95,18 @@ cleanup_control_expr_graph (basic_block switch (gimple_code (stmt)) { case GIMPLE_COND: - { - code_helper rcode; - tree ops[3] = {}; - if (gimple_simplify (stmt, &rcode, ops, NULL, no_follow_ssa_edges, -no_follow_ssa_edges) - && rcode == INTEGER_CST) - val = ops[0]; - break; - } + val = const_binop (gimple_cond_code (stmt), boolean_type_node, +gimple_cond_lhs (stmt), gimple_cond_rhs (stmt)); + if (!val) + { + code_helper rcode; + tree ops[3] = {}; + if (gimple_simplify (stmt, &rcode, ops, NULL, no_follow_ssa_edges, + no_follow_ssa_edges) + && rcode == INTEGER_CST) + gcc_unreachable (); + } + break; case GIMPLE_SWITCH: val = gimple_switch_index (as_a (stmt));
[Bug middle-end/68870] [6 Regression] ICE on valid code at -O1, -O2 and -O3 on x86_64-linux-gnu
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68870 Richard Biener changed: What|Removed |Added CC||law at gcc dot gnu.org, ||rguenth at gcc dot gnu.org --- Comment #2 from Richard Biener --- Similar reason as PR68625. So we need to go a step further only allowing "trivial" dead code and thus skip case GIMPLE_COND: { code_helper rcode; tree ops[3] = {}; if (gimple_simplify (stmt, &rcode, ops, NULL, no_follow_ssa_edges, no_follow_ssa_edges) && rcode == INTEGER_CST) val = ops[0]; Skip as in use const_binop (condition) [note that doing this change always will regress things] Hmm, but doing const_binop in the "first" iteration only makes the situation less likely in following ones (and we'd need to unconditonally iterate at least once). Conceptually doing only const_binob above would be the right thing to do (TM) but as said it regressed (just place an assert above). Note the above call doesn't need to follow SSA edges to hit the issue but h_1 in h_1 != 0 is already released (its def stmt is released and is not dominating this use as DOM figured out one edge was not executable) Note that DOM does Optimizing block #15 LKUP STMT .MEM_35 = PHI <.MEM_8, .MEM_19> 2>>> STMT .MEM_35 = PHI <.MEM_8, .MEM_19> STMT .MEM_35 = PHI <.MEM_8, .MEM_19> Optimizing statement a = 0; LKUP STMT a = 0 with .MEM_35 LKUP STMT 0 = a with .MEM_35 LKUP STMT 0 = a with .MEM_12 2>>> STMT 0 = a with .MEM_12 Optimizing statement a.8_30 = a; LKUP STMT a.8_30 = a with .MEM_12 FIND: 0 Replaced redundant expr 'a' with '0' ASGN a.8_30 = 0 Optimizing statement if (a.8_30 <= 0) Replaced 'a.8_30' with constant '0' gimple_simplified to if (1 != 0) Folded to: if (1 != 0) LKUP STMT 1 ne_expr 0 ... Optimizing block #10 so this is the usual dominator-iteration doesn't see #10 is unreachable because #9 is not yet visited. Eventually a pass over all BBs _after_ the domwalk and then forcefully "simplifying" conditions in unreachable regions might be a workaround. OTOH we really should work toward being able to do the right thing (TM) in cfg-cleanup...
[Bug middle-end/68870] [6 Regression] ICE on valid code at -O1, -O2 and -O3 on x86_64-linux-gnu
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68870 Jakub Jelinek changed: What|Removed |Added Status|UNCONFIRMED |NEW Last reconfirmed||2015-12-12 CC||jakub at gcc dot gnu.org Target Milestone|--- |6.0 Summary|ICE on valid code at -O1, |[6 Regression] ICE on valid |-O2 and -O3 on |code at -O1, -O2 and -O3 on |x86_64-linux-gnu|x86_64-linux-gnu Ever confirmed|0 |1 --- Comment #1 from Jakub Jelinek --- Started with r231527. Though it seems to be yet another case of gimple_simplify during cfg cleanup finding in-free-list SSA_NAMEs.