On Tue, Jun 3, 2025 at 4:19 PM Spencer Abson <spencer.ab...@arm.com> wrote:
>
> On Tue, Jun 03, 2025 at 03:26:40PM +0200, Richard Biener wrote:
> > On Tue, Jun 3, 2025 at 3:09 PM Spencer Abson <spencer.ab...@arm.com> wrote:
> > >
> > > Floating-point to integer conversions can be inexact or invalid (e.g.,
> > > due to
> > > overflow or NaN). However, since users of operation_could_trap_p infer
> > > the
> > > bool FP_OPERATION argument from the expression's type, the FIX_TRUNC
> > > family
> > > are considered non-trapping here.
> > >
> > > This patch handles them explicitly.
> > >
> > > gcc/ChangeLog:
> > >
> > > * tree-eh.cc (operation_could_trap_helper_p): Cover FIX_TRUNC
> > > expressions explicitly.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > > * gcc.target/aarch64/sve/pr96357.c: Change to avoid producing
> > > a conditional FIX_TRUNC_EXPR, whilst still reproducing the bug
> > > in PR96357.
> > > * gcc.dg/tree-ssa/ifcvt-fix-trunc-1.c: New test.
> > > * gcc.dg/tree-ssa/ifcvt-fix-trunc-2.c: Likewise.
> > > ---
> > > .../gcc.dg/tree-ssa/ifcvt-fix-trunc-1.c | 19 +++++++++++++++++++
> > > .../gcc.dg/tree-ssa/ifcvt-fix-trunc-2.c | 6 ++++++
> > > .../gcc.target/aarch64/sve/pr96357.c | 8 ++++----
> > > gcc/tree-eh.cc | 7 +++++++
> > > 4 files changed, 36 insertions(+), 4 deletions(-)
> > > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/ifcvt-fix-trunc-1.c
> > > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/ifcvt-fix-trunc-2.c
> > >
> > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ifcvt-fix-trunc-1.c
> > > b/gcc/testsuite/gcc.dg/tree-ssa/ifcvt-fix-trunc-1.c
> > > new file mode 100644
> > > index 00000000000..801a53fa30b
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.dg/tree-ssa/ifcvt-fix-trunc-1.c
> > > @@ -0,0 +1,19 @@
> > > + /* { dg-do compile } */
> > > + /* { dg-options "-O2 -ftree-vectorize -fdump-tree-ifcvt-stats" } */
> > > +
> > > +void
> > > +test (int *dst, float *arr, int *pred, int n)
> > > +{
> > > + for (int i = 0; i < n; i++)
> > > + {
> > > + int pred_i = pred[i];
> > > + float arr_i = arr[i];
> > > +
> > > + dst[i] = pred_i ? (int)arr_i : 5;
> > > + }
> > > +}
> > > +
> > > +/* We expect this to fail if_convertible_loop_p so long as we have no
> > > + conditional IFN for FIX_TRUNC_EXPR. */
> > > +
> > > +/* { dg-final { scan-tree-dump-times "Applying if-conversion" 0 "ifcvt"
> > > } } */
> > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ifcvt-fix-trunc-2.c
> > > b/gcc/testsuite/gcc.dg/tree-ssa/ifcvt-fix-trunc-2.c
> > > new file mode 100644
> > > index 00000000000..628b754e94d
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.dg/tree-ssa/ifcvt-fix-trunc-2.c
> > > @@ -0,0 +1,6 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-options "-O2 -ftree-vectorize -fno-trapping-math
> > > -fdump-tree-ifcvt-stats" } */
> > > +
> > > +#include "ifcvt-fix-trunc-1.c"
> > > +
> > > +/* { dg-final { scan-tree-dump-times "Applying if-conversion" 1 "ifcvt"
> > > } } */
> > > diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr96357.c
> > > b/gcc/testsuite/gcc.target/aarch64/sve/pr96357.c
> > > index 9a7f912e529..6dd0409f3c8 100644
> > > --- a/gcc/testsuite/gcc.target/aarch64/sve/pr96357.c
> > > +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr96357.c
> > > @@ -5,10 +5,10 @@ int d;
> > >
> > > void
> > > f1(char f, char *g, char *h, char *l, char *n) {
> > > - double i = d, j = 1.0 - f, k = j ? d : j;
> > > - if (k == 1.0)
> > > - i = 0.0;
> >
> > So why does if-conversion not work with the previous variant?
>
> The body of the previous variant of f1 becomes (after ch_vect):
>
> ...
>
> double _70;
>
> ...
>
> _34 = d.0_22 == 1;
> _35 = j_26 != 0.0;
> _36 = _34 & _35;
> if (_36 != 0)
> goto <bb 5>; [17.00%]
> else
> goto <bb 4>; [83.00%]
>
> <bb 4> [local count: 882293657]:
> _70 = i_23 * 5.0e-1;
> _72 = (char) _70;
>
> <bb 5> [local count: 1063004408]:
> # prephitmp_73 = PHI <0(3), _72(4)>
>
> ...
>
> The interesting expression being _72 = (char) _70. if-conversion would have
> previously converted this part to:
>
> _34 = d.0_22 == 1;
> _35 = j_26 != 0.0;
> _36 = _34 & _35;
> _77 = ~_36;
> _70 = .COND_MUL (_77, i_23, 5.0e-1, i_23);
> _72 = (char) _70;
> prephitmp_73 = _36 ? 0 : _72;
>
> Which is invalid given FIX_TRUNC_EXPR can trap. We have no predicated form
> of this expression, so it fails if_convertible_gimple_assign_stmt_p with this
> patch applied.
Ah OK, that seems like a correct thing.
> The change to f1 gets rid of the FIX_TRUNC_EXPR.
And that looks reasonable - the truncation to 'char' doesn't seem very practical
relevant.
The patch is OK.
Thanks,
Richard.
> Thanks,
> Spencer
> >
> > > - *l = *n = *g = *h = i * 0.5;
> > > + double j = 1.0 - f, k = j ? d : j;
> > > +
> > > + char i = (k == 1.0) ? 10 : 50;
> > > + *l = *n = *g = *h = i;
> > > }
> > >
> > > void
> > > diff --git a/gcc/tree-eh.cc b/gcc/tree-eh.cc
> > > index a4d59954c05..8cc81ebcf5e 100644
> > > --- a/gcc/tree-eh.cc
> > > +++ b/gcc/tree-eh.cc
> > > @@ -2538,6 +2538,13 @@ operation_could_trap_helper_p (enum tree_code op,
> > > /* Constructing an object cannot trap. */
> > > return false;
> > >
> > > + case FIX_TRUNC_EXPR:
> > > + case VEC_PACK_FIX_TRUNC_EXPR:
> > > + case VEC_UNPACK_FIX_TRUNC_HI_EXPR:
> > > + case VEC_UNPACK_FIX_TRUNC_LO_EXPR:
> > > + /* The FIX_TRUNC family are always potentially trapping. */
> > > + return flag_trapping_math;
> > > +
> > > case COND_EXPR:
> > > case VEC_COND_EXPR:
> > > /* Whether *COND_EXPR can trap depends on whether the
> > > --
> > > 2.34.1
> > >
