Starting from r237064 I see ICE in: $ cat ice.i int a, b, c; void d () { while (a + c > b) a--; } $ gcc ice.i -O3 -ftrapv In function ‘d’: cc1: internal compiler error: in gimplify_modify_expr, at gimplify.c:5638 Please submit a full bug report, with preprocessed source if appropriate. See <http://bugs.opensuse.org/> for instructions. It's with -mtune=generic -march=x86-64. Thanks
niter analysis creates sth that gimplifies to control flow because trapping insns are involved. Confirmed.
So do we need some predicate that will tell us if it is safe to evaluate some generic expression using force_gimple_operand* and bail out if it isn't possible? Even if gimplify_cond_expr tried harder around: 3865 if (gimplify_ctxp->allow_rhs_cond_expr 3866 /* If either branch has side effects or could trap, it can't be 3867 evaluated unconditionally. */ 3868 && !TREE_SIDE_EFFECTS (then_) 3869 && !generic_expr_could_trap_p (then_) 3870 && !TREE_SIDE_EFFECTS (else_) 3871 && !generic_expr_could_trap_p (else_)) 3872 return gimplify_pure_cond_expr (expr_p, pre_p); 3873 3874 tmp = create_tmp_var (type, "iftmp"); and built for the case where gimplify_pure_cond_expr can't be used a SSA_NAME and a PHI if we're in SSA, I guess the vast majority of callers of force_gimple_operand* don't really expect that the sequence might need to be split into several basic blocks.
On Wed, 2 Aug 2017, jakub at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81661 > > Jakub Jelinek <jakub at gcc dot gnu.org> changed: > > What |Removed |Added > ---------------------------------------------------------------------------- > CC| |jakub at gcc dot gnu.org > > --- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> --- > So do we need some predicate that will tell us if it is safe to evaluate some > generic expression using force_gimple_operand* and bail out if it isn't > possible? Yes, see for example niter analysis expand_simple_operations which avoids to turn GIMPLE into GENERIC if that might trap. I suspect we're looking at a similar case here, just needs to be tracked down... > Even if gimplify_cond_expr tried harder around: > 3865 if (gimplify_ctxp->allow_rhs_cond_expr > 3866 /* If either branch has side effects or could trap, it > can't be > 3867 evaluated unconditionally. */ > 3868 && !TREE_SIDE_EFFECTS (then_) > 3869 && !generic_expr_could_trap_p (then_) > 3870 && !TREE_SIDE_EFFECTS (else_) > 3871 && !generic_expr_could_trap_p (else_)) > 3872 return gimplify_pure_cond_expr (expr_p, pre_p); > 3873 > 3874 tmp = create_tmp_var (type, "iftmp"); > and built for the case where gimplify_pure_cond_expr can't be used a SSA_NAME > and a PHI if we're in SSA, I guess the vast majority of callers of > force_gimple_operand* don't really expect that the sequence might need to be > split into several basic blocks. Yes, which is why we shouldn't really support this. Of course the real issue is that we can't emit a non-trapping PLUS_EXPR besides forcing unsigned arithmetic. That said, this would be a whole lot easier to the middle-end if we would have lowered -ftrapv operations and the actual arithmetic were just wrapping or undefined... which is AFAIK where we want to go anyway. Given we now have the overflow IFNs maybe we can try lowering this at gimplification time? (and clear flag_trapv afterwards)
*** Bug 82539 has been marked as a duplicate of this bug. ***
One possibility is instead of using may_be_zero ? 0 : niter as niter for code-generation use (may_be_zero ? 0 : 1) * niter This avoids the gimplification issue. We assemble this as call __addvsi3 movl %eax, %esi subl %ebp, %esi cmpl %ebp, %eax setge %al movzbl %al, %eax imull %eax, %esi so it definitely is ugly (we're lacking any pattern matching on this turning it back to a COND_EXPR). And of course we're then unconditionally emitting a possibly trapping expression into the IL. For the case in question SCEV analysis knows the expression is executed if the loop is entered (otherwise it would have rewritten it to unsigned arithmetic). Also the niter expression is usually similar to the condition operators. So I think a solution boils down to the consumer doing sth more intelligent than building a COND_EXPR and later expecting to just code-gen that via force_gimple_operand. For the vectorizer this could mean a workaround like storing niter away somewhere and using a new SSA name for the niter part in the COND_EXPR and making sure to emit the definition for it in the correct place. Or going the multiplication way suggested above for maybe trapping niter (note that tree_could_trap_p doesn't work recursively, the gimplifier has generic_expr_could_trap_p for this). Which also means a solution could be to simply not vectorize loops with IV operations that might trap.
(In reply to Richard Biener from comment #5) > One possibility is instead of using > > may_be_zero ? 0 : niter > > as niter for code-generation use > > (may_be_zero ? 0 : 1) * niter > > This avoids the gimplification issue. We assemble this as > > call __addvsi3 > movl %eax, %esi > subl %ebp, %esi > cmpl %ebp, %eax > setge %al > movzbl %al, %eax > imull %eax, %esi > > so it definitely is ugly (we're lacking any pattern matching on this > turning it back to a COND_EXPR). r250377 added a pattern which should have caught that ...
On Sat, 28 Oct 2017, pinskia at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81661 > > --- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> --- > (In reply to Richard Biener from comment #5) > > One possibility is instead of using > > > > may_be_zero ? 0 : niter > > > > as niter for code-generation use > > > > (may_be_zero ? 0 : 1) * niter > > > > This avoids the gimplification issue. We assemble this as > > > > call __addvsi3 > > movl %eax, %esi > > subl %ebp, %esi > > cmpl %ebp, %eax > > setge %al > > movzbl %al, %eax > > imull %eax, %esi > > > > so it definitely is ugly (we're lacking any pattern matching on this > > turning it back to a COND_EXPR). > > r250377 added a pattern which should have caught that ... I suspect we're missing cond ? 0 : 1 -> (int) cond for it to trigger.
GCC 7.3 is being released, adjusting target milestone.
Author: jakub Date: Thu Feb 1 10:08:26 2018 New Revision: 257284 URL: https://gcc.gnu.org/viewcvs?rev=257284&root=gcc&view=rev Log: PR tree-optimization/81661 PR tree-optimization/84117 * tree-eh.h (rewrite_to_non_trapping_overflow): Declare. * tree-eh.c: Include gimplify.h. (find_trapping_overflow, replace_trapping_overflow, rewrite_to_non_trapping_overflow): New functions. * tree-vect-loop.c: Include tree-eh.h. (vect_get_loop_niters): Use rewrite_to_non_trapping_overflow. * tree-data-ref.c: Include tree-eh.h. (get_segment_min_max): Use rewrite_to_non_trapping_overflow. * gcc.dg/pr81661.c: New test. * gfortran.dg/pr84117.f90: New test. Added: trunk/gcc/testsuite/gcc.dg/pr81661.c trunk/gcc/testsuite/gfortran.dg/pr84117.f90 Modified: trunk/gcc/ChangeLog trunk/gcc/testsuite/ChangeLog trunk/gcc/tree-data-ref.c trunk/gcc/tree-eh.c trunk/gcc/tree-eh.h trunk/gcc/tree-vect-loop.c
Fixed on the trunk so far.
Author: jakub Date: Sat Mar 3 13:25:27 2018 New Revision: 258183 URL: https://gcc.gnu.org/viewcvs?rev=258183&root=gcc&view=rev Log: Backported from mainline 2018-02-01 Jakub Jelinek <jakub@redhat.com> PR tree-optimization/81661 PR tree-optimization/84117 * tree-eh.h (rewrite_to_non_trapping_overflow): Declare. * tree-eh.c: Include gimplify.h. (find_trapping_overflow, replace_trapping_overflow, rewrite_to_non_trapping_overflow): New functions. * tree-vect-loop.c: Include tree-eh.h. (vect_get_loop_niters): Use rewrite_to_non_trapping_overflow. * gcc.dg/pr81661.c: New test. * gfortran.dg/pr84117.f90: New test. Added: branches/gcc-7-branch/gcc/testsuite/gcc.dg/pr81661.c branches/gcc-7-branch/gcc/testsuite/gfortran.dg/pr84117.f90 Modified: branches/gcc-7-branch/gcc/ChangeLog branches/gcc-7-branch/gcc/testsuite/ChangeLog branches/gcc-7-branch/gcc/tree-eh.c branches/gcc-7-branch/gcc/tree-eh.h branches/gcc-7-branch/gcc/tree-vect-loop.c
Fixed for 7.4+.