[Bug tree-optimization/104931] wrong-code with number_of_iterations_lt_to_ne
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104931
--- Comment #4 from Richard Biener ---
(In reply to Richard Biener from comment #3)
> The bug was made latent by g:51d464b608b38b9e2007948d10b1e0f1dcec142c which
> ensured that
>
> /* If the loop exits immediately, there is nothing to do. */
> tree tem = fold_binary (code, boolean_type_node, iv0->base, iv1->base);
> if (tem && integer_zerop (tem))
> {
> if (!every_iteration)
> return false;
> niter->niter = build_int_cst (unsigned_type_for (type), 0);
> niter->max = 0;
> return true;
> }
>
> triggered, folding (_2 + 4294967272) + 12 < _2 + 4294967272 to false. That's
> the following part of the revision, and it probably triggers when doing
> expand_simple_operations.
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 84c9b918041..f5dcbf32bc7 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -2143,6 +2143,11 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> (simplify
>(pointer_plus (pointer_plus:s @0 @1) @3)
>(pointer_plus @0 (plus @1 @3)))
> +#if GENERIC
> +(simplify
> + (pointer_plus (convert:s (pointer_plus:s @0 @1)) @3)
> + (convert:type (pointer_plus @0 (plus @1 @3
> +#endif
>
> /* Pattern match
>
> It does seem to me that niter analysis relies on statically detecting
> not rolling loops, at least for the cases we assume no overflow happens
> and we use number_of_iterations_lt_to_ne. I can't convince myself that
> only INTEGER_CST appearant negative delta are problematic for
> pointer types (which we always assume to have no overflow), but that would
> be the most simplistic solution here. Still "negative" delta values should
> be problematic for all cases, and since we only restrict us to constant
> modulo, delta can also be non-constant.
I verified backporting the above fixes the issue. That's really the patch
I'm most comfortable with at this point ... :/
[Bug tree-optimization/104931] wrong-code with number_of_iterations_lt_to_ne
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104931
--- Comment #3 from Richard Biener ---
The bug was made latent by g:51d464b608b38b9e2007948d10b1e0f1dcec142c which
ensured that
/* If the loop exits immediately, there is nothing to do. */
tree tem = fold_binary (code, boolean_type_node, iv0->base, iv1->base);
if (tem && integer_zerop (tem))
{
if (!every_iteration)
return false;
niter->niter = build_int_cst (unsigned_type_for (type), 0);
niter->max = 0;
return true;
}
triggered, folding (_2 + 4294967272) + 12 < _2 + 4294967272 to false. That's
the following part of the revision, and it probably triggers when doing
expand_simple_operations.
diff --git a/gcc/match.pd b/gcc/match.pd
index 84c9b918041..f5dcbf32bc7 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -2143,6 +2143,11 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(simplify
(pointer_plus (pointer_plus:s @0 @1) @3)
(pointer_plus @0 (plus @1 @3)))
+#if GENERIC
+(simplify
+ (pointer_plus (convert:s (pointer_plus:s @0 @1)) @3)
+ (convert:type (pointer_plus @0 (plus @1 @3
+#endif
/* Pattern match
It does seem to me that niter analysis relies on statically detecting
not rolling loops, at least for the cases we assume no overflow happens
and we use number_of_iterations_lt_to_ne. I can't convince myself that
only INTEGER_CST appearant negative delta are problematic for
pointer types (which we always assume to have no overflow), but that would
be the most simplistic solution here. Still "negative" delta values should
be problematic for all cases, and since we only restrict us to constant
modulo, delta can also be non-constant.
[Bug tree-optimization/104931] wrong-code with number_of_iterations_lt_to_ne
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104931 --- Comment #2 from Richard Biener --- But maybe the code relies on a positive 'delta' - after all the same issue must exist for integer typed IVs. Probably most interesting cases get folded early in the correct way if they do not iterate. Given niter_type is unsigned_type_for (type) the use of FLOOR_MOD_EXPR seems pointless but would be required when doing a signed floor-mod as in the fix. Note there's tree niter_type = TREE_TYPE (step); tree mod = fold_build2 (FLOOR_MOD_EXPR, niter_type, *delta, step); .. if (integer_nonzerop (mod)) mod = fold_build2 (MINUS_EXPR, niter_type, step, mod); .. mpz_init (mmod); wi::to_mpz (wi::to_wide (mod), mmod, UNSIGNED); mpz_neg (mmod, mmod); so there's evidence that 'mod' is assumed to be positive in the end but 'delta' can be effectively signed here. Note 'step' is the absolute value of step, it's always (made) positive. The code has been this way since Zdenek wrote it. The proposed patch has passed bootstrap & testing (but I believe it's incomplete).
[Bug tree-optimization/104931] wrong-code with number_of_iterations_lt_to_ne
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104931
--- Comment #1 from Richard Biener ---
I think the issue is latent on trunk. The fix should be
diff --git a/gcc/tree-ssa-loop-niter.c b/gcc/tree-ssa-loop-niter.c
index b767056aeb0..21e25cabbd2 100644
--- a/gcc/tree-ssa-loop-niter.c
+++ b/gcc/tree-ssa-loop-niter.c
@@ -1171,7 +1171,17 @@ number_of_iterations_lt_to_ne (tree type, affine_iv
*iv0, affine_iv *iv1,
bool exit_must_be_taken, bounds *bnds)
{
tree niter_type = TREE_TYPE (step);
- tree mod = fold_build2 (FLOOR_MOD_EXPR, niter_type, *delta, step);
+ tree mod;
+ if (POINTER_TYPE_P (type))
+{
+ /* For pointers both step and delta have to be interpreted signed. */
+ mod = fold_build2 (FLOOR_MOD_EXPR, ssizetype,
+fold_convert (ssizetype, *delta),
+fold_convert (ssizetype, step));
+ mod = fold_convert (niter_type, mod);
+}
+ else
+mod = fold_build2 (FLOOR_MOD_EXPR, niter_type, *delta, step);
tree tmod;
mpz_t mmod;
tree assumption = boolean_true_node, bound, noloop;
[Bug tree-optimization/104931] wrong-code with number_of_iterations_lt_to_ne
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104931 Richard Biener changed: What|Removed |Added Last reconfirmed||2022-03-15 Known to fail||10.3.1, 11.2.1 Assignee|unassigned at gcc dot gnu.org |rguenth at gcc dot gnu.org Ever confirmed|0 |1 Keywords||wrong-code Known to work||12.0 Status|UNCONFIRMED |ASSIGNED
