diff mbox

[edk2] StdLib: the long standing build error

Message ID 0877601216922E4B83A7129715B5DA2BAC50E117CD@GEORGE.Emea.Arm.com
State New
Headers show

Commit Message

Olivier Martin Aug. 5, 2014, 10:31 p.m. UTC
Why do not put '*va_arg(ap, long double *) = res;' which seems to be the valid solution and add a #ifdef for the faulty toolchains.
I do not feel fair to keep a workaround by default when it blocks all the other toolchains.

Comments

Laszlo Ersek Aug. 6, 2014, 1:28 p.m. UTC | #1
On 08/06/14 00:31, Olivier Martin wrote:
> Why do not put '*va_arg(ap, long double *) = res;' which seems to be the valid solution and add a #ifdef for the faulty toolchains.
> I do not feel fair to keep a workaround by default when it blocks all the other toolchains.

Last night I wanted to understand why this issue exists at all... Daryl
said in the thread before,

"The Microsoft compiler does not really support (long double)"

I was uncapable of imagining any valid reason for this, so I googled...
The most authentic response I could find was:

http://forums.codeguru.com/showthread.php?390950-RESOLVED-When-is-80bit-long-double-coming-back

One June 17th, 2006, Ronald Laeremans, then Acting Product Unit Manager
of the Visual C++ Team, said:

> There are no current plans to add 80 bit FP support.
>
> The major reason is that FP code generation has been switching to the
> use of SSE/SSE2/SSE3 instruction sets instead of the x87 FP stack
> since that is what both the AMD and Intel recent and future chip
> generations are focusing their performance efforts on. These
> instruction sets only support 32 and 64 bit FP formats.

Maybe I'm mistaken and incorrectly connecting independent issues, but
this certainly sounds like a joke. Just because the 80-bit extended
precision is gone, it's fully possible to support the standard C type
*long double*. There's no requirement for long double to differ
(internally) from double (64-bit) at all.

If Microsoft compilers fail to provide a va_arg that works with long
double (whatever the representation of long double), then those
compilers aren't even C89-compliant.

(If you check normative Annex F in the C99 std, it says

  F.1 Introduction

  1 [...] An implementation that defines _ _STDC_IEC_559_ _ shall
    conform to the specifications in this annex. [...]

  F.2 Types

  1 The C floating types match the IEC 60559 formats as follows:
    — The float type matches the IEC 60559 single format.
    — The double type matches the IEC 60559 double format.
    — The long double type matches an IEC 60559 extended format [...]
      else a non-IEC 60559 extended format, else the IEC 60559 double
      format.

It's officially allowed to have a long double that matches double.)

Edk2 could use conditional preprocessing directives to find out about
the properties of long double, and if it seems to match double, then
just do whatever workarounds are necessary. The directive could examine
the precise toolchain (as Olivier suggests), and/or check:

5.2.4.2.2 Characteristics of floating types <float.h>

  5 All integer values in the <float.h> header, except FLT_ROUNDS,
    shall be constant expressions suitable for use in #if preprocessing
    directives [...]

  [...]

  8 The values given in the following list shall be replaced by
    constant expressions with implementation-defined values that are
    greater or equal in magnitude (absolute value) to those shown, with
    the same sign:

    [...]

    - number of base-FLT_RADIX digits in the floating-point
      significand, /p/

      [...]
      LDBL_MANT_DIG
      [...]

Similarly the exponent range is visible from LDBL_MIN_EXP and LDBL_MAX_EXP.

Consider the following test program, called "ldbl.c":

-----------------------
#include <stdio.h>
#include <float.h>

int
main(void)
{
  fprintf(stdout, "LDBL_MANT_DIG=%d LDBL_MIN_EXP=%d LDBL_MAX_EXP=%d\n",
    (int)LDBL_MANT_DIG, (int)LDBL_MIN_EXP, (int)LDBL_MAX_EXP);
  return 0;
}
-----------------------

Let's compile it with 64-bit and 80-bit long doubles:

$ gcc -o ldbl-64 -mlong-double-64 ldbl.c
$ gcc -o ldbl-80 -mlong-double-80 ldbl.c

The output is:

$ ./ldbl-64
LDBL_MANT_DIG=53 LDBL_MIN_EXP=-1021 LDBL_MAX_EXP=1024

$ ./ldbl-80
LDBL_MANT_DIG=64 LDBL_MIN_EXP=-16381 LDBL_MAX_EXP=16384

Again, these are constant expressions suitable for #if directives.

And, StdLib already depends on the build system's <float.h> header:

$ git grep -l -F '<float.h>' -- StdLib

StdLib/Include/stddef.h
StdLib/LibC/Main/Ipf/flt_rounds.c
StdLib/LibC/Stdio/vfwprintf.c
StdLib/LibC/gdtoa/ldtoa.c

I suggest to recognize in the code that Microsoft compilers are broken
in this regard, and punish ^W special-case *them*, not the conformant ones.

Thanks,
Laszlo

------------------------------------------------------------------------------
Infragistics Professional
Build stunning WinForms apps today!
Reboot your WinForms applications with our WinForms controls. 
Build a bridge from your legacy apps to the future.
http://pubads.g.doubleclick.net/gampad/clk?id=153845071&iu=/4140/ostg.clktrk
diff mbox

Patch

--- a/StdLib/LibC/Stdio/vfscanf.c
+++ b/StdLib/LibC/Stdio/vfscanf.c
@@ -843,12 +843,8 @@  literal:
         goto match_failure;
       if ((flags & SUPPRESS) == 0) {
         if (flags & LONGDBL) {
-          long double **mp = (long double **)ap;
           long double res = strtold(buf, &p);
-
-          *(*mp) = res;
-          ap += sizeof(long double *);
-/*???*/   //*va_arg(ap, long double *) = res;
+          *va_arg(ap, long double *) = res;
         } else if (flags & LONG) {
           double res = strtod(buf, &p);
           *va_arg(ap, double *) = res;