* [PATCH] Fix PR80158
@ 2017-03-23 19:08 Bill Schmidt
2017-03-24 8:29 ` Richard Biener
0 siblings, 1 reply; 3+ messages in thread
From: Bill Schmidt @ 2017-03-23 19:08 UTC (permalink / raw)
To: GCC Patches; +Cc: Richard Biener
Hi,
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80158 reports an ICE in
SLSR while building 416.gamess on x86_64. This is a latent (but
previously harmless) bug that was exposed by the fix for PR80054.
When replacing any statement with a strength reduction, SLSR updates
the candidate statement S associated with the associated candidate
record in the candidate table. However, S may actually be associated
with two candidate records when an expression may be treated as
either a CAND_ADD or a CAND_MULT. In this case, the second one can
be reached via the next_interp field.
In the excerpt from 416.gamess, the first candidate record was
updated when its statement was replaced, but the next-interp record
was not. Later, that record was used as a basis for another tree
of strength-reduction candidates, but since it now pointed to an
orphaned statement without a basic block, the ICE occurred when
checking dominance against the statement's block.
The fix is simply to ensure that the next_interp record is always
updated when present.
I've bootstrapped and tested this on powerpc64le-unknown-linux-gnu
with no regressions. I tested that the standalone test works on
x86_64 with this fix, but I have been unable to perform an x86_64
regstrap because the machine I have access to has insufficient
disk space. :/ I would appreciate it if you could do this for me.
Assuming no problems with x86_64 regstrap, is this ok for trunk?
Note: I will be unreachable from now until next Tuesday (i.e.,
returning the 28th). If needed, please feel free to commit the
patch on my behalf. Otherwise I will attend to it when I return.
Thanks!
Bill
[gcc]
2017-03-23 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.
[gcc/testsuite]
2017-03-23 Bill Schmidt <wschmidt@linux.vnet.ibm.com>
PR tree-optimization/80158
* gfortran.fortran-torture/compile/pr80158.f: New file.
Index: gcc/gimple-ssa-strength-reduction.c
===================================================================
--- gcc/gimple-ssa-strength-reduction.c (revision 246420)
+++ 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;
Index: gcc/testsuite/gfortran.fortran-torture/compile/pr80158.f
===================================================================
--- gcc/testsuite/gfortran.fortran-torture/compile/pr80158.f (nonexistent)
+++ gcc/testsuite/gfortran.fortran-torture/compile/pr80158.f (working copy)
@@ -0,0 +1,16 @@
+ 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
+
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Fix PR80158
2017-03-23 19:08 [PATCH] Fix PR80158 Bill Schmidt
@ 2017-03-24 8:29 ` Richard Biener
2017-03-28 13:50 ` Bill Schmidt
0 siblings, 1 reply; 3+ messages in thread
From: Richard Biener @ 2017-03-24 8:29 UTC (permalink / raw)
To: Bill Schmidt; +Cc: GCC Patches
On Thu, Mar 23, 2017 at 7:20 PM, Bill Schmidt
<wschmidt@linux.vnet.ibm.com> wrote:
> Hi,
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80158 reports an ICE in
> SLSR while building 416.gamess on x86_64. This is a latent (but
> previously harmless) bug that was exposed by the fix for PR80054.
> When replacing any statement with a strength reduction, SLSR updates
> the candidate statement S associated with the associated candidate
> record in the candidate table. However, S may actually be associated
> with two candidate records when an expression may be treated as
> either a CAND_ADD or a CAND_MULT. In this case, the second one can
> be reached via the next_interp field.
Hmm, the following code in SLSR suggests there may be more than
one such alternate candidate:
while (arg_cand->kind != CAND_ADD && arg_cand->kind != CAND_PHI)
{
if (!arg_cand->next_interp)
return;
arg_cand = lookup_cand (arg_cand->next_interp);
}
> In the excerpt from 416.gamess, the first candidate record was
> updated when its statement was replaced, but the next-interp record
> was not. Later, that record was used as a basis for another tree
> of strength-reduction candidates, but since it now pointed to an
> orphaned statement without a basic block, the ICE occurred when
> checking dominance against the statement's block.
>
> The fix is simply to ensure that the next_interp record is always
> updated when present.
>
> I've bootstrapped and tested this on powerpc64le-unknown-linux-gnu
> with no regressions. I tested that the standalone test works on
> x86_64 with this fix, but I have been unable to perform an x86_64
> regstrap because the machine I have access to has insufficient
> disk space. :/ I would appreciate it if you could do this for me.
>
> Assuming no problems with x86_64 regstrap, is this ok for trunk?
>
> Note: I will be unreachable from now until next Tuesday (i.e.,
> returning the 28th). If needed, please feel free to commit the
> patch on my behalf. Otherwise I will attend to it when I return.
I'll do a regstrap and gamess build and will commit if that succeeds.
The above can be addressed as followup.
Thanks,
Richard.
> Thanks!
> Bill
>
>
> [gcc]
>
> 2017-03-23 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.
>
> [gcc/testsuite]
>
> 2017-03-23 Bill Schmidt <wschmidt@linux.vnet.ibm.com>
>
> PR tree-optimization/80158
> * gfortran.fortran-torture/compile/pr80158.f: New file.
>
>
> Index: gcc/gimple-ssa-strength-reduction.c
> ===================================================================
> --- gcc/gimple-ssa-strength-reduction.c (revision 246420)
> +++ 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;
> Index: gcc/testsuite/gfortran.fortran-torture/compile/pr80158.f
> ===================================================================
> --- gcc/testsuite/gfortran.fortran-torture/compile/pr80158.f (nonexistent)
> +++ gcc/testsuite/gfortran.fortran-torture/compile/pr80158.f (working copy)
> @@ -0,0 +1,16 @@
> + 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
> +
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Fix PR80158
2017-03-24 8:29 ` Richard Biener
@ 2017-03-28 13:50 ` Bill Schmidt
0 siblings, 0 replies; 3+ messages in thread
From: Bill Schmidt @ 2017-03-28 13:50 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches
On Mar 24, 2017, at 3:19 AM, Richard Biener <richard.guenther@gmail.com> wrote:
>
> On Thu, Mar 23, 2017 at 7:20 PM, Bill Schmidt
> <wschmidt@linux.vnet.ibm.com> wrote:
>> Hi,
>>
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80158 reports an ICE in
>> SLSR while building 416.gamess on x86_64. This is a latent (but
>> previously harmless) bug that was exposed by the fix for PR80054.
>> When replacing any statement with a strength reduction, SLSR updates
>> the candidate statement S associated with the associated candidate
>> record in the candidate table. However, S may actually be associated
>> with two candidate records when an expression may be treated as
>> either a CAND_ADD or a CAND_MULT. In this case, the second one can
>> be reached via the next_interp field.
>
> Hmm, the following code in SLSR suggests there may be more than
> one such alternate candidate:
>
> while (arg_cand->kind != CAND_ADD && arg_cand->kind != CAND_PHI)
> {
> if (!arg_cand->next_interp)
> return;
>
> arg_cand = lookup_cand (arg_cand->next_interp);
> }
You're right, although with the current design there is never more than one
alternate interpretation. So the patch is correct now, but I should make it handle
additional alternate interpretations to future-proof it. I'll fix that later this week.
Thanks for catching this.
>> In the excerpt from 416.gamess, the first candidate record was
>> updated when its statement was replaced, but the next-interp record
>> was not. Later, that record was used as a basis for another tree
>> of strength-reduction candidates, but since it now pointed to an
>> orphaned statement without a basic block, the ICE occurred when
>> checking dominance against the statement's block.
>>
>> The fix is simply to ensure that the next_interp record is always
>> updated when present.
>>
>> I've bootstrapped and tested this on powerpc64le-unknown-linux-gnu
>> with no regressions. I tested that the standalone test works on
>> x86_64 with this fix, but I have been unable to perform an x86_64
>> regstrap because the machine I have access to has insufficient
>> disk space. :/ I would appreciate it if you could do this for me.
>>
>> Assuming no problems with x86_64 regstrap, is this ok for trunk?
>>
>> Note: I will be unreachable from now until next Tuesday (i.e.,
>> returning the 28th). If needed, please feel free to commit the
>> patch on my behalf. Otherwise I will attend to it when I return.
>
> I'll do a regstrap and gamess build and will commit if that succeeds.
Thank you, I appreciate you handling this for me in my absence!
Bill
>
> The above can be addressed as followup.
>
> Thanks,
> Richard.
>
>> Thanks!
>> Bill
>>
>>
>> [gcc]
>>
>> 2017-03-23 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.
>>
>> [gcc/testsuite]
>>
>> 2017-03-23 Bill Schmidt <wschmidt@linux.vnet.ibm.com>
>>
>> PR tree-optimization/80158
>> * gfortran.fortran-torture/compile/pr80158.f: New file.
>>
>>
>> Index: gcc/gimple-ssa-strength-reduction.c
>> ===================================================================
>> --- gcc/gimple-ssa-strength-reduction.c (revision 246420)
>> +++ 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;
>> Index: gcc/testsuite/gfortran.fortran-torture/compile/pr80158.f
>> ===================================================================
>> --- gcc/testsuite/gfortran.fortran-torture/compile/pr80158.f (nonexistent)
>> +++ gcc/testsuite/gfortran.fortran-torture/compile/pr80158.f (working copy)
>> @@ -0,0 +1,16 @@
>> + 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
>> +
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-03-28 12:51 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-23 19:08 [PATCH] Fix PR80158 Bill Schmidt
2017-03-24 8:29 ` Richard Biener
2017-03-28 13:50 ` Bill Schmidt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).