[Bug tree-optimization/84178] [7/8 Regression] ICE in release_bb_predicate

[asolokha at gmx dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84178

--- Comment #11 from Arseny Solokha  ---
(In reply to Arseny Solokha from comment #10)
> This PR apparently can be closed now?

Sorry, didn't realize it's pending for backport to the 7 branch.

[Bug tree-optimization/84178] [7/8 Regression] ICE in release_bb_predicate

[asolokha at gmx dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84178

--- Comment #10 from Arseny Solokha  ---
This PR apparently can be closed now?

[Bug tree-optimization/84178] [7/8 Regression] ICE in release_bb_predicate

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84178
Bug 84178 depends on bug 84775, which changed state.

Bug 84775 Summary: [8 Regression] ICE on valid code at -O3: in 
check_loop_closed_ssa_def, at tree-ssa-loop-manip.c:709
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84775

   What|Removed |Added

 Status|ASSIGNED|RESOLVED
 Resolution|--- |FIXED

[Bug tree-optimization/84178] [7/8 Regression] ICE in release_bb_predicate

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84178

--- Comment #9 from Richard Biener  ---
Author: rguenth
Date: Fri Mar  9 13:29:39 2018
New Revision: 258387

URL: https://gcc.gnu.org/viewcvs?rev=258387=gcc=rev
Log:
2018-03-09  Richard Biener  

PR tree-optimization/84775
* tree-if-conv.c (add_bb_predicate_gimplified_stmts): Delink
immediate uses of predicate stmts and mark them modified.

Revert
PR tree-optimization/84178
* tree-if-conv.c (combine_blocks): Move insert_gimplified_predicates
to caller.
(version_loop_for_if_conversion): Delay update_ssa call.
(tree_if_conversion): Delay update_ssa until after predicate
insertion.

* gcc.dg/torture/pr84775.c: New testcase.

Added:
trunk/gcc/testsuite/gcc.dg/torture/pr84775.c
Modified:
trunk/gcc/ChangeLog
trunk/gcc/testsuite/ChangeLog
trunk/gcc/tree-if-conv.c

[Bug tree-optimization/84178] [7/8 Regression] ICE in release_bb_predicate

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84178

--- Comment #8 from Richard Biener  ---
Author: rguenth
Date: Thu Mar  8 12:56:40 2018
New Revision: 258364

URL: https://gcc.gnu.org/viewcvs?rev=258364=gcc=rev
Log:
2018-03-08  Richard Biener  

PR tree-optimization/84178
* tree-if-conv.c (combine_blocks): Move insert_gimplified_predicates
to caller.
(version_loop_for_if_conversion): Delay update_ssa call.
(tree_if_conversion): Delay update_ssa until after predicate
insertion.

* gcc.dg/torture/pr84178-2.c: New testcase.

Added:
trunk/gcc/testsuite/gcc.dg/torture/pr84178-2.c
Modified:
trunk/gcc/ChangeLog
trunk/gcc/testsuite/ChangeLog
trunk/gcc/tree-if-conv.c

[Bug tree-optimization/84178] [7/8 Regression] ICE in release_bb_predicate

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84178

--- Comment #7 from David Malcolm  ---
(In reply to David Malcolm from comment #6)
> Author: dmalcolm
> Date: Thu Mar  8 12:17:36 2018
> New Revision: 258363

This fixes the testcase in comment #0 on trunk.  Not yet fixed for
gcc-7-branch.

The testcase in comment #2 still affects trunk and gcc-7-branch; candidate
patch for that is here:
  https://gcc.gnu.org/ml/gcc-patches/2018-03/msg00376.html

[Bug tree-optimization/84178] [7/8 Regression] ICE in release_bb_predicate

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84178

--- Comment #6 from David Malcolm  ---
Author: dmalcolm
Date: Thu Mar  8 12:17:36 2018
New Revision: 258363

URL: https://gcc.gnu.org/viewcvs?rev=258363=gcc=rev
Log:
tree-if-conv.c: fix ICE seen with -fno-tree-forwprop (PR
tree-optimization/84178)

PR tree-optimization/84178 reports a couple of source files that ICE inside
ifcvt when compiled with -03 -fno-tree-forwprop (trunk and gcc 7).

Both cases involve problems with ifcvt's per-BB gimplified predicates.

Testcase 1 fails this assertion within release_bb_predicate during cleanup:

283   if (flag_checking)
284 for (gimple_stmt_iterator i = gsi_start (stmts);
285  !gsi_end_p (i); gsi_next ())
286   gcc_assert (! gimple_use_ops (gsi_stmt (i)));

The testcase contains a division in the loop, which leads to
if_convertible_loop_p returning false (due to gimple_could_trap_p being true
for the division).  This happens *after* the per-BB gimplified predicates
have been created in predicate_bbs (loop).
Hence tree_if_conversion bails out to "cleanup", but the gimplified predicates
exist and make use of SSA names; for example this conjunction for two BB
conditions:

  _4 = h4.1_112 != 0;
  _175 = (signed char) _117;
  _176 = _175 >= 0;
  _174 = _4 & _176;

is using SSA names.

This assertion was added in r236498 (aka
c3deca2519d97c55876869c57cf11ae1e5c6cf8b):

2016-05-20  Richard Biener  

* tree-if-conv.c (add_bb_predicate_gimplified_stmts): Use
gimple_seq_add_seq_without_update.
(release_bb_predicate): Assert we have no operands to free.
(if_convertible_loop_p_1): Calculate post dominators later.
Do not free BB predicates here.
(combine_blocks): Do not recompute BB predicates.
(version_loop_for_if_conversion): Save BB predicates around
loop versioning.

* gcc.dg/tree-ssa/ifc-cd.c: Adjust.

The following patch fixes this by adding a call to gimple_seq_discard
to release_bb_predicate.  It also updates the assertion, so that
instead of asserting the stmts have no imm uses, instead assert that
they weren't added to a bb before discarding them (otherwise discarding
them would be a bug).  We know this is the case because
insert_gimplified_predicates has:

  /* Once the sequence is code generated, set it to NULL.  */
  set_bb_predicate_gimplified_stmts (bb, NULL);

but asserting it seems appropriate as a double-check.

The patch doesn't address the 2nd issue within PR tree-optimization/84178.

gcc/ChangeLog:
PR tree-optimization/84178
* tree-if-conv.c (release_bb_predicate): Remove the
the assertion that the stmts have NULL use_ops.
Discard the statements, asserting that they haven't
yet been added to a BB.

gcc/testsuite/ChangeLog:
PR tree-optimization/84178
* gcc.c-torture/compile/pr84178-1.c: New test.


Added:
trunk/gcc/testsuite/gcc.c-torture/compile/pr84178-1.c
Modified:
trunk/gcc/ChangeLog
trunk/gcc/testsuite/ChangeLog
trunk/gcc/tree-if-conv.c

[Bug tree-optimization/84178] [7/8 Regression] ICE in release_bb_predicate

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84178

Richard Biener  changed:

   What|Removed |Added

   Priority|P3  |P2
 CC||rguenth at gcc dot gnu.org
   Target Milestone|--- |7.4

[Bug tree-optimization/84178] [7/8 Regression] ICE in release_bb_predicate

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84178

--- Comment #5 from David Malcolm  ---
Candidate patch/RFC:
  https://gcc.gnu.org/ml/gcc-patches/2018-02/msg00468.html

[Bug tree-optimization/84178] [7/8 Regression] ICE in release_bb_predicate

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84178

--- Comment #4 from David Malcolm  ---
FWIW, in both cases the ICE started with r240865.

I have a fix for the ICE in comment #0, but not yet for the one in comment #2.

[Bug tree-optimization/84178] [7/8 Regression] ICE in release_bb_predicate

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84178

--- Comment #3 from David Malcolm  ---
(In reply to Arseny Solokha from comment #2)
> Trivial change to the testcase also makes gcc-8 ICE in
> mark_block_for_update():

Thanks; yes, I see this one segfault both trunk and gcc 7; gcc 6 is unaffected.

[Bug tree-optimization/84178] [7/8 Regression] ICE in release_bb_predicate

[asolokha at gmx dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84178

--- Comment #2 from Arseny Solokha  ---
Trivial change to the testcase also makes gcc-8 ICE in mark_block_for_update():

--- tt.c~   2018-02-05 11:46:10.616358242 +0700
+++ tt.c2018-02-05 11:46:14.640320524 +0700
@@ -7,7 +7,7 @@ r8 (long int mu, int *jr, int *fi, short
 {
   int tx;

-  tx = !!h4 ? (zy / h4) : 1;
+  tx = !!h4 ? (zy + h4) : 1;
   mu = tx;
   *jr = (((unsigned char) mu > (254 >> dv)) ? 0 : (unsigned char) tx) +
*fi;
 } while (*jr == 0);

% gcc-8.0.0-alpha20180204 -O3 -fno-tree-forwprop -c tt.c
during GIMPLE pass: ifcvt
tt.c: In function 'r8':
tt.c:4:1: internal compiler error: Segmentation fault
 r8 (long int mu, int *jr, int *fi, short int dv)
 ^~
0xc97d0f crash_signal
   
/var/tmp/portage/sys-devel/gcc-8.0.0_alpha20180204/work/gcc-8-20180204/gcc/toplev.c:325
0xd1d788 mark_block_for_update
   
/var/tmp/portage/sys-devel/gcc-8.0.0_alpha20180204/work/gcc-8-20180204/gcc/tree-into-ssa.c:449
0xd27297 mark_use_interesting
   
/var/tmp/portage/sys-devel/gcc-8.0.0_alpha20180204/work/gcc-8-20180204/gcc/tree-into-ssa.c:2546
0xd27297 prepare_use_sites_for
   
/var/tmp/portage/sys-devel/gcc-8.0.0_alpha20180204/work/gcc-8-20180204/gcc/tree-into-ssa.c:2711
0xd27297 prepare_names_to_update
   
/var/tmp/portage/sys-devel/gcc-8.0.0_alpha20180204/work/gcc-8-20180204/gcc/tree-into-ssa.c:2779
0xd27297 update_ssa(unsigned int)
   
/var/tmp/portage/sys-devel/gcc-8.0.0_alpha20180204/work/gcc-8-20180204/gcc/tree-into-ssa.c:3345
0xd028aa version_loop_for_if_conversion
   
/var/tmp/portage/sys-devel/gcc-8.0.0_alpha20180204/work/gcc-8-20180204/gcc/tree-if-conv.c:2594
0xd0829a tree_if_conversion(loop*)
   
/var/tmp/portage/sys-devel/gcc-8.0.0_alpha20180204/work/gcc-8-20180204/gcc/tree-if-conv.c:2854
0xd0a335 execute
   
/var/tmp/portage/sys-devel/gcc-8.0.0_alpha20180204/work/gcc-8-20180204/gcc/tree-if-conv.c:2962

[Bug tree-optimization/84178] [7/8 Regression] ICE in release_bb_predicate

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84178

David Malcolm  changed:

   What|Removed |Added

 Status|UNCONFIRMED |ASSIGNED
   Last reconfirmed||2018-02-02
 CC||dmalcolm at gcc dot gnu.org
   Assignee|unassigned at gcc dot gnu.org  |dmalcolm at gcc dot 
gnu.org
 Ever confirmed|0   |1

--- Comment #1 from David Malcolm  ---
Confirmed with trunk and gcc 7.

There seem to be two issues here; the differences seen between trunk and gcc 7
appear to be due to trunk currently defaulting to
  --enable-checking=yes,extra
and 7 defaulting to
  --enable-checking=release
and thus gcc_assert is a no-op by default for gcc 7.

Checked builds of trunk and gcc 7 both fail this assertion inside "ifcvt":

283   if (flag_checking)
284 for (gimple_stmt_iterator i = gsi_start (stmts);
285  !gsi_end_p (i); gsi_next ())
286   gcc_assert (! gimple_use_ops (gsi_stmt (i)));

#1  0x010b98ff in release_bb_predicate (bb=) at ../../src/gcc/tree-if-conv.c:286
#2  0x010b994c in free_bb_predicate (bb=) at ../../src/gcc/tree-if-conv.c:300
#3  0x010c091f in tree_if_conversion (loop=0x71a09ee0) at
../../src/gcc/tree-if-conv.c:2897
#4  0x010c0aa2 in (anonymous namespace)::pass_if_conversion::execute
(this=0x2abab80, fun=0x71a00160)
at ../../src/gcc/tree-if-conv.c:2962
#5  0x00ec11b2 in execute_one_pass (pass=) at ../../src/gcc/passes.c:2497
[...]

on this GIMPLE_ASSIGN:

_175 = (signed char) _117;


Release builds of trunk and of gcc 7 fail later, reading through a NULL
basic_block in VRP:

(gdb) bt
#0  mark_block_for_update(basic_block_def*) () at
../../src/gcc/tree-into-ssa.c:447
#1  0x00af1c10 in mark_use_interesting (insert_phi_p=false,
bb=, stmt=, 
var=) at ../../src/gcc/tree-into-ssa.c:2540
#2  prepare_use_sites_for (insert_phi_p=false, name=)
at ../../src/gcc/tree-into-ssa.c:2705
#3  prepare_names_to_update (insert_phi_p=false) at
../../src/gcc/tree-into-ssa.c:2773
#4  update_ssa(unsigned int) () at ../../src/gcc/tree-into-ssa.c:3339
#5  0x00c39fd0 in insert_range_assertions () at
../../src/gcc/tree-vrp.c:6849
#6  execute_vrp (warn_array_bounds_p=false) at ../../src/gcc/tree-vrp.c:11724
#7  (anonymous namespace)::pass_vrp::execute(function*) () at
../../src/gcc/tree-vrp.c:11833
#8  0x009df329 in execute_one_pass (pass=pass@entry=) at ../../src/gcc/passes.c:2465
[...]

again on a GIMPLE_ASSIGN:
_177 = (signed char) _26;

due to it having a NULL bb:

(gdb) p stmt->bb
$5 =