Bug 30013 - Multiple flaws in decimal floating-point arithmetic conversions fixed
Summary: Multiple flaws in decimal floating-point arithmetic conversions fixed
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 4.3.0
: P3 normal
Target Milestone: ---
Assignee: Janis Johnson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-11-29 02:02 UTC by beebe
Modified: 2007-10-23 16:50 UTC (History)
2 users (show)

See Also:
Host: x86_64-unknown-linux-gnu
Target: x86_64-unknown-linux-gnu
Build: x86_64-unknown-linux-gnu
Known to work:
Known to fail:
Last reconfirmed: 2006-11-29 02:45:38


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description beebe 2006-11-29 02:02:52 UTC
Testing of my decimal floating-point arithmetic code with gcc-4.2 and
gcc-4.3 releases turned up several bugs in gcc's type conversions that
I've had to track down and fix.

They repair several problems:

(1) Loss of trailing digits by failure to provide a suitable precision
    in conversion formats; I use the values from the formula in David
    Matula's CACM 19(3) 716--723 June 1968 paper, expressed by this
    code snippet:

	### matula(nbits): return number of decimal digits needed to ensure
	### correct round-trip conversion between binary and decimal
	func matula(nbits) { return ceil(nbits/log2(10) + 1) }

(2) Failure to provide prototypes of library functions that are not
    necessarily supplied by system header files: if the gcc developers
    would routinely use the gcc -Werror-implicit-function-declaration
    option in builds, this problem could have, and should have, been
    caught before release.

(3) Incorrect data types for conversion functions.

(4) Failure to match sprintf() argument types with format items.

(5) Serious loss of precision for _Decimal128 by using double strtod()
    instead of long double strtold().

The test file given below can be used to check the conversions before
and after the patch.

Here are diffs suitable for input to patch:

103 airy> diff gcc/config/dfp-bit.c.org gcc/config/dfp-bit.c
516c516
<   sprintf (buf, BFP_FMT, x);
---
>   sprintf (buf, BFP_FMT, (double)x);

104 airy> diff gcc/config/dfp-bit.h.org gcc/config/dfp-bit.h
241c241
< #define BFP_FMT "%e"
---
> #define BFP_FMT "%.9e"
246c246
< #define BFP_FMT "%e"
---
> #define BFP_FMT "%.17e"
253,255c253,257
< #define BFP_FMT "%e"
< #define BFP_VIA_TYPE double
< #define STR_TO_BFP strtod
---
> #define BFP_FMT "%.36Le"
> #define BFP_VIA_TYPE long double
> /* strtold is declared in <stdlib.h> only for C99.  */
> extern long double strtold (const char *, char **);
> #define STR_TO_BFP strtold
422c424
< typedef float DFtype __attribute__ ((mode (DF)));
---
> typedef double DFtype __attribute__ ((mode (DF)));
424c426
< typedef float XFtype __attribute__ ((mode (XF)));
---
> typedef long double XFtype __attribute__ ((mode (XF)));

Here is the test file:

% cat dec-cast-bug.c
/***********************************************************************
[23-Nov-2006]
***********************************************************************/

#include <stdio.h>
#include <stdlib.h>
#include <math.h>

void
test(long double x)
{
    float f;
    double d;
    long double y;
    _Decimal32 d32;
    _Decimal64 d64;
    _Decimal128 d128;

    d32 = (_Decimal32)x;
    (void)printf(" (_Decimal32)(%.21Lg) = 0x%08x\n", x,
		 ((unsigned int *)(&d32))[0]);

    d64 = (_Decimal64)x;
    (void)printf(" (_Decimal64)(%.21Lg) = 0x%08x_%08x\n", x,
		 ((unsigned int *)(&d64))[1],
		 ((unsigned int *)(&d64))[0]);

    d128 = (_Decimal128)x;
    (void)printf("(_Decimal128)(%.21Lg) = 0x%08x_%08x_%08x_%08x\n", x,
		 ((unsigned int *)(&d128))[3],
		 ((unsigned int *)(&d128))[2],
		 ((unsigned int *)(&d128))[1],
		 ((unsigned int *)(&d128))[0]);

    (void)printf("\n");

    f = (float)x;
    (void)printf("      (float)(%.21Lg) = %.9g             = %a\n", x, (double)f, (double)f);

    d = (double)x;
    (void)printf("     (double)(%.21Lg) = %.17g     = %a\n", x, (double)d, (double)d);

    y = (long double)x;
    (void)printf("(long double)(%.21Lg) = %.21Lg = %La\n", x, (long double)y, (long double)y);

    (void)printf("\n");

    y = (long double)(float)x;
    (void)printf("      (long double)(float)(%.21Lg) = %.21Lg = %La\n", x, y, y);

    y = (long double)(double)x;
    (void)printf("     (long double)(double)(%.21Lg) = %.21Lg = %La\n", x, y, y);

    y = (long double)(long double)x;
    (void)printf("(long double)(long double)(%.21Lg) = %.21Lg = %La\n", x, y, y);

    (void)printf("\n");

    f = (float)(_Decimal32)x;
    (void)printf(" (float)(_Decimal32)(%.21Lg) = %.9g = %a\n", x, (double)f, (double)f);

    f = (float)(_Decimal64)x;
    (void)printf(" (float)(_Decimal64)(%.21Lg) = %.9g = %a\n", x, (double)f, (double)f);

    f = (float)(_Decimal128)x;
    (void)printf("(float)(_Decimal128)(%.21Lg) = %.9g = %a\n", x, (double)f, (double)f);

    (void)printf("\n");

    d = (double)(_Decimal32)x;
    (void)printf(" (double)(_Decimal32)(%.21Lg) = %.17g = %a\n", x, (double)d, (double)d);

    d = (double)(_Decimal64)x;
    (void)printf(" (double)(_Decimal64)(%.21Lg) = %.17g = %a\n", x, (double)d, (double)d);

    d = (double)(_Decimal128)x;
    (void)printf("(double)(_Decimal128)(%.21Lg) = %.17g = %a\n", x, (double)d, (double)d);

    (void)printf("\n");

    y = (long double)(_Decimal32)x;
    (void)printf(" (long double)(_Decimal32)(%.21Lg) = %.21Lg = %La\n", x, (long double)y, (long double)y);

    y = (long double)(_Decimal64)x;
    (void)printf(" (long double)(_Decimal64)(%.21Lg) = %.21Lg = %La\n", x, (long double)y, (long double)y);

    y = (long double)(_Decimal128)x;
    (void)printf("(long double)(_Decimal128)(%.21Lg) = %.21Lg = %La\n", x, (long double)y, (long double)y);

    (void)printf("\n");

    y = (long double)(float)(_Decimal32)x;
    (void)printf(" (long double)(float)(_Decimal32)(%.21Lg) = %.21Lg = %La\n", x, y, y);

    y = (long double)(float)(_Decimal64)x;
    (void)printf(" (long double)(float)(_Decimal64)(%.21Lg) = %.21Lg = %La\n", x, y, y);

    y = (long double)(float)(_Decimal128)x;
    (void)printf("(long double)(float)(_Decimal128)(%.21Lg) = %.21Lg = %La\n", x, y, y);

    (void)printf("\n");

    y = (long double)(double)(_Decimal32)x;
    (void)printf(" (long double)(double)(_Decimal32)(%.21Lg) = %.21Lg = %La\n", x, y, y);

    y = (long double)(double)(_Decimal64)x;
    (void)printf(" (long double)(double)(_Decimal64)(%.21Lg) = %.21Lg = %La\n", x, y, y);

    y = (long double)(double)(_Decimal128)x;
    (void)printf("(long double)(double)(_Decimal128)(%.21Lg) = %.21Lg = %La\n", x, y, y);

    (void)printf("\n");

    y = (long double)(_Decimal32)x;
    (void)printf(" (long double)(_Decimal32)(%.21Lg) = %.21Lg = %La\n", x, y, y);

    y = (long double)(_Decimal64)x;
    (void)printf(" (long double)(_Decimal64)(%.21Lg) = %.21Lg = %La\n", x, y, y);

    y = (long double)(_Decimal128)x;
    (void)printf("(long double)(_Decimal128)(%.21Lg) = %.21Lg = %La\n", x, y, y);
}

int
main(int argc, char* argv[])
{
    test(3.141592653589793238462643383279502884L);

    return (EXIT_SUCCESS);
}
Comment 1 Andrew Pinski 2006-11-29 02:13:24 UTC
422c424
< typedef float DFtype __attribute__ ((mode (DF)));
---
> typedef double DFtype __attribute__ ((mode (DF)));
424c426
< typedef float XFtype __attribute__ ((mode (XF)));
---
> typedef long double XFtype __attribute__ ((mode (XF)));



This part really should not matter as mode tells the compiler what the real type is.
Comment 2 Andrew Pinski 2006-11-29 02:13:59 UTC
> /* strtold is declared in <stdlib.h> only for C99.  */
> extern long double strtold (const char *, char **);
> #define STR_TO_BFP strtold

Well use -std=gnu99 then.
Comment 3 Andrew Pinski 2006-11-29 02:57:00 UTC
PS the testcase violates C aliasing rules.
Comment 4 Janis Johnson 2007-08-17 00:40:08 UTC
Ben just mentioned this PR to me, I hadn't seen it before.

I'm working on a patch to support TFmode for powerpc*-linux, and I'll talk to HJ about proper support for XFmode.  Initially we didn't support long double because  we were testing on powerpc64-linux where there was no C library support for 128-bit long double.  Now that there is, we can support it.

Thanks for the test, I'll modify it so it doesn't violate aliasing rules and add it to the testsuite.  Please report other problems you find.
Comment 5 Janis Johnson 2007-08-17 20:44:17 UTC
Why use "%.9e", "%.17e", and "%.36Le" to write the binary float values to a string, instead of using lengths of FLT_DIG, DBL_DIG, and LDBL_DIG?  For i686-linux those are 6, 15, and 18.
Comment 6 Janis Johnson 2007-09-11 01:11:29 UTC
Subject: Bug 30013

Author: janis
Date: Tue Sep 11 01:11:16 2007
New Revision: 128361

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=128361
Log:
gcc/
	PR c/30013
	* config/dfp-bit.c: Don't skip TFmode conversions; move strto*
	declarations to top.
	(DFP_TO_BFP): Use for either XFmode or TFmode.
	(BFP_TO_DFP): Use for either XFmode or TFmode; always use cast
	of BFP_VIA_TYPE.
	* config/dfp-bit.h: Include float.h.
	(LONG_DOUBLE_HAS_XF_MODE, LONG_DOUBLE_HAS_TF_MODE): Define if long
	double is one of these modes, rather than using LIBGCC_HAS_*F_MODE
	which doesn't mean the same thing.
	(BFP_KIND): Use 4 to mean TFmode.
	(BFP_FMT): Specify the number of decimal digits based on the
	number of mantissa digits.
	(BFP_VIA_TYPE): Binary float type to use as cast for sprintf.
	(BFP_TO_DFP, DFP_TO_BFP): Define names for TFmode variants.
	(STR_TO_BFP): Use strtold for XFmode or TFmode.
	(TFtype): Define if TFmode is supported.
	* doc/libgcc.texi (Decimal float library routines): Document
	TF conversion functions.

gcc/testsuite/
	* gcc.dg/dfp/convert-bfp.c: Replace SKIP_LONG_DOUBLE with runtime
	checks for size of long double.
	* gcc.dg/dfp/convert.h: New file.
	* gcc.dg/dfp/convert-bfp-2.c: New test.
	* gcc.dg/dfp/convert-bfp-3.c: Ditto.
	* gcc.dg/dfp/convert-bfp-4.c: Ditto.
	* gcc.dg/dfp/convert-bfp-5.c: Ditto.
	* gcc.dg/dfp/convert-bfp-6.c: Ditto.
	* gcc.dg/dfp/convert-bfp-7.c: Ditto.
	* gcc.dg/dfp/convert-bfp-8.c: Ditto.
	* gcc.dg/dfp/convert-bfp-9.c: Ditto.
	* gcc.dg/dfp/convert-bfp-10.c: Ditto.
	* gcc.dg/dfp/convert-bfp-11.c: Ditto.

Added:
    trunk/gcc/testsuite/gcc.dg/dfp/convert-bfp-10.c
    trunk/gcc/testsuite/gcc.dg/dfp/convert-bfp-11.c
    trunk/gcc/testsuite/gcc.dg/dfp/convert-bfp-2.c
    trunk/gcc/testsuite/gcc.dg/dfp/convert-bfp-3.c
    trunk/gcc/testsuite/gcc.dg/dfp/convert-bfp-4.c
    trunk/gcc/testsuite/gcc.dg/dfp/convert-bfp-5.c
    trunk/gcc/testsuite/gcc.dg/dfp/convert-bfp-6.c
    trunk/gcc/testsuite/gcc.dg/dfp/convert-bfp-7.c
    trunk/gcc/testsuite/gcc.dg/dfp/convert-bfp-8.c
    trunk/gcc/testsuite/gcc.dg/dfp/convert-bfp-9.c
    trunk/gcc/testsuite/gcc.dg/dfp/convert.h
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/dfp-bit.c
    trunk/gcc/config/dfp-bit.h
    trunk/gcc/doc/libgcc.texi
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gcc.dg/dfp/convert-bfp.c

Comment 7 Janis Johnson 2007-10-23 16:50:37 UTC
Fixed in what will be 4.3.0.