Skip to content

Commit d8a88d7

Browse files
Ravenwaterclaude
andauthored
fix(cfloat): frexp returns a [0.5,1) fraction (std::frexp semantics) (#1029)
cfloat's frexp returned a fraction in [1,2) with *exp = scale() (floor(log2|x|)), which does not match std::frexp (fraction in [0.5,1), *exp = floor(log2|x|)+1) -- issue #1027. Generic code written to the std contract (and every other Universal type's frexp: dd, qd, ereal, bfloat16 all use [0.5,1)) saw the wrong exponent. Now: place the fraction at scale -1 (*exp = scale()+1). Extreme low-range configs (es <= 2, minimum normal exponent >= 0) cannot represent any value below 1.0 as a normal, so [0.5,1) is unachievable there; those fall back to the [1,2) fraction. ldexp is UNCHANGED -- it rebuilds the exponent from scale(), so the round-trip ldexp(frexp(x,&e),e) == x holds in every case. Also added the std special cases: +-0/inf/nan return unchanged with *exp = 0 (the old code corrupted them). Dependency sweep (the reason this is safe): the only callers of cfloat's frexp are round-trip tests, which are convention-agnostic; no generic/templated code instantiates cfloat's frexp expecting [1,2); manipulators/attributes/conversions do not use it. So the change is confined to frexp + its test. Test (fractional.cpp): assert the [0.5,1) fraction range for normal inputs where representable, plus frexp(0)/inf/nan special cases; added half and cfloat<16,8> coverage. (Note: a separate cfloat quirk -- isnormal() reports true for +-0 -- required excluding zero from the range assertion explicitly.) Verified gcc + clang: cfloat_fractional passes; the #1022 elreal oracle (which reads cfloat bits directly) is regression-clean. Resolves #1027 Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent c04efce commit d8a88d7

3 files changed

Lines changed: 59 additions & 18 deletions

File tree

elastic/elreal/arithmetic/exact_value_oracle.cpp

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -86,14 +86,15 @@ using namespace sw::universal;
8686
// * p > 53 with the Universal FP API (cfloat quad and up): read the encoding
8787
// DIRECTLY -- sign, scale, and the fbits stored fraction bits -- and assemble
8888
// value = (-1)^sign * (2^fbits + F) * 2^(scale - fbits), fbits = p - 1.
89-
// This deliberately avoids cfloat's wide-precision frexp and floor: cfloat
90-
// frexp normalises the mantissa to [1,2) (not std's [0.5,1)), and cfloat
91-
// floor mis-handles large integers (floor(2^100+8) != 2^100+8, it routes
92-
// through double) -- both filed separately. The earlier frexp/floor-based
93-
// extraction passed #1023's quad sweeps only because those fed double-derived
94-
// (<= 53-bit) q128 values; it silently mis-extracted genuine 113-bit values.
95-
// The bit-based path is verified consistent across widths and against an
96-
// independent 2x-wider cfloat product.
89+
// Reading the encoding directly keeps this oracle self-contained: it depends
90+
// on no cfloat math function, only on the bit layout. (Historically it also
91+
// side-stepped two wide-precision cfloat bugs that an earlier frexp/floor
92+
// extraction tripped over -- cfloat floor mis-handling large integers, #1026,
93+
// and cfloat frexp's non-std [1,2) fraction, #1027; both now fixed. That
94+
// earlier extraction had passed #1023's quad sweeps only because they fed
95+
// double-derived <= 53-bit q128 values, silently mis-extracting genuine
96+
// 113-bit ones.) The bit-based path is verified consistent across widths and
97+
// against an independent 2x-wider cfloat product.
9798
template <typename T>
9899
dyadic exact_real(T v) {
99100
if (v == T(0)) return dyadic();

include/sw/universal/number/cfloat/cfloat_impl.hpp

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4352,11 +4352,25 @@ constexpr inline bool operator>=(const cfloat<nbits, es, bt, hasSubnormals, hasM
43524352

43534353
// standard library functions for floating point
43544354

4355+
// frexp: decompose x into a normalized fraction and an integer power of two, so
4356+
// that x == fraction * 2^(*exp), following std::frexp semantics with the fraction
4357+
// in [0.5, 1). (Earlier this returned a [1,2) fraction with *exp = scale(), which
4358+
// did not match std::frexp -- issue #1027.)
43554359
template<unsigned nbits, unsigned es, typename bt, bool hasSubnormals, bool hasMaxExpValues, bool isSaturating>
43564360
constexpr inline cfloat<nbits, es, bt, hasSubnormals, hasMaxExpValues, isSaturating> frexp(const cfloat<nbits, es, bt, hasSubnormals, hasMaxExpValues, isSaturating>& x, int* exp) {
4357-
*exp = x.scale();
4358-
cfloat<nbits, es, bt, hasSubnormals, hasMaxExpValues, isSaturating> fraction(x);
4359-
fraction.setexponent(0);
4361+
using Cfloat = cfloat<nbits, es, bt, hasSubnormals, hasMaxExpValues, isSaturating>;
4362+
// std::frexp special cases: +-0, inf, and nan are returned unchanged with *exp = 0.
4363+
if (x.iszero() || x.isinf() || x.isnan()) { *exp = 0; return x; }
4364+
// Place the fraction at scale -1 so |fraction| lands in [0.5, 1) (std::frexp).
4365+
// A few extreme low-range configs (es <= 2, where the minimum normal exponent
4366+
// is >= 0) cannot represent any value below 1.0 as a normal, so [0.5,1) is not
4367+
// achievable; those fall back to the [1,2) fraction (scale 0). Either way the
4368+
// round-trip ldexp(frexp(x,&e),e) == x holds, since ldexp rebuilds the
4369+
// exponent from scale().
4370+
constexpr int targetScale = (std::numeric_limits<Cfloat>::min_exponent <= 0) ? -1 : 0;
4371+
*exp = x.scale() - targetScale; // scale() is floor(log2|x|); +1 for the [0.5,1) case
4372+
Cfloat fraction(x);
4373+
fraction.setexponent(targetScale);
43604374
return fraction;
43614375
}
43624376

static/float/cfloat/math/fractional.cpp

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,23 +19,43 @@ int VerifyCfloatFractionExponent(bool reportTestCases) {
1919
TestType a, b, c;
2020
int exp;
2121

22+
// std::frexp returns the fraction in [0.5, 1). cfloat can represent that range
23+
// only when its minimum normal exponent reaches at most -1; the extreme es<=2
24+
// configs cannot, and frexp falls back to a [1,2) fraction there (still a valid
25+
// round-trip). Assert the [0.5,1) range only where it is representable.
26+
constexpr bool checkRange = (std::numeric_limits<TestType>::min_exponent <= 0);
2227
for (size_t i = 1; i < NR_TEST_CASES; ++i) {
2328
a.setbits(i);
29+
if (a.isnan() || a.isinf()) continue;
2430
b = frexp(a, &exp);
2531
c = ldexp(b, exp);
26-
// std::cout << "input : " << to_binary(a) << " : " << a << '\n';
27-
// std::cout << "frexp : " << to_binary(b) << " : " << b << '\n';
28-
// std::cout << "ldexp : " << to_binary(c) << " : " << c << '\n';
29-
if (a != c) {
30-
if (a.isnan() && c.isnan()) continue; // (s)nan != (s)nan, so the regular equivalance test fails
32+
if (a != c) { // round-trip must always hold
3133
nrOfFailedTests++;
3234
if (reportTestCases) ReportOneInputFunctionError("FAIL", "frexp/ldexp", a, b, c);
3335
}
34-
else {
35-
// if (reportTestCases) ReportOneInputFunctionError("PASS", "frexp/ldexp", a, b, c);
36+
// (cfloat isnormal() also reports true for +-0, so exclude zero explicitly)
37+
if (checkRange && a.isnormal() && !a.iszero()) { // std::frexp fraction range, where representable
38+
double fb = std::abs(double(b));
39+
if (!(fb >= 0.5 && fb < 1.0)) {
40+
nrOfFailedTests++;
41+
if (reportTestCases) std::cout << "frexp range FAIL: " << to_binary(a) << " -> fraction " << b << " (|f|=" << fb << ")\n";
42+
}
3643
}
3744
if (nrOfFailedTests > 24) return 25;
3845
}
46+
// special cases: +-0, inf, nan return unchanged with exp == 0
47+
{
48+
TestType z(0); int e = -99; TestType f = frexp(z, &e);
49+
if (!f.iszero() || e != 0) { ++nrOfFailedTests; if (reportTestCases) std::cout << "frexp(0) FAIL: f=" << f << " e=" << e << '\n'; }
50+
}
51+
if (std::numeric_limits<TestType>::has_infinity) {
52+
TestType inf; inf.setinf(false); int e = 0; TestType f = frexp(inf, &e);
53+
if (!f.isinf()) { ++nrOfFailedTests; if (reportTestCases) std::cout << "frexp(inf) FAIL\n"; }
54+
}
55+
if (std::numeric_limits<TestType>::has_quiet_NaN) {
56+
TestType nan; nan.setnan(); int e = 0; TestType f = frexp(nan, &e);
57+
if (!f.isnan()) { ++nrOfFailedTests; if (reportTestCases) std::cout << "frexp(nan) FAIL\n"; }
58+
}
3959
return nrOfFailedTests;
4060
}
4161

@@ -470,6 +490,12 @@ try {
470490
nrOfFailedTestCases += ReportTestResult(
471491
VerifyCfloatFractionExponent < cfloat<8, 4, uint8_t, true, true, false> >(reportTestCases), type_tag(cfloat<8, 4, uint8_t, true, true, false>()), "frexp/ldexp");
472492

493+
// wider configs exercise the std::frexp [0.5,1) fraction range (issue #1027)
494+
nrOfFailedTestCases += ReportTestResult(
495+
VerifyCfloatFractionExponent < half >(reportTestCases), type_tag(half()), "frexp/ldexp");
496+
nrOfFailedTestCases += ReportTestResult(
497+
VerifyCfloatFractionExponent < cfloat<16, 8, uint16_t, true, false, false> >(reportTestCases), type_tag(cfloat<16, 8, uint16_t, true, false, false>()), "frexp/ldexp");
498+
473499
nrOfFailedTestCases += ReportTestResult(
474500
VerifyCfloatFmod < cfloat<8, 4, uint8_t, true, false, false> >(reportTestCases), type_tag(cfloat<8, 4, uint8_t, true, false, false>()), "fmod");
475501

0 commit comments

Comments
 (0)