On March 28, 2020 1:50:59 AM GMT+01:00, Jakub Jelinek <ja...@redhat.com> wrote: >Hi! > >The following testcase FAILs with -fcompare-debug, because >reassociate_bb >mishandles the case when the last stmt in a bb has zero uses. In that >case >reassoc_remove_stmt (like gsi_remove) moves the iterator to the next >stmt, >i.e. gsi_end_p is true, which means the code sets the iterator back to >gsi_last_bb. The problem is that the for loop does gsi_prev on that >before >handling the next statement, which means the former penultimate stmt, >now >last one, is not processed by reassociate_bb. >Now, with -g, if there is at least one debug stmt at the end of the bb, >reassoc_remove_stmt moves the iterator to that following debug stmt and >we >just do gsi_prev and continue with the former penultimate non-debug >stmt, >now last non-debug stmt. > >The following patch fixes that by not doing the gsi_prev in this case; >there >are too many continue; cases, so I didn't want to copy over the >gsi_prev to >all of them, so this patch uses a bool for that instead. The second >gsi_end_p check isn't needed anymore, because when we don't do the >undesirable gsi_prev after gsi = gsi_last_bb, the loop !gsi_end_p (gsi) >condition will catch the removal of the very last stmt from a bb. > >Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK. Richard. >2020-03-28 Jakub Jelinek <ja...@redhat.com> > > PR tree-optimization/94329 > * tree-ssa-reassoc.c (reassociate_bb): When calling >reassoc_remove_stmt > on the last stmt in a bb, make sure gsi_prev isn't done immediately > after gsi_last_bb. > > * gfortran.dg/pr94329.f90: New test. > >--- gcc/tree-ssa-reassoc.c.jj 2020-03-17 13:50:52.319942549 +0100 >+++ gcc/tree-ssa-reassoc.c 2020-03-27 15:49:14.323217631 +0100 >@@ -6224,8 +6224,11 @@ reassociate_bb (basic_block bb) > if (stmt && !gimple_visited_p (stmt)) > cfg_cleanup_needed |= maybe_optimize_range_tests (stmt); > >- for (gsi = gsi_last_bb (bb); !gsi_end_p (gsi); gsi_prev (&gsi)) >+ bool do_prev = false; >+ for (gsi = gsi_last_bb (bb); >+ !gsi_end_p (gsi); do_prev ? gsi_prev (&gsi) : (void) 0) > { >+ do_prev = true; > stmt = gsi_stmt (gsi); > > if (is_gimple_assign (stmt) >@@ -6246,15 +6249,12 @@ reassociate_bb (basic_block bb) > release_defs (stmt); > /* We might end up removing the last stmt above which > places the iterator to the end of the sequence. >- Reset it to the last stmt in this case which might >- be the end of the sequence as well if we removed >- the last statement of the sequence. In which case >- we need to bail out. */ >+ Reset it to the last stmt in this case and make sure >+ we don't do gsi_prev in that case. */ > if (gsi_end_p (gsi)) > { > gsi = gsi_last_bb (bb); >- if (gsi_end_p (gsi)) >- break; >+ do_prev = false; > } > } > continue; >--- gcc/testsuite/gfortran.dg/pr94329.f90.jj 2020-03-27 >15:54:46.453249143 +0100 >+++ gcc/testsuite/gfortran.dg/pr94329.f90 2020-03-27 15:54:23.474592894 >+0100 >@@ -0,0 +1,12 @@ >+! PR tree-optimization/94329 >+! { dg-do compile } >+! { dg-options "-O1 -fno-tree-loop-optimize -fwrapv -fcompare-debug" } >+ >+subroutine pr94329 (s, t) >+ real :: s, t(:,:) >+ do i = 1,3 >+ do j = 1,3 >+ s = t(i,j) >+ end do >+ end do >+end > > Jakub