> On 1 Oct 2024, at 14:27, Richard Biener <rguent...@suse.de> wrote: > > External email: Use caution opening links or attachments > > > On Tue, 1 Oct 2024, Jennifer Schmitz wrote: > >> This patch implements 4 rules for logarithmic identities in match.pd >> under -funsafe-math-optimizations: >> 1) logN(1.0/a) -> -logN(a). This avoids the division instruction. >> 2) logN(C/a) -> logN(C) - logN(a), where C is a real constant. Same as 1). >> 3) logN(a) + logN(b) -> logN(a*b). This reduces the number of calls to >> log function. >> 4) logN(a) - logN(b) -> logN(a/b). Same as 4). >> Tests were added for float, double, and long double. >> >> The patch was bootstrapped and regtested on aarch64-linux-gnu and >> x86_64-linux-gnu, no regression. >> Additionally, SPEC 2017 fprate was run. While the transform does not seem >> to be triggered, we also see no non-noise impact on performance. >> OK for mainline? > > Since log can set errno we have the builtins affect global memory and > thus have VDEFs, this posses issues for match.pd which does not assign > new VDEFs upon materializing the result, esp. for the case where > you duplicate a call. There's a similar issue for -frounding-math > where intermediate FP status changes can be lost. match.pd simply > follows the SSA use-def chains without regarding memory side-effects. > > The transforms are guarded by flag_unsafe_math_optimizations but here > I think we need !HONOR_SIGN_DEPENDENT_ROUNDING, !flag_trapping_math > (exception state might be different for logN(a) - logN(b) -> logN(a/b), > at least WRT INEXACT?), and !flag_errno_math (because of the VDEFs). > > + /* Simplify logN(C/a) into logN(C)-logN(a). */ > + (simplify > + (logs (rdiv:s REAL_CST@0 @1)) > + (minus (logs @0) (logs @1))) > > I think you want > > (minus (logs! @0) (logs @1)) > > here to make sure we constant-fold. > > + (simplify > + (minus (logs:s @0) (logs:s @1)) > + (logs (rdiv @0 @1)))) > > I think that's somewhat dangerous for @1 == 0 given log for > zero arg results in -HUGE_VAL but a FP division by gives a NaN. > I'm not exactly sure whether !HONOR_INFINITIES && !HONOR_NANS > is good enough here. > > Your testcases probably all trigger during GENERIC folding, > bypassing the VDEF issue - you might want to try assigning > the comparison operands to tempoaries to run into the actual > issues. Dear Richard, Thanks for the review and suggesting the additional flags. I added - !HONOR_SIGN_DEPENDENT_ROUNDING - !flag_trapping_math - !flag_errno_math - !HONOR_INFINITIES - !HONOR_NANS as guard before the patterns. Can we add anything else to account for HUGE_VAL or will !HONOR_INFINITIES && !HONOR_NANS be enough? Or do you have a suggestion how I can check this? I validated again on aarch64 and x86_64. Best, Jennifer
0001-PR86710-PR116826-match.pd-Fold-logarithmic-identitie.patch
Description: Binary data
> > Thanks, > Richard. > > >> Signed-off-by: Jennifer Schmitz <jschm...@nvidia.com> >> >> gcc/ >> * match.pd: Fold logN(1.0/a) -> -logN(a), >> logN(C/a) -> logN(C) - logN(a), logN(a) + logN(b) -> logN(a*b), >> and logN(a) - logN(b) -> logN(a/b). >> >> gcc/testsuite/ >> * gcc.dg/tree-ssa/log_ident.c: New test. >> > > -- > Richard Biener <rguent...@suse.de> > SUSE Software Solutions Germany GmbH, > Frankenstrasse 146, 90461 Nuernberg, Germany; > GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
smime.p7s
Description: S/MIME cryptographic signature
