Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> When trying to associate (v + INT_MAX) + INT_MAX we are using
> the TREE_OVERFLOW bit to check for correctness. That isn't
> working for VECTOR_CSTs and it can't in general when one considers
> VL vectors. It looks like it should work for COMPLEX_CSTs but
> I didn't try to single out _Complex int in this change.
>
> The following makes sure that for vectors we use the fallback of
> using unsigned arithmetic when associating the above to
> v + (INT_MAX + INT_MAX).
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, OK?
>
> Thanks,
> Richard.
>
> PR middle-end/110495
> * tree.h (TREE_OVERFLOW): Do not mention VECTOR_CSTs
> since we do not set TREE_OVERFLOW on those since the
> introduction of VL vectors.
> * match.pd (x +- CST +- CST): For VECTOR_CST do not look
> at TREE_OVERFLOW to determine validity of association.
>
> * gcc.dg/tree-ssa/addadd-2.c: Amend.
> * gcc.dg/tree-ssa/forwprop-27.c: Adjust.
> ---
> gcc/match.pd | 9 +++++----
> gcc/testsuite/gcc.dg/tree-ssa/addadd-2.c | 1 +
> gcc/testsuite/gcc.dg/tree-ssa/forwprop-27.c | 4 +++-
> gcc/tree.h | 2 +-
> 4 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index f09583bbcac..d193a572005 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -3025,7 +3025,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> (with { tree cst = const_binop (outer_op == inner_op
> ? PLUS_EXPR : MINUS_EXPR,
> type, @1, @2); }
> - (if (cst && !TREE_OVERFLOW (cst))
> + (if (INTEGRAL_TYPE_P (type) && cst && !TREE_OVERFLOW (cst))
> (inner_op @0 { cst; } )
> /* X+INT_MAX+1 is X-INT_MIN. */
> (if (INTEGRAL_TYPE_P (type) && cst
> @@ -3037,7 +3037,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> (view_convert (inner_op
> (view_convert:utype @0)
> (view_convert:utype
> - { drop_tree_overflow (cst); }))))))))))))))
> + { TREE_OVERFLOW (cst)
> + ? drop_tree_overflow (cst) : cst; }))))))))))))))
It looks like the whole “(with …)” expects cst to be nonnull,
but the “last resort” doesn't check it (unless I'm misreading).
Would it be easier to add a top-level “if (cst)”? (Obviously
a preexisting thing.)
>
> /* (CST1 - A) +- CST2 -> CST3 - A */
> (for outer_op (plus minus)
> @@ -3049,7 +3050,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> forever if something doesn't simplify into a constant. */
> (if (!CONSTANT_CLASS_P (@0))
> (minus (outer_op! (view_convert @1) @2) (view_convert @0)))
> - (if (!ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0))
> + (if (!INTEGRAL_TYPE_P (TREE_TYPE (@0))
> || TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0)))
> (view_convert (minus (outer_op! @1 (view_convert @2)) @0))
> (if (types_match (type, @0) && !TYPE_OVERFLOW_SANITIZED (type))
> @@ -3068,7 +3069,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> forever if something doesn't simplify into a constant. */
> (if (!CONSTANT_CLASS_P (@0))
> (plus (view_convert @0) (minus! @1 (view_convert @2))))
> - (if (!ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0))
> + (if (!INTEGRAL_TYPE_P (TREE_TYPE (@0))
> || TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0)))
> (view_convert (plus @0 (minus! (view_convert @1) @2)))
> (if (types_match (type, @0) && !TYPE_OVERFLOW_SANITIZED (type))
I didn't understand this part. Doesn't it mean that we allow
overflow-inducing reassociations for all vector integer types,
albeit not immediately folded away?
Also, why do we keep the:
!ANY_INTEGRAL_TYPE_P (type) || TYPE_OVERFLOW_WRAPS (type)
in the outer ifs?
But that's just me not understanding match.pd very well.
Feel free to ignore if it's nonsense. :)
Thanks,
Richard
