Thank you Richard, I misunderstood your request to “always state how you tested a patch”. I improved the test description in the patch comment, instead.
I tested the patch on both x86_64-pc-linux-gnu and aarch64-arm64-linux-gnu. Since I have another patch under review, I’ll write a follow-up email on that thread with this information. Thanks again, Matteo > On Sep 11, 2025, at 1:19 PM, Richard Biener <richard.guent...@gmail.com> > wrote: > > 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 >> >> >> >> >> >>
