On Fri, Aug 2, 2024 at 2:43 PM Juergen Christ <[email protected]> wrote:
>
> Do not convert floats to ints in multiple step if trapping math is
> enabled. This might hide some inexact signals.
>
> Also use correct sign (the sign of the target integer type) for the
> intermediate steps. This only affects undefined behaviour (casting
> floats to unsigned datatype where the float is negative).
>
> gcc/ChangeLog:
>
> * tree-vect-stmts.cc (vectorizable_conversion): multi-step
> float to int conversion only with trapping math and correct
> sign.
>
> Signed-off-by: Juergen Christ <[email protected]>
>
> Bootstrapped and tested on x84 and s390. Ok for trunk?
>
> ---
> gcc/tree-vect-stmts.cc | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc
> index fdcda0d2abae..2ddd13383193 100644
> --- a/gcc/tree-vect-stmts.cc
> +++ b/gcc/tree-vect-stmts.cc
> @@ -5448,7 +5448,8 @@ vectorizable_conversion (vec_info *vinfo,
> break;
>
> cvt_type
> - = build_nonstandard_integer_type (GET_MODE_BITSIZE (rhs_mode), 0);
> + = build_nonstandard_integer_type (GET_MODE_BITSIZE (rhs_mode),
> + TYPE_UNSIGNED (lhs_type));
But lhs_type should be a float type here, the idea that for a
FLOAT_EXPR (int -> float)
a signed integer type is the natural one to use - as it's 2x wider
than the original
RHS type it's signedness doesn't matter. Note all float types should be
!TYPE_UNSIGNED so this hunk is a no-op but still less clear on the intent IMO.
Please drop it.
> cvt_type = get_same_sized_vectype (cvt_type, vectype_in);
> if (cvt_type == NULL_TREE)
> goto unsupported;
> @@ -5505,10 +5506,11 @@ vectorizable_conversion (vec_info *vinfo,
> if (GET_MODE_SIZE (lhs_mode) >= GET_MODE_SIZE (rhs_mode))
> goto unsupported;
>
> - if (code == FIX_TRUNC_EXPR)
> + if (code == FIX_TRUNC_EXPR && !flag_trapping_math)
> {
> cvt_type
> - = build_nonstandard_integer_type (GET_MODE_BITSIZE (rhs_mode), 0);
> + = build_nonstandard_integer_type (GET_MODE_BITSIZE (rhs_mode),
> + TYPE_UNSIGNED (lhs_type));
Here it might be relevant for correctness - we have to choose between
sfix and ufix for the float -> [u]int conversion.
Do you have a testcase? Shouldn't the exactness be independent of the integer
type we convert to?
> cvt_type = get_same_sized_vectype (cvt_type, vectype_in);
> if (cvt_type == NULL_TREE)
> goto unsupported;
> --
> 2.43.5
>
