www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.ldc - MSVC: fix for failing tests due to strtod+hexadecimal literals

reply "Johan Engelen" <j j.nl> writes:
Hi all,
   MSVC's strtod does not deal correctly with hexadecimal floating 
point literals, resulting a few unittests failing. For example, 
the following program

import std.math, std.stdio;
void main() {
     writeln(PI);
}

outputs "0" (zero), simply because std.math.PI is defined using a 
hex literal and LDC2 compiled with MSVC cannot deal with that.
See: https://github.com/ldc-developers/ldc/issues/761

I added
#if _MSC_VER <= 1800
     
VersionCondition::addPredefinedGlobalIdent("MSVC_STRTOD_NOHEXREALS");
#endif
to main.cpp, and then in std.math I defined PI and friends using 
decimal floating point literals for 
version(MSVC_STRTOD_NOHEXREALS).

Is this a desired fix? (i.e. trying to make a better build with 
MSVC while we wait for the new version)

One positive effect of this change is that it un-hides some other 
failing tests.

thanks for comments,
   Johan
Mar 01 2015
next sibling parent reply "Kevin Brogan" <kevin brogan.ca> writes:
On Sunday, 1 March 2015 at 21:00:53 UTC, Johan Engelen wrote:

 Hi all,
   MSVC's strtod does not deal correctly with hexadecimal 
 floating point literals, resulting a few unittests failing. For 
 example, the following program

 import std.math, std.stdio;
 void main() {
     writeln(PI);
 }

 outputs "0" (zero), simply because std.math.PI is defined using 
 a hex literal and LDC2 compiled with MSVC cannot deal with that.
 See: https://github.com/ldc-developers/ldc/issues/761

 I added
 #if _MSC_VER <= 1800
     
 VersionCondition::addPredefinedGlobalIdent("MSVC_STRTOD_NOHEXREALS");
 #endif
 to main.cpp, and then in std.math I defined PI and friends 
 using decimal floating point literals for 
 version(MSVC_STRTOD_NOHEXREALS).

 Is this a desired fix? (i.e. trying to make a better build with 
 MSVC while we wait for the new version)

 One positive effect of this change is that it un-hides some 
 other failing tests.

 thanks for comments,
   Johan


Why use hex constants to begin with? Why not just use the 
floating point equivalent? The compiler will just substitute with 
the binary representation that is closest will it not?
Mar 01 2015
parent "Kai Nacke" <kai redstar.de> writes:
On Monday, 2 March 2015 at 00:23:33 UTC, Kevin Brogan wrote:

 Why use hex constants to begin with? Why not just use the 
 floating point equivalent? The compiler will just substitute 
 with the binary representation that is closest will it not?


Hi Kevin,

it is a very complicated problem to create a floating point 
number from the usual decimal scientific representation, e.g. 
3.1415. I think a good reading is 
http://dl.acm.org/citation.cfm?id=93557.
The hex representation is far easier to parse.

Regards,
Kai
Mar 01 2015
prev sibling parent reply "Kai Nacke" <kai redstar.de> writes:
Hi Johan!

On Sunday, 1 March 2015 at 21:00:53 UTC, Johan Engelen wrote:

 Is this a desired fix? (i.e. trying to make a better build with 
 MSVC while we wait for the new version)


Not really. It fixes building std.math but fails with any user 
application.

What about implement a hex value parsing function which is 
invoked if the result is 0?


 One positive effect of this change is that it un-hides some 
 other failing tests.


As a local fix, this is good.

Regards,
Kai
Mar 01 2015
parent reply "Johan Engelen" <j j.nl> writes:
On Monday, 2 March 2015 at 07:17:37 UTC, Kai Nacke wrote:

 Hi Johan!

 On Sunday, 1 March 2015 at 21:00:53 UTC, Johan Engelen wrote:

 Is this a desired fix? (i.e. trying to make a better build 
 with MSVC while we wait for the new version)


 Not really. It fixes building std.math but fails with any user 
 application.

 What about implement a hex value parsing function which is 
 invoked if the result is 0?


dmd has a strold for compilers that do not support hex floats:
https://github.com/D-Programming-Language/dmd/blob/master/src/backend/strtold.c

