On Wed, 20 Oct 2021, Andre Vieira (lists) wrote: > > On 19/10/2021 00:22, Joseph Myers wrote: > > On Fri, 15 Oct 2021, Richard Biener via Gcc-patches wrote: > > > >> On Fri, Sep 24, 2021 at 2:59 PM Jirui Wu via Gcc-patches > >> <gcc-patches@gcc.gnu.org> wrote: > >>> Hi, > >>> > >>> Ping: https://gcc.gnu.org/pipermail/gcc-patches/2021-August/577846.html > >>> > >>> The patch is attached as text for ease of use. Is there anything that > >>> needs to change? > >>> > >>> Ok for master? If OK, can it be committed for me, I have no commit rights. > >> I'm still not sure about the correctness. I suppose the > >> flag_fp_int_builtin_inexact && !flag_trapping_math is supposed to guard > >> against spurious inexact exceptions, shouldn't that be > >> !flag_fp_int_builtin_inexact || !flag_trapping_math instead? > > The following remarks may be relevant here, but are not intended as an > > assertion of what is correct in this case. > > > > 1. flag_fp_int_builtin_inexact is the more permissive case ("inexact" may > > or may not be raised). All existing uses in back ends are > > "flag_fp_int_builtin_inexact || !flag_trapping_math" or equivalent. > > > > 2. flag_fp_int_builtin_inexact only applies to certain built-in functions > > (as listed in invoke.texi). It's always unspecified, even in C2X, whether > > casts of non-integer values from floating-point to integer types raise > > "inexact". So flag_fp_int_builtin_inexact should not be checked in insn > > patterns corresponding to simple casts from floating-point to integer, > > only in insn patterns corresponding to the built-in functions listed for > > -fno-fp-int-builtin-inexact in invoke.texi (or for operations that combine > > such a built-in function with a cast of the *result* to integer type). > Hi, > > I agree with Joseph, I don't think we should be checking > flag_fp_int_builtin_inexact here because we aren't transforming the math > function 'trunc', but rather a piece of C-code that has trunc-like semantics.
But we are generating 'trunc' which may now raise a spurious exception. OTOH flag_fp_int_builtin_inexact wouldn't help here because "may or may not" stil may raise spurious exception flags. > As for flag_trapping_math, it's definition says 'Assume floating point > operations can trap'. I assume IFN_TRUNC would not trap, since I don't think > IFN_TRUNC will preserve the overflow behaviour, in the cases where the FP > value is bigger than the intermediate integer type range. So I think we should > prevent the transformation if we are assuming the FP instructions can trap. Note trap == set exception flags, not only raise a trap. > If we don't assume the FP instructions can trap, then I think it's fine to > ignore the overflow as this behavior is undefined in C. > > Also changed the comment. Slightly different to your suggestion Richard, in an > attempt to be more generic. Do you still have concerns regarding the checks? I think your updated patch is OK. Thanks, Richard.