Bug 80158 - [7 Regression] ICE in all_phi_incrs_profitable
Summary: [7 Regression] ICE in all_phi_incrs_profitable
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 7.0.1
: P1 normal
Target Milestone: 7.0
Assignee: Bill Schmidt
URL:
Keywords: ice-on-valid-code
Depends on:
Blocks:
 
Reported: 2017-03-23 10:36 UTC by Richard Biener
Modified: 2017-03-29 12:57 UTC (History)
2 users (show)

See Also:
Host:
Target: i?86-*-*
Build:
Known to work:
Known to fail:
Last reconfirmed: 2017-03-23 00:00:00


Attachments
reassoc2 dump (3.54 KB, text/plain)
2017-03-23 14:42 UTC, Richard Biener
Details
slsr dump (4.68 KB, text/plain)
2017-03-23 14:42 UTC, Richard Biener
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Biener 2017-03-23 10:36:26 UTC
416.gamess fails to build on x86_64 with -m32 -Ofast -march=bdver2:

/gcc/spec/sb-megrez-head-64-32o-2006/x86_64/install-hack/bin/gfortran -c -o efpaul.fppized.o    -Ofast -march=native -m32       -DSPEC_CPU_LP64  -ffixed-form       efpaul.fppized.f
efpaul.fppized.f:2153:72:

      *          X(IENG),ZQQ,NROT)
                                                                        1
Warning: Type mismatch in argument 'ibuf' at (1); passed REAL(8) to INTEGER(4) -Wargument-mismatch]
efpaul.fppized.f:2174:72:

      *           NINTMX)
                                                                        1
Warning: Type mismatch in argument 'ibuf' at (1); passed REAL(8) to INTEGER(4) -Wargument-mismatch]
efpaul.fppized.f:1651:0:

       SUBROUTINE DRPAUL(SMAT,TMAT,EPS,EPT,SIJ,TIJ,WRK,VEC,ARRAY,FMO,
 
internal compiler error: Segmentation fault
0xaf251f crash_signal
        /gcc/spec/sb-megrez-head-64-32o-2006/gcc/gcc/toplev.c:337
0x79b22b dominated_by_p(cdi_direction, basic_block_def const*, basic_block_def const*)
        /gcc/spec/sb-megrez-head-64-32o-2006/gcc/gcc/dominance.c:1117
0x11c10c8 all_phi_incrs_profitable
        /gcc/spec/sb-megrez-head-64-32o-2006/gcc/gcc/gimple-ssa-strength-reduction.c:3297
0x11c1a81 replace_profitable_candidates
        /gcc/spec/sb-megrez-head-64-32o-2006/gcc/gcc/gimple-ssa-strength-reduction.c:3587
0x11c184f replace_profitable_candidates
        /gcc/spec/sb-megrez-head-64-32o-2006/gcc/gcc/gimple-ssa-strength-reduction.c:3619
0x11c587a analyze_candidates_and_replace
        /gcc/spec/sb-megrez-head-64-32o-2006/gcc/gcc/gimple-ssa-strength-reduction.c:3698
0x11c8783 execute
        /gcc/spec/sb-megrez-head-64-32o-2006/gcc/gcc/gimple-ssa-strength-reduction.c:3772
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.

This is a recent regression.
Comment 1 Richard Biener 2017-03-23 10:46:43 UTC
#1  0x00000000019a81e7 in all_phi_incrs_profitable (c=0x2c907b0, 
    phi=0x7ffff6684400)
    at /space/rguenther/src/svn/gcc-7-branch/gcc/gimple-ssa-strength-reduction.c:3298
3298          || !dominated_by_p (CDI_DOMINATORS, phi_bb, basis_bb))
(gdb) l
3293         using the basis here.  */
3294      basic_block basis_bb = gimple_bb (basis->cand_stmt);
3295      basic_block phi_bb = gimple_bb (phi);
3296
3297      if (phi_bb == basis_bb
3298          || !dominated_by_p (CDI_DOMINATORS, phi_bb, basis_bb))
3299        return false;
3300
3301      for (i = 0; i < gimple_phi_num_args (phi); i++)
3302        {
(gdb) p basis_bb
$4 = <basic_block 0x0>

basis->cand_stmt is not in any BB.  Reducing testcase...
Comment 2 Richard Biener 2017-03-23 11:20:37 UTC
      SUBROUTINE DRPAUL(SMAT,TMAT,EPS,EPT,SIJ,TIJ,WRK,VEC,ARRAY,FMO,
     *                  XMKVIR,TMJ,XMI,YMI,ZMI,ZQQ,L1,L1EF,LNA,LNA2,
     *                  NAEF,L2,NLOC,NVIR,PROVEC,FOCKMA,MXBF,MXMO2)
      DIMENSION CMO(L1,L1),TLOC(LNA,LNA),SMJ(L1,NAEF),XMK(L1,LNA)
      DO I = 1,LNA
         DO J = 1,LNA
            IF (I.LE.NOUT) TLOC(I,J) = ZERO
            IF (J.LE.NOUT) TLOC(I,J) = ZERO
         END DO
         DO NA=1,NOC
            IF ( ABS(E(NI)-E(NA)) .GE.TOL) THEN
            END IF
         END DO
      END DO
      END

possibly somewhat nonsensical reduced but it reproduces the issue.
Comment 3 Bill Schmidt 2017-03-23 13:24:42 UTC
Richard, what flags are you using with the reduced test case?  Hoping I can reproduce this on ppc64le without a cross, but so far no luck.
Comment 4 rguenther@suse.de 2017-03-23 13:52:51 UTC
On Thu, 23 Mar 2017, wschmidt at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80158
> 
> --- Comment #3 from Bill Schmidt <wschmidt at gcc dot gnu.org> ---
> Richard, what flags are you using with the reduced test case?  Hoping I can
> reproduce this on ppc64le without a cross, but so far no luck.

-Ofast -m32 -march=bdver2

looks like vectorization is needed to trigger it
Comment 5 Bill Schmidt 2017-03-23 14:07:05 UTC
OK, I will have to find an x86 box -- fortran cross is too challenging.  Meanwhile, could you please add -fdump-tree-reassoc2 and -fdump-tree-slsr-details and post the results?  Might be able to figure it out from that.
Comment 6 Richard Biener 2017-03-23 14:42:18 UTC
Created attachment 41036 [details]
reassoc2 dump
Comment 7 Richard Biener 2017-03-23 14:42:49 UTC
Created attachment 41037 [details]
slsr dump

probably not too helpful as it crashes
Comment 8 rguenther@suse.de 2017-03-23 14:44:29 UTC
On Thu, 23 Mar 2017, wschmidt at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80158
> 
> --- Comment #5 from Bill Schmidt <wschmidt at gcc dot gnu.org> ---
> OK, I will have to find an x86 box -- fortran cross is too challenging. 
> Meanwhile, could you please add -fdump-tree-reassoc2 and
> -fdump-tree-slsr-details and post the results?  Might be able to figure it out
> from that.

Attached.  Fortran cross should be easy enough as you just need f951,
so make all-gcc is enough.
Comment 9 Bill Schmidt 2017-03-23 14:46:53 UTC
Thanks, those dumps are very helpful.  I found an x86 box, just need to get set up now.  The SLSR dump is truncated but it still tells me what it was working on when it died, so should help me out.
Comment 10 Bill Schmidt 2017-03-23 15:10:36 UTC
Pretty certain the problem is in this chunk:

      if (bump == 0)
        {
          tree lhs = gimple_assign_lhs (c->cand_stmt);
          gassign *copy_stmt = gimple_build_assign (lhs, basis_name);
          gimple_stmt_iterator gsi = gsi_for_stmt (c->cand_stmt);
          gimple_set_location (copy_stmt, gimple_location (c->cand_stmt));
          gsi_replace (&gsi, copy_stmt, false);
          c->cand_stmt = copy_stmt;
          if (dump_file && (dump_flags & TDF_DETAILS))
            stmt_to_print = copy_stmt;
	}

We need a gimple_set_bb (copy_stmt, gimple_bb (c->cand_stmt)) in here if we're going to rely on the BB information for dominators subsequently.  There may be other cases like this in the code.
Comment 11 Jakub Jelinek 2017-03-23 15:14:14 UTC
(In reply to Bill Schmidt from comment #10)
> Pretty certain the problem is in this chunk:
> 
>       if (bump == 0)
>         {
>           tree lhs = gimple_assign_lhs (c->cand_stmt);
>           gassign *copy_stmt = gimple_build_assign (lhs, basis_name);
>           gimple_stmt_iterator gsi = gsi_for_stmt (c->cand_stmt);
>           gimple_set_location (copy_stmt, gimple_location (c->cand_stmt));
>           gsi_replace (&gsi, copy_stmt, false);
>           c->cand_stmt = copy_stmt;
>           if (dump_file && (dump_flags & TDF_DETAILS))
>             stmt_to_print = copy_stmt;
> 	}
> 
> We need a gimple_set_bb (copy_stmt, gimple_bb (c->cand_stmt)) in here if
> we're going to rely on the BB information for dominators subsequently. 
> There may be other cases like this in the code.

gsi_replace does
gimple_set_bb (stmt, gsi_bb (*gsi));
so I don't see why that would be needed.
Comment 12 rguenther@suse.de 2017-03-23 15:19:32 UTC
On Thu, 23 Mar 2017, wschmidt at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80158
> 
> --- Comment #10 from Bill Schmidt <wschmidt at gcc dot gnu.org> ---
> Pretty certain the problem is in this chunk:
> 
>       if (bump == 0)
>         {
>           tree lhs = gimple_assign_lhs (c->cand_stmt);
>           gassign *copy_stmt = gimple_build_assign (lhs, basis_name);
>           gimple_stmt_iterator gsi = gsi_for_stmt (c->cand_stmt);
>           gimple_set_location (copy_stmt, gimple_location (c->cand_stmt));
>           gsi_replace (&gsi, copy_stmt, false);
>           c->cand_stmt = copy_stmt;
>           if (dump_file && (dump_flags & TDF_DETAILS))
>             stmt_to_print = copy_stmt;
>         }
> 
> We need a gimple_set_bb (copy_stmt, gimple_bb (c->cand_stmt)) in here if we're
> going to rely on the BB information for dominators subsequently.  There may be
> other cases like this in the code.

But copy_stmt should have a BB as you did a gsi_replace on it.  The
stmt in question that causes the crash has none.

Instead I susepct somehwere we replace the affected stmt but keep a
dangling c->cand_stmt pointing to it around.
Comment 13 Bill Schmidt 2017-03-23 15:21:35 UTC
OK, sure, that is quite possible.  Seems like something that should have popped up before, but I guess the information gathered from the old cand->stmt must have been harmless.
Comment 14 Bill Schmidt 2017-03-23 15:37:41 UTC
I can reproduce now, confirmed.
Comment 15 Bill Schmidt 2017-03-23 15:52:59 UTC
This is the only spot where we don't do an in-situ replacement.  Testing a patch to fix that.
Comment 16 Bill Schmidt 2017-03-23 16:11:21 UTC
Ah, that's not it at all.  This is much more subtle.  This has to do with candidates that have alternate interpretations (as either a CAND_ADD or a CAND_MULT).  We fix up the candidate that we replace, but not the alternate interpretation, if any.  In this case the alternate interpretation is used as a basis for another chain of expressions, and we run into trouble.  Should not be too hard to fix.
Comment 17 Bill Schmidt 2017-03-23 16:27:50 UTC
The following fixes the reduced test case.  Could you please test it on the full 416.gamess build?  I'll regstrap it on x86-64 and ppc64le.

Index: gcc/gimple-ssa-strength-reduction.c
===================================================================
--- gcc/gimple-ssa-strength-reduction.c	(revision 246419)
+++ gcc/gimple-ssa-strength-reduction.c	(working copy)
@@ -2089,6 +2089,8 @@ replace_mult_candidate (slsr_cand_t c, tree basis_
 	  gimple_set_location (copy_stmt, gimple_location (c->cand_stmt));
 	  gsi_replace (&gsi, copy_stmt, false);
 	  c->cand_stmt = copy_stmt;
+	  if (c->next_interp)
+	    lookup_cand (c->next_interp)->cand_stmt = copy_stmt;
 	  if (dump_file && (dump_flags & TDF_DETAILS))
 	    stmt_to_print = copy_stmt;
 	}
@@ -2118,6 +2120,8 @@ replace_mult_candidate (slsr_cand_t c, tree basis_
 					      basis_name, bump_tree);
 	      update_stmt (gsi_stmt (gsi));
               c->cand_stmt = gsi_stmt (gsi);
+	      if (c->next_interp)
+		lookup_cand (c->next_interp)->cand_stmt = gsi_stmt (gsi);
 	      if (dump_file && (dump_flags & TDF_DETAILS))
 		stmt_to_print = gsi_stmt (gsi);
 	    }
@@ -3405,6 +3409,8 @@ replace_rhs_if_not_dup (enum tree_code new_code, t
       gimple_assign_set_rhs_with_ops (&gsi, new_code, new_rhs1, new_rhs2);
       update_stmt (gsi_stmt (gsi));
       c->cand_stmt = gsi_stmt (gsi);
+      if (c->next_interp)
+	lookup_cand (c->next_interp)->cand_stmt = gsi_stmt (gsi);
 
       if (dump_file && (dump_flags & TDF_DETAILS))
 	return gsi_stmt (gsi);
@@ -3511,6 +3517,8 @@ replace_one_candidate (slsr_cand_t c, unsigned i,
 	  gimple_assign_set_rhs_with_ops (&gsi, MINUS_EXPR, basis_name, rhs2);
 	  update_stmt (gsi_stmt (gsi));
           c->cand_stmt = gsi_stmt (gsi);
+	  if (c->next_interp)
+	    lookup_cand (c->next_interp)->cand_stmt = gsi_stmt (gsi);
 
 	  if (dump_file && (dump_flags & TDF_DETAILS))
 	    stmt_to_print = gsi_stmt (gsi);
@@ -3532,6 +3540,8 @@ replace_one_candidate (slsr_cand_t c, unsigned i,
 	  gimple_set_location (copy_stmt, gimple_location (c->cand_stmt));
 	  gsi_replace (&gsi, copy_stmt, false);
 	  c->cand_stmt = copy_stmt;
+	  if (c->next_interp)
+	    lookup_cand (c->next_interp)->cand_stmt = copy_stmt;
 
 	  if (dump_file && (dump_flags & TDF_DETAILS))
 	    stmt_to_print = copy_stmt;
@@ -3543,6 +3553,8 @@ replace_one_candidate (slsr_cand_t c, unsigned i,
 	  gimple_set_location (cast_stmt, gimple_location (c->cand_stmt));
 	  gsi_replace (&gsi, cast_stmt, false);
 	  c->cand_stmt = cast_stmt;
+	  if (c->next_interp)
+	    lookup_cand (c->next_interp)->cand_stmt = cast_stmt;
 
 	  if (dump_file && (dump_flags & TDF_DETAILS))
 	    stmt_to_print = cast_stmt;
Comment 18 Richard Biener 2017-03-24 12:34:42 UTC
Fixed.
Comment 19 Richard Biener 2017-03-24 12:34:50 UTC
Author: rguenth
Date: Fri Mar 24 12:34:19 2017
New Revision: 246439

URL: https://gcc.gnu.org/viewcvs?rev=246439&root=gcc&view=rev
Log:
2017-03-24  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	PR tree-optimization/80158
	* gimple-ssa-strength-reduction.c (replace_mult_candidate): When
	replacing a candidate statement, also replace it for the
	candidate's alternate interpretation.
	(replace_rhs_if_not_dup): Likewise.
	(replace_one_candidate): Likewise.

	* gfortran.fortran-torture/compile/pr80158.f: New file.

Added:
    trunk/gcc/testsuite/gfortran.fortran-torture/compile/pr80158.f
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/gimple-ssa-strength-reduction.c
    trunk/gcc/testsuite/ChangeLog
Comment 20 Bill Schmidt 2017-03-29 12:57:00 UTC
Author: wschmidt
Date: Wed Mar 29 12:56:26 2017
New Revision: 246567

URL: https://gcc.gnu.org/viewcvs?rev=246567&root=gcc&view=rev
Log:
2017-03-29  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	PR tree-optimization/80158
	* gimple-ssa-strength-reduction.c (replace_mult_candidate):
	Handle possible future case of more than one alternate
	interpretation.
	(replace_rhs_if_not_dup): Likewise.
	(replace_one_candidate): Likewise.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/gimple-ssa-strength-reduction.c