Or do you think that is overkill and we are better off 
implementing our own hex parser?
Mar 02 2015
parent reply "Johan Engelen" <j j.nl> writes:
On Monday, 2 March 2015 at 18:49:42 UTC, Johan Engelen wrote:

 On Monday, 2 March 2015 at 07:17:37 UTC, Kai Nacke wrote:

 What about implement a hex value parsing function which is 
 invoked if the result is 0?


 dmd has a strold for compilers that do not support hex floats:
 https://github.com/D-Programming-Language/dmd/blob/master/src/backend/strtold.c

 Or do you think that is overkill and we are better off 
 implementing our own hex parser?


Actually I am thinking about ripping out just the hex parsing 
part from dmd's strtold, and tweaking it for our purposes...
Mar 02 2015
parent reply "kinke" <noone nowhere.com> writes:
On Monday, 2 March 2015 at 18:51:27 UTC, Johan Engelen wrote:

 On Monday, 2 March 2015 at 18:49:42 UTC, Johan Engelen wrote:

 On Monday, 2 March 2015 at 07:17:37 UTC, Kai Nacke wrote:

 What about implement a hex value parsing function which is 
 invoked if the result is 0?


 dmd has a strold for compilers that do not support hex floats:
 https://github.com/D-Programming-Language/dmd/blob/master/src/backend/strtold.c

 Or do you think that is overkill and we are better off 
 implementing our own hex parser?


 Actually I am thinking about ripping out just the hex parsing 
 part from dmd's strtold, and tweaking it for our purposes...


We've already had DMD's strtold() implementation. It was removed 
when we switched to double-precision real on Win64 and MSVCRT, 
see https://github.com/ldc-developers/ldc/pull/747.
For MSVCRT < 14, we'd need a standard-compliant strtod(), not 
strtold().
Mar 02 2015
parent reply "Johan Engelen" <j j.nl> writes:
On Monday, 2 March 2015 at 21:52:10 UTC, kinke wrote:

 We've already had DMD's strtold() implementation. It was 
 removed when we switched to double-precision real on Win64 and 
 MSVCRT, see https://github.com/ldc-developers/ldc/pull/747.
 For MSVCRT < 14, we'd need a standard-compliant strtod(), not 
 strtold().


OK, but it is relatively easy to write a hex parser on top of 
MSVC's strtod (for the heavy duty work). I'll see how far I get :)
Mar 02 2015
parent reply "Johan Engelen" <j j.nl> writes:
I have an initial implementation working now.
Where should I put this code in the codebase? Create a new file 
dmd2/root/strtod_hex.c ?

thanks,
   Johan
Mar 03 2015
parent reply "Johan Engelen" <j j.nl> writes:
Also, where do I add tests for this?
I could add a .d test file, where correct parsing of hex float 
literals are tested against binary representations.
Mar 03 2015
parent reply "Dan Olson" <zans4cans yahoo.com> writes:
On Tuesday, 3 March 2015 at 21:14:47 UTC, Johan Engelen wrote:

 Also, where do I add tests for this?
 I could add a .d test file, where correct parsing of hex float 
 literals are tested against binary representations.


Could David Gay's strtod() be used?
http://www.exploringbinary.com/how-strtod-works-and-sometimes-doesnt/
Mar 03 2015
next sibling parent "Johan Engelen" <j j.nl> writes:
On Tuesday, 3 March 2015 at 22:12:39 UTC, Dan Olson wrote:

 Could David Gay's strtod() be used?
 http://www.exploringbinary.com/how-strtod-works-and-sometimes-doesnt/


Well... :(
https://github.com/JohanEngelen/ldc/commit/417cded588bcd8cf5a783ac0da731dfd53048b5d
;-)

After adding proper rounding and overflow, I think it is done. It 
already works well on more normal input.
Mar 03 2015
prev sibling next sibling parent reply "Johan Engelen" <j j.nl> writes:
On Tuesday, 3 March 2015 at 22:12:39 UTC, Dan Olson wrote:

 Could David Gay's strtod() be used?
 http://www.exploringbinary.com/how-strtod-works-and-sometimes-doesnt/


Thanks for that link. I had a look at David Gay's strtod() (a
complicated piece of code) and it also accepts hex floats [1].
I think it is clear that my hex parser is much simpler but also
more naive and may need quite some extra complication to make it
deal with all corner cases (I am by no measure an expert on IEEE
fp). Although a fun exercise, I guess I should accept the fact
that David Gay's function is very much superior to mine :)

Should we replace MSVC's strtod() altogether with David Gay's, or
use only the hex parser "gethex()"?
Mar 04 2015
parent reply Dan Olson <zans.is.for.cans yahoo.com> writes:
"Johan Engelen" <j j.nl> writes:


 On Tuesday, 3 March 2015 at 22:12:39 UTC, Dan Olson wrote:

 Could David Gay's strtod() be used?
 http://www.exploringbinary.com/how-strtod-works-and-sometimes-doesnt/


 Thanks for that link. I had a look at David Gay's strtod() (a
 complicated piece of code) and it also accepts hex floats [1].
 I think it is clear that my hex parser is much simpler but also
 more naive and may need quite some extra complication to make it
 deal with all corner cases (I am by no measure an expert on IEEE
 fp). Although a fun exercise, I guess I should accept the fact
 that David Gay's function is very much superior to mine :)

 Should we replace MSVC's strtod() altogether with David Gay's, or
 use only the hex parser "gethex()"?


Personally I don't know but it does look like the gethex() function
could be used.  I would think whatever is easiest to keep going and
passes unittests.  BTW, David Gay is listed as the author in BDS man
page for strtod (uses by OS X).

Another thing.  Kai has worked on using LLVM'S own float (APFloat)
support class.  Seem to me like this will eventually be the best way to
go as it does all this and will be consistent across platforms.  Clang
appears to use this to parse float literals (I just grepped the source).

P.S. exploringbinary is a fun website.
Mar 04 2015
parent "Kai Nacke" <kai redstar.de> writes:
On Thursday, 5 March 2015 at 05:00:04 UTC, Dan Olson wrote:

 "Johan Engelen" <j j.nl> writes:


 On Tuesday, 3 March 2015 at 22:12:39 UTC, Dan Olson wrote:

 Could David Gay's strtod() be used?
 http://www.exploringbinary.com/how-strtod-works-and-sometimes-doesnt/


 Thanks for that link. I had a look at David Gay's strtod() (a
 complicated piece of code) and it also accepts hex floats [1].
 I think it is clear that my hex parser is much simpler but also
 more naive and may need quite some extra complication to make 
 it
 deal with all corner cases (I am by no measure an expert on 
 IEEE
 fp). Although a fun exercise, I guess I should accept the fact
 that David Gay's function is very much superior to mine :)

 Should we replace MSVC's strtod() altogether with David Gay's, 
 or
 use only the hex parser "gethex()"?


 Personally I don't know but it does look like the gethex() 
 function
 could be used.  I would think whatever is easiest to keep going 
 and
 passes unittests.  BTW, David Gay is listed as the author in 
 BDS man
 page for strtod (uses by OS X).

 Another thing.  Kai has worked on using LLVM'S own float 
 (APFloat)
 support class.  Seem to me like this will eventually be the 
 best way to
 go as it does all this and will be consistent across platforms.
  Clang
 appears to use this to parse float literals (I just grepped the 
 source).

 P.S. exploringbinary is a fun website.


Hi Dan!

Thanks for the hint! I now use the LLVM functions instead of MSVC 
strtold(). Maybe someone can comment on the test results?

Regards,
Kai
Mar 06 2015
prev sibling parent "Kai Nacke" <kai redstar.de> writes:
On Tuesday, 3 March 2015 at 22:12:39 UTC, Dan Olson wrote:

 On Tuesday, 3 March 2015 at 21:14:47 UTC, Johan Engelen wrote:

 Also, where do I add tests for this?
 I could add a .d test file, where correct parsing of hex float 
 literals are tested against binary representations.


 Could David Gay's strtod() be used?
 http://www.exploringbinary.com/how-strtod-works-and-sometimes-doesnt/


I think we should add this to druntime. The license seems to be 
compatible.
(Johan has already created a pull request I am going to reuse.)

Regards,
Kai
Mar 06 2015