Re: WHY? commit ac76bc1145dd7f4476e5d982ce8f355f71015713

On Fri, Nov 05, 2021 at 03:14:46PM -0700, Steve Kargl wrote:
> On Fri, Nov 05, 2021 at 10:32:26PM +0100, Dimitry Andric wrote:
> > On 5 Nov 2021, at 21:13, Steve Kargl <sgk@troutmask.apl.washington.edu> wrote:
> > > 
> > > Why was this committed?
> > > 
> > > commit ac76bc1145dd7f4476e5d982ce8f355f71015713
> > > Author: Dimitry Andric <dim@FreeBSD.org>
> > > Date:   Tue Feb 9 22:06:51 2021 +0100
> > > 
> > >    Fix lib/msun's ctrig_test/test_inf_inputs test case with clang >= 10
> > > 
> > >    This sprinkles a few strategic volatiles in an attempt to defeat clang's
> > >    optimization interfering with the expected floating-point exception
> > >    flags.
> > > 
> > > There is nothing, and I mean, nothing strategic about "sprinkling"
> > > volatile onto the declaration of "float x, y, h;"  These variables
> > > are referenced in all floating pointing operations in the file,
> > > which means that there are needless reloading of x, y, and h
> > > from memory.
> > 
> > There was more context in https://bugs.freebsd.org/244732, but the gist
> > was that with clang >= 10, ctanh() and ctanhf() had FE_INVALID set after
> > calling them with {inf,inf}. The reasons for this were obscure to me at
> > the time, since it regressed with an llvm commit that seemed to have
> > very little to do with floating point.
> > 
> > However, in 3b00222f156d we added -fp-exception-behavior=maytrap to
> > clang's compile flags for lib/msun, for https://bugs.freebsd.org/254911,
> > to force it to use stricter floating point semantics. This turns out to
> > also make the admittedly ugly volatile fixes unnecessary.
> > 
> > So I have reverted ac76bc1145dd (minus the ctrig_test.c part) in:
> > https://cgit.freebsd.org/src/commit/?id=e2157cd0000f6dbb6465d7a885f2dcfd4d3596cb
> > 
> 
> Thanks for the explanation!  This would indeed be strange.
> The evaluation of ctanhf does not invoke ccoshf(z).
> 
> The relevant section of code from s_ctanh.c (similar code in s_ctanhf.c)
> is 
> 
> 	if (ix >= 0x7ff00000) {
> 		if ((ix & 0xfffff) | lx)	/* x is NaN */
> 			return (CMPLX(nan_mix(x, y),
> 			    y == 0 ? y : nan_mix(x, y)));
> 		SET_HIGH_WORD(x, hx - 0x40000000);	/* x = copysign(1, x) */
> 		return (CMPLX(x, copysign(0, isinf(y) ? y : sin(y) * cos(y))));
> 	}
> 
> The testcase should get to the second return statement.  So,
> either isinf(y) or copysign(0, inf) was raising the exception.
> 

I just scanned the llvm report pointed to in the FreeBSD bug
report.  The llvm report discusses changes in speculative
execution.  If clang was evaluating isinf(y) and either of
sin(y) or cos(y), speculatively, then yes, FE_INVALID will
be raised.

% tlibm_libm -e cos
cosf( 0) =  1.  Got no exception.  Expected 1 without exception.
cosf(-0) =  1.  Got no exception.  Expected 1 without exception.
cosf( inf) = nan.  Got FE_INVALID.  Expected nan with FE_INVALID.
cosf(-inf) = nan.  Got FE_INVALID.  Expected nan with FE_INVALID.
cos( 0) =  1.  Got no exception.  Expected 1 without exception.
cos(-0) =  1.  Got no exception.  Expected 1 without exception.
cos( inf) = nan.  Got FE_INVALID.  Expected nan with FE_INVALID.
cos(-inf) = nan.  Got FE_INVALID.  Expected nan with FE_INVALID.
cosl( 0) =  1.  Got no exception.  Expected 1 without exception.
cosl(-0) =  1.  Got no exception.  Expected 1 without exception.
cosl( inf) = nan.  Got FE_INVALID.  Expected nan with FE_INVALID.
cosl(-inf) = nan.  Got FE_INVALID.  Expected nan with FE_INVALID.

-- 
Steve