Message ID | 20230821003237.376935-1-richard.henderson@linaro.org |
---|---|
State | New |
Headers | show |
Series | softfloat: Handle m68k extended precision denormals properly | expand |
On 8/20/23 18:24, Keith Packard wrote: > >> Motorola treats denormals with explicit integer bit set as >> having unbiased exponent 0, unlike Intel which treats it as >> having unbiased exponent 1 (like all other IEEE formats). > > Thanks for having a look at this. Your patch fixes a couple of cases, > but there are further adventures that await if you're interested. > > x: 0x1p0 0x3fff 0x80000000 0x00000000 > y: 0x1p-16383 0x0000 0x80000000 0x00000000 > build_mul: 0x1p-16382 0x0000 0x80000000 0x00000000 > runtime_mul: 0x1p-16383 0x0001 0x80000000 0x00000000 > > I think the enclosed additional patch fixes this. I've still got 75 fmal > failures on this target, but the obvious 'multiply is broken' problem > appears fixed. > > From b722c92f8329f56f5243496eca3779f1156aff4f Mon Sep 17 00:00:00 2001 > From: Keith Packard <keithp@keithp.com> > Date: Sun, 20 Aug 2023 18:20:13 -0700 > Subject: [PATCH] softfloat: Handle m68k LDBL_MIN_EXP normal values > > Unlike Intel 80-bit floats, Motorola allows for normal values with a > zero exponent. Handle that by not setting exponent to 1 when the value > is normal for this format. > > Signed-off-by: Keith Packard <keithp@keithp.com> > --- > fpu/softfloat-parts.c.inc | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fpu/softfloat-parts.c.inc b/fpu/softfloat-parts.c.inc > index d0c43c28fb..cea854cdf1 100644 > --- a/fpu/softfloat-parts.c.inc > +++ b/fpu/softfloat-parts.c.inc > @@ -288,7 +288,7 @@ static void partsN(uncanon_normal)(FloatPartsN *p, float_status *s, > p->frac_lo &= ~round_mask; > } > > - exp = (p->frac_hi & DECOMPOSED_IMPLICIT_BIT) != 0; > + exp = (p->frac_hi & DECOMPOSED_IMPLICIT_BIT) != 0 && !fmt->m68k_denormal; That does look like a correct change. I'll fold it in. Please let us know if you encounter anything else. r~
On 21/8/23 02:32, Richard Henderson wrote: > Motorola treats denormals with explicit integer bit set as > having unbiased exponent 0, unlike Intel which treats it as > having unbiased exponent 1 (like all other IEEE formats). > > Add a flag on FloatFmt to differentiate the behaviour. > > Reported-by: Keith Packard <keithp@keithp.com> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > fpu/softfloat.c | 9 +++++- > tests/tcg/m68k/denormal.c | 53 ++++++++++++++++++++++++++++++++++ > fpu/softfloat-parts.c.inc | 5 ++-- > tests/tcg/m68k/Makefile.target | 2 +- > 4 files changed, 65 insertions(+), 4 deletions(-) > create mode 100644 tests/tcg/m68k/denormal.c Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
On Sun, 20 Aug 2023, Richard Henderson wrote: > Motorola treats denormals with explicit integer bit set as > having unbiased exponent 0, unlike Intel which treats it as > having unbiased exponent 1 (like all other IEEE formats). > > Add a flag on FloatFmt to differentiate the behaviour. > > Reported-by: Keith Packard <keithp@keithp.com> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > fpu/softfloat.c | 9 +++++- > tests/tcg/m68k/denormal.c | 53 ++++++++++++++++++++++++++++++++++ > fpu/softfloat-parts.c.inc | 5 ++-- > tests/tcg/m68k/Makefile.target | 2 +- > 4 files changed, 65 insertions(+), 4 deletions(-) > create mode 100644 tests/tcg/m68k/denormal.c > > diff --git a/fpu/softfloat.c b/fpu/softfloat.c > index 0cc130ae9b..3adfa8cee0 100644 > --- a/fpu/softfloat.c > +++ b/fpu/softfloat.c > @@ -517,6 +517,7 @@ typedef struct { > * round_mask: bits below lsb which must be rounded > * The following optional modifiers are available: > * arm_althp: handle ARM Alternative Half Precision > + * m68k_denormal: explicit integer bit for extended precision may be 1 Is there a better name for this such as unbiased_exp or something telling it's an integer and maybe also somehow avoiding the ! when using it? This naming seems a bit unspecific and confusing. Regards, BALATON Zoltan > */ > typedef struct { > int exp_size; > @@ -526,6 +527,7 @@ typedef struct { > int frac_size; > int frac_shift; > bool arm_althp; > + bool m68k_denormal; > uint64_t round_mask; > } FloatFmt; > > @@ -576,7 +578,12 @@ static const FloatFmt float128_params = { > static const FloatFmt floatx80_params[3] = { > [floatx80_precision_s] = { FLOATX80_PARAMS(23) }, > [floatx80_precision_d] = { FLOATX80_PARAMS(52) }, > - [floatx80_precision_x] = { FLOATX80_PARAMS(64) }, > + [floatx80_precision_x] = { > + FLOATX80_PARAMS(64), > +#ifdef TARGET_M68K > + .m68k_denormal = true, > +#endif > + }, > }; > > /* Unpack a float to parts, but do not canonicalize. */ > diff --git a/tests/tcg/m68k/denormal.c b/tests/tcg/m68k/denormal.c > new file mode 100644 > index 0000000000..599dafa663 > --- /dev/null > +++ b/tests/tcg/m68k/denormal.c > @@ -0,0 +1,53 @@ > +/* > + * Test m68k extended double denormals. > + */ > + > +#include <stdio.h> > +#include <stdint.h> > + > +#define X0 0x1p+16383l > +#define Y0 0x1p-16446l > +#define X1 0x1.1p-8223l > +#define Y1 0x1.1p-8224l > + > +static volatile long double test[2][3] = { > + { X0, Y0, X0 * Y0 }, > + { X1, Y1, X1 * Y1 }, > +}; > + > +static void dump_ld(const char *label, long double ld) > +{ > + union { > + long double d; > + struct { > + uint32_t exp:16; > + uint32_t space:16; > + uint32_t h; > + uint32_t l; > + }; > + } u; > + > + u.d = ld; > + printf("%12s: % -27La 0x%04x 0x%08x 0x%08x\n", label, u.d, u.exp, u.h, u.l); > +} > + > +int main(void) > +{ > + int i, err = 0; > + > + for (i = 0; i < 2; ++i) { > + long double x = test[i][0]; > + long double y = test[i][1]; > + long double build_mul = test[i][2]; > + long double runtime_mul = x * y; > + > + if (runtime_mul != build_mul) { > + dump_ld("x", x); > + dump_ld("y", y); > + dump_ld("build_mul", build_mul); > + dump_ld("runtime_mul", runtime_mul); > + err = 1; > + } > + } > + return err; > +} > diff --git a/fpu/softfloat-parts.c.inc b/fpu/softfloat-parts.c.inc > index 527e15e6ab..d0c43c28fb 100644 > --- a/fpu/softfloat-parts.c.inc > +++ b/fpu/softfloat-parts.c.inc > @@ -118,7 +118,8 @@ static void partsN(canonicalize)(FloatPartsN *p, float_status *status, > } else { > int shift = frac_normalize(p); > p->cls = float_class_normal; > - p->exp = fmt->frac_shift - fmt->exp_bias - shift + 1; > + p->exp = fmt->frac_shift - fmt->exp_bias > + - shift + !fmt->m68k_denormal; > } > } else if (likely(p->exp < fmt->exp_max) || fmt->arm_althp) { > p->cls = float_class_normal; > @@ -256,7 +257,7 @@ static void partsN(uncanon_normal)(FloatPartsN *p, float_status *s, > is_tiny = !frac_addi(&discard, p, inc); > } > > - frac_shrjam(p, 1 - exp); > + frac_shrjam(p, !fmt->m68k_denormal - exp); > > if (p->frac_lo & round_mask) { > /* Need to recompute round-to-even/round-to-odd. */ > diff --git a/tests/tcg/m68k/Makefile.target b/tests/tcg/m68k/Makefile.target > index 1163c7ef03..6ff214e60a 100644 > --- a/tests/tcg/m68k/Makefile.target > +++ b/tests/tcg/m68k/Makefile.target > @@ -4,7 +4,7 @@ > # > > VPATH += $(SRC_PATH)/tests/tcg/m68k > -TESTS += trap > +TESTS += trap denormal > > # On m68k Linux supports 4k and 8k pages (but 8k is currently broken) > EXTRA_RUNS+=run-test-mmap-4096 # run-test-mmap-8192 >
diff --git a/fpu/softfloat.c b/fpu/softfloat.c index 0cc130ae9b..3adfa8cee0 100644 --- a/fpu/softfloat.c +++ b/fpu/softfloat.c @@ -517,6 +517,7 @@ typedef struct { * round_mask: bits below lsb which must be rounded * The following optional modifiers are available: * arm_althp: handle ARM Alternative Half Precision + * m68k_denormal: explicit integer bit for extended precision may be 1 */ typedef struct { int exp_size; @@ -526,6 +527,7 @@ typedef struct { int frac_size; int frac_shift; bool arm_althp; + bool m68k_denormal; uint64_t round_mask; } FloatFmt; @@ -576,7 +578,12 @@ static const FloatFmt float128_params = { static const FloatFmt floatx80_params[3] = { [floatx80_precision_s] = { FLOATX80_PARAMS(23) }, [floatx80_precision_d] = { FLOATX80_PARAMS(52) }, - [floatx80_precision_x] = { FLOATX80_PARAMS(64) }, + [floatx80_precision_x] = { + FLOATX80_PARAMS(64), +#ifdef TARGET_M68K + .m68k_denormal = true, +#endif + }, }; /* Unpack a float to parts, but do not canonicalize. */ diff --git a/tests/tcg/m68k/denormal.c b/tests/tcg/m68k/denormal.c new file mode 100644 index 0000000000..599dafa663 --- /dev/null +++ b/tests/tcg/m68k/denormal.c @@ -0,0 +1,53 @@ +/* + * Test m68k extended double denormals. + */ + +#include <stdio.h> +#include <stdint.h> + +#define X0 0x1p+16383l +#define Y0 0x1p-16446l +#define X1 0x1.1p-8223l +#define Y1 0x1.1p-8224l + +static volatile long double test[2][3] = { + { X0, Y0, X0 * Y0 }, + { X1, Y1, X1 * Y1 }, +}; + +static void dump_ld(const char *label, long double ld) +{ + union { + long double d; + struct { + uint32_t exp:16; + uint32_t space:16; + uint32_t h; + uint32_t l; + }; + } u; + + u.d = ld; + printf("%12s: % -27La 0x%04x 0x%08x 0x%08x\n", label, u.d, u.exp, u.h, u.l); +} + +int main(void) +{ + int i, err = 0; + + for (i = 0; i < 2; ++i) { + long double x = test[i][0]; + long double y = test[i][1]; + long double build_mul = test[i][2]; + long double runtime_mul = x * y; + + if (runtime_mul != build_mul) { + dump_ld("x", x); + dump_ld("y", y); + dump_ld("build_mul", build_mul); + dump_ld("runtime_mul", runtime_mul); + err = 1; + } + } + return err; +} diff --git a/fpu/softfloat-parts.c.inc b/fpu/softfloat-parts.c.inc index 527e15e6ab..d0c43c28fb 100644 --- a/fpu/softfloat-parts.c.inc +++ b/fpu/softfloat-parts.c.inc @@ -118,7 +118,8 @@ static void partsN(canonicalize)(FloatPartsN *p, float_status *status, } else { int shift = frac_normalize(p); p->cls = float_class_normal; - p->exp = fmt->frac_shift - fmt->exp_bias - shift + 1; + p->exp = fmt->frac_shift - fmt->exp_bias + - shift + !fmt->m68k_denormal; } } else if (likely(p->exp < fmt->exp_max) || fmt->arm_althp) { p->cls = float_class_normal; @@ -256,7 +257,7 @@ static void partsN(uncanon_normal)(FloatPartsN *p, float_status *s, is_tiny = !frac_addi(&discard, p, inc); } - frac_shrjam(p, 1 - exp); + frac_shrjam(p, !fmt->m68k_denormal - exp); if (p->frac_lo & round_mask) { /* Need to recompute round-to-even/round-to-odd. */ diff --git a/tests/tcg/m68k/Makefile.target b/tests/tcg/m68k/Makefile.target index 1163c7ef03..6ff214e60a 100644 --- a/tests/tcg/m68k/Makefile.target +++ b/tests/tcg/m68k/Makefile.target @@ -4,7 +4,7 @@ # VPATH += $(SRC_PATH)/tests/tcg/m68k -TESTS += trap +TESTS += trap denormal # On m68k Linux supports 4k and 8k pages (but 8k is currently broken) EXTRA_RUNS+=run-test-mmap-4096 # run-test-mmap-8192
Motorola treats denormals with explicit integer bit set as having unbiased exponent 0, unlike Intel which treats it as having unbiased exponent 1 (like all other IEEE formats). Add a flag on FloatFmt to differentiate the behaviour. Reported-by: Keith Packard <keithp@keithp.com> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- fpu/softfloat.c | 9 +++++- tests/tcg/m68k/denormal.c | 53 ++++++++++++++++++++++++++++++++++ fpu/softfloat-parts.c.inc | 5 ++-- tests/tcg/m68k/Makefile.target | 2 +- 4 files changed, 65 insertions(+), 4 deletions(-) create mode 100644 tests/tcg/m68k/denormal.c