On Tue, Sep 9, 2025 at 10:39 PM Matteo Nicoli
<matteo.nicoli...@gmail.com> wrote:
>
> Dear Richard,
>
> You are right, there was a typo in my patch. I removed the duplicate */ and
> updated the description of the test in the commit description.
Thanks. I have bootstrapped and tested this on x86_64-unknown-linux-gnu
(please always state how you tested a patch).
The changelog needed some minor adjustments:
PR tree-optimization/121595
* gcc/match.pd:(fabs(a + 0.0) -> fabs (a)): Optimization pattern
limited to
the -fno-trapping-math case.
gcc/ should be dropped and indenting for multi-line entries is to the column
of the '*', not the ':'.
All lines should be indented by a tab.
* gcc.dg/fabs-plus-zero-1.c: With -ftrapping-math (default behavior),
the '+0.0' must be preserved.
should simply say: New testcase.
I have pushed the change with this adjustments as r16-3802-gaa4aafbad5235f
Richard.
> Best regards,
> Matteo
>
>
>
> On 5 Sep 2025, at 9:27 am, Richard Biener <richard.guent...@gmail.com> wrote:
>
> On Thu, Sep 4, 2025 at 4:15 PM Matteo Nicoli <matteo.nicoli...@gmail.com>
> wrote:
>
>
> Dear Richard,
>
> No, I don’t have access to gcc git. Anyway, I updated the patch description
> and attached here the new patch.
>
>
> Please always state how you tested a patch. This one doesn't compile because:
>
> ../../gcc/gcc/match.pd:2050:56 error: expected (, got *
> Otherwise !HONOR_SNANS would be sufficient here. */*/
> ^
> make[3]: *** [Makefile:3053: s-generic-match] Error 1
>
> @@ -2045,6 +2045,15 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> (abs (negate @0))
> (abs @0))
>
> +/* fabs(x + 0.0) -> fabs(x), safe even with signed zeros when
> -fno-trapping-math.
> + With non-default exception handling denormal + 0.0 might trap.
> + Otherwise !HONOR_SNANS would be sufficient here. */*/
> +(for op (plus minus)
>
> duplicate */ here.
>
> Richard.
>
> Best regards,
> Matteo
>
>
>
> On Sep 2, 2025, at 1:57 PM, Richard Biener <richard.guent...@gmail.com> wrote:
>
> On Fri, Aug 29, 2025 at 2:20 PM Matteo Nicoli
> <matteo.nicoli...@gmail.com> wrote:
>
>
> Here’s the patch with the modified comment before the rule in match.pd
>
>
> OK. Do you have git access? Btw, the ChangeLog is currently wrong:
>
> Subject: [PATCH] tree-optimization/121595 - the optimization fabs(a+0.0) ->
> fabs(a) should apply only for non trapping case
>
> * added a match expression
> - gcc/match.pd
> * added tests
> - gcc/testsuite/gcc.dg/fabs-plus-zero-1.c
> - gcc/testsuite/gcc.dg/fabs-plus-zero-2.c
>
>
> there's a missing patch description and the changlog part should look like
> this:
>
> PR tree-optimization/121595
> * match.pd (fabs(a + 0.0) -> fabs (a)): New pattern.
>
> * gcc.dg/fabs-plus-zero-1.c: New testcase.
> * gcc.dg/fabs-plus-zero-2.c: Likewise.
>
>
>
>
> On Aug 29, 2025, at 12:53 PM, Richard Biener <richard.guent...@gmail.com>
> wrote:
>
> On Fri, Aug 29, 2025 at 10:56 AM Matteo Nicoli
> <matteo.nicoli...@gmail.com> wrote:
>
>
> Dear Richard,
>
>
> It can trap with sNaN ± 0.0. ±Inf ± 0.0 = ±Inf, so it does not raise an
> FE_OVERFLOW (because there’s no overflow of a finite quantity), and qNaN does
> not raise an FE_INVALID because it’s quiet.
>
>
> There’s already a check for sNaN in fold-const.cc
>
>
> /* Don't allow the fold with -fsignaling-nans. */
> if (arg ? tree_expr_maybe_signaling_nan_p (arg) : HONOR_SNANS (type))
>
> return false;
>
>
> As far as I know, the purpose of this bug fix was to suppress this specific
> optimization when the program is compiled with -ftrapping-math flag.
>
>
> So we've discussed on IRC and the conclusion was that with default
> exception handling !HONOR_SNANS would be sufficient
> but alternate exception handling might trap on x + 0.0 when x is
> denormal. Ideally we'd have a separate flag for
> non-default exception handling, but as-is we don't.
>
> Please add a comment like
>
> /* With non-default exception handling denormal + 0.0 might trap.
> Otherwise !HONOR_SNANS would be sufficient here. */
>
> The patch is OK with that change.
>
> Richard.
>
>
>
>
> Best regards,
>
> Matteo
>
>
>
>
> On Aug 28, 2025, at 10:43 AM, Richard Biener <richard.guent...@gmail.com>
> wrote:
>
> On Sat, Aug 23, 2025 at 11:56 PM Matteo Nicoli
> <matteo.nicoli...@gmail.com> wrote:
>
>
> Dear reviewers,
>
> I attached a patch for bug 121595
> (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=121595). I signed it, and added
> `Reviewed-by: Andrew Pinski <andrew.pin...@oss.qualcomm.com>` (here in CC).
>
>
> +/* fabs(x + 0.0) -> fabs(x), safe even with signed zeros when
> -fno-trapping-math. */
> +(for op (plus minus)
> + (simplify
> + (abs (op @0 real_zerop@1))
> + (if (!flag_trapping_math)
> + (abs @0))))
>
> so forgive my ignorance, possibly IEEE abs() never raises FP exceptions
> (unless operating on sNaN?)? But does Inf + 0.0 raise FE_OVERFLOW?
> Does NaN + 0.0 raise FE_INVALID?
>
> So what I wonder is whether !HONOR_SNANS (@0) would be enough to check?
>
> I refrained from trusting AI on those questions ...
>
> Richard.
>
> Best regards,
> Matteo
>
>
>
>
>
>
