[Bug middle-end/68870] [6 Regression] ICE on valid code at -O1, -O2 and -O3 on x86_64-linux-gnu

[rguenth at gcc dot gnu.org]

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

[rguenth at gcc dot gnu.org]

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=gcc=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

[rguenth at gcc dot gnu.org]

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, , 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, , 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

[asolokha at gmx dot com]

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

[law at redhat dot com]

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

[rguenth at gcc dot gnu.org]

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, , 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, , 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, , 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

[rguenther at suse dot de]

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

[rguenth at gcc dot gnu.org]

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, , 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

[jakub at gcc dot gnu.org]

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

[law at redhat dot com]

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

[jakub at gcc dot gnu.org]

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.