> On Aug 28, 2016, at 01:49, Bruce Evans <b...@optusnet.com.au> wrote:
... I agree. This commit in effect papered over the problem. More investigation will be done with the PR that introduced the expected failure. Thanks! -Ngie > > This can't depend on 64-bitness. It might depend on FLT_EPSILON, but > IEEE might require a specific representation of floats and we only have > and only support one. > > This is probably a bug in the tests that shows up on arches with extra > precision. Perhaps just a complier bug. > >> Modified: stable/11/tests/sys/kern/acct/acct_test.c >> ============================================================================== >> --- stable/11/tests/sys/kern/acct/acct_test.c Sun Aug 28 07:09:45 2016 >> (r304946) >> +++ stable/11/tests/sys/kern/acct/acct_test.c Sun Aug 28 07:10:48 2016 >> (r304947) >> @@ -204,7 +204,10 @@ ATF_TC_BODY(encode_tv_random_million, tc >> struct timeval tv; >> long k; >> >> - atf_tc_expect_fail("the testcase violates FLT_EPSILON"); >> +#ifdef __LP64__ >> + atf_tc_expect_fail("the testcase violates FLT_EPSILON on 64-bit " >> + "platforms, e.g. amd64"); >> +#endif >> >> ATF_REQUIRE_MSG(unsetenv("TZ") == 0, "unsetting TZ failed; errno=%d", >> errno); > > The rest of the function is: > > X for (k = 1; k < 1000000L; k++) { > X tv.tv_sec = random(); > X tv.tv_usec = (random() % 1000000L); > X v.c = encode_timeval(tv); > X check_result(atf_tc_get_ident(tc), > X (float)tv.tv_sec * AHZ + tv.tv_usec, v); > X } > > AHZ here is less than an obfuscation of literal 10000000 or just 1e6 or > 1e6F. It doesn't even have the style bug of an L suffix like the nearby > literals. Types are important here, but the L isn't. > > AHZ used to be a constant related to fixed-point conversions in acct.h. > It used to have value 1000. Note much like the AHZ. <sys/acct.h> now > devfines AHZV1 and this has value 64. This is for a legacy API. Not > very compatible. > > This file doesn't include <sys/acct.h> except possibly via namespace > pollution, so it doesn't get any AHZ*. It only uses AHZ to convert > tv_sec to usec. This was magical and may be broken. The file convert.c > is included. This is a converted copy of kern_acct.c. Back when AHZ > existed in acct.h, kern_acct.c used to use AHZ with its different value. > I don't see how overriding that value either didn't cause a redefinition > error or inconsistencies. Now kern_acct.c doesn't use either AHZ* so > this definition is not magical. > > So AHZ should be replaced by literal 1000000 except possibly for type > problems. IIRC, C99 specifies the dubious behaviour of converting > integers to float in float expressions to support the dubious behaviour > of evaluating float expressions in float precision. 1000000 is even > exactly representable in float precision. But the result of the > mutliplication isn't in general. Adding a small tv_usec to a not > very large tv_sec converted to usec is almost certain to not be > representable in a 32-bit float after a few random choices. So > we expect innacuracies. > > The float expression may be evaluated in extra precision, and is on > i386. So we expect smaller inaccuracies on i386. > > It is not clear if remaining bugs are in the test or the compiler. > Probably both. The test asks for inaccuracies and gets them in the > expression sometimes. It doesn't try to force the inaccuracies by > casting to float, and only C90+ compilers do this cast as specified > since the specification specifies behaviour that is too pessimal to > use. C90+ compilers are in short supply, but gcc later than aout > 4.6 properlay pessimizes the cast when instructed to by certain > compiler flags. > > But the test it calls a function which should do the conversion. It > takes excessive inlining combined with the de-pessimization to not > do the conversion. Apparently, clang does do the excessive inlining. > Otherwise the result would be the same on i386 as on amd64. > > The test seems to be quite broken. encode_timeval() does some > conversion which is presumably correct. We calculate a value in > usec to compare against. This is only done in float precision > (possibly higher, but we don't control this). We expect a relative > error of about FLT_EPSILON in this. Later we convert to a relative > error, so this is only slightly broken. encode_timeval() must > have a rounding error, and our calculation has one and the scaling > has more. So we should expect errors of several times FLT_EPSILON. > So the test only seems to be slightly broken. Strictly less than > FLT_EPSILON is too strict if the calculations are actually done in > float precision since it is too difficult to calculate the reference > and do the scaling without increasing the error. The worst case > for the reference is tv_sec = 2**31-1 (31 bits) and tv_usec = 999999 > (20 bits). That is exactly representable in 53 bits (double precision) > so we should use that. > > Bruce _______________________________________________ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"