www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Replacing std.math raw pointer arithmetic with a union type

reply tsbockman <thomas.bockman gmail.com> writes:
std.math contains a lot of raw pointer arithmetic for accessing 
the various bit fields of floating-point values (see 
https://en.wikipedia.org/wiki/IEEE_754-1985). Much of this code 
has several nearly-identical copies, manually specialized for 
each supported floating-point format.

Such code is verbose, hard to read, and, judging by the bugs and 
missing specializations I've found, hard to write correctly. It 
is also not compatible with CTFE.


(https://github.com/dlang/phobos/pull/4336), which begins the 
process of replacing all the pointer arithmetic in std.math, and 
the supporting floatTraits template, using a fast union type: 
RealRep.

Ian Buclaw recently approved my work, but I believe that a pull 
request of this size and importance should be review by at least 
one other qualified person.

Would any of our other floating-point experts be willing to take 
a look?
May 16 2017
next sibling parent "H. S. Teoh via Digitalmars-d" <digitalmars-d puremagic.com> writes:
On Wed, May 17, 2017 at 03:02:02AM +0000, tsbockman via Digitalmars-d wrote:
 std.math contains a lot of raw pointer arithmetic for accessing the
 various bit fields of floating-point values (see
 https://en.wikipedia.org/wiki/IEEE_754-1985). Much of this code has
 several nearly-identical copies, manually specialized for each
 supported floating-point format.
 
 Such code is verbose, hard to read, and, judging by the bugs and
 missing specializations I've found, hard to write correctly. It is
 also not compatible with CTFE.
 

 (https://github.com/dlang/phobos/pull/4336), which begins the process
 of replacing all the pointer arithmetic in std.math, and the
 supporting floatTraits template, using a fast union type: RealRep.
 
 Ian Buclaw recently approved my work, but I believe that a pull
 request of this size and importance should be review by at least one
 other qualified person.
 
 Would any of our other floating-point experts be willing to take a
 look?
Nowhere near a floating-point "expert" here, but I think one of the big blockers for CTFE right now is that none of the ways of accessing the underlying bits in the floating-point representation are supported: neither pointer arithmetic nor unions are supported. Well, the current CTFE engine does support unions, but you can only read values of the same type that you write into it, so you can't use it to get at the bit representation of floating-point values. And AFAIK, Stefan's new CTFE engine won't be supporting unions until the last stage (floating-point support is scheduled to be implemented last because of anticipated complexities), so it will be a while before we have any hope of std.math working in CTFE. Having said that, though, prepping Phobos to be more maintainable is always a good thing, and avoiding pointer arithmetic means it's more likely to be supported by the new CTFE engine eventually, so in principle I like the idea, even though I don't think I'm qualified to officially approve it. T -- Tell me and I forget. Teach me and I remember. Involve me and I understand. -- Benjamin Franklin
May 16 2017
prev sibling next sibling parent reply Simen =?UTF-8?B?S2rDpnLDpXM=?= <simen.kjaras gmail.com> writes:
On Wednesday, 17 May 2017 at 03:02:02 UTC, tsbockman wrote:
 std.math contains a lot of raw pointer arithmetic for accessing 
 the various bit fields of floating-point values (see 
 https://en.wikipedia.org/wiki/IEEE_754-1985). Much of this code 
 has several nearly-identical copies, manually specialized for 
 each supported floating-point format.

 Such code is verbose, hard to read, and, judging by the bugs 
 and missing specializations I've found, hard to write 
 correctly. It is also not compatible with CTFE.


 (https://github.com/dlang/phobos/pull/4336), which begins the 
 process of replacing all the pointer arithmetic in std.math, 
 and the supporting floatTraits template, using a fast union 
 type: RealRep.

 Ian Buclaw recently approved my work, but I believe that a pull 
 request of this size and importance should be review by at 
 least one other qualified person.

 Would any of our other floating-point experts be willing to 
 take a look?
I had a look-see, and it certainly is more readable and less error-prone than the incessant casting-to-pointer in the existing implementation. Having all the magic in one place makes a lot of sense, and if it also brings performance benefits as your table indicates, all the better. This should also make std.math more approachable to the curious and make fixing bugs and implementing new functionality easier, especially when supporting more than one format is required. On the topic of format support: the details of ibmExtended seems to leak a lot - would it be possible to encapsulate that better? I realize it's a weird format and some leakage may be unavoidable, but currently it seems basically every function you've converted contains a happy path dealing with every other format, and a special case for ibmExtended. All in all, this seems solid, and a great improvement over the current state. -- Simen
May 17 2017
parent tsbockman <thomas.bockman gmail.com> writes:
On Wednesday, 17 May 2017 at 13:09:45 UTC, Simen Kjærås wrote:
 On the topic of format support: the details of ibmExtended 
 seems to leak a lot - would it be possible to encapsulate that 
 better? I realize it's a weird format and some leakage may be 
 unavoidable, but currently it seems basically every function 
 you've converted contains a happy path dealing with every other 
 format, and a special case for ibmExtended.
Sadly, no. I don't think that it's possible to share the same code for ibmExtended; it's just too different. I am, however, open to just dropping it entirely. I believe the current implementation for ibmExtended is completely broken, so this wouldn't be a regression.
 All in all, this seems solid, and a great improvement over the 
 current state.

 --
   Simen
Thanks for taking a look! Would you mind leaving a "Looks good to me" comment on the Github page?
May 17 2017
prev sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 5/16/2017 8:02 PM, tsbockman wrote:
 Such code is verbose, hard to read, and, judging by the bugs and missing
 specializations I've found, hard to write correctly. It is also not compatible
 with CTFE.
CTFE does support things like: double d; int i = *(cast(int*)&d); as a special case, specifically to support bit manipulation of floating point types.
May 17 2017
parent reply Stefan Koch <uplink.coder googlemail.com> writes:
On Wednesday, 17 May 2017 at 15:15:04 UTC, Walter Bright wrote:
 On 5/16/2017 8:02 PM, tsbockman wrote:
 Such code is verbose, hard to read, and, judging by the bugs 
 and missing
 specializations I've found, hard to write correctly. It is 
 also not compatible
 with CTFE.
CTFE does support things like: double d; int i = *(cast(int*)&d); as a special case, specifically to support bit manipulation of floating point types.
the special case it supports if cast(uint*)&float; and cast(ulong*)&double.
May 17 2017
next sibling parent Walter Bright <newshound2 digitalmars.com> writes:
On 5/17/2017 8:30 AM, Stefan Koch wrote:
 On Wednesday, 17 May 2017 at 15:15:04 UTC, Walter Bright wrote:
 On 5/16/2017 8:02 PM, tsbockman wrote:
 Such code is verbose, hard to read, and, judging by the bugs and missing
 specializations I've found, hard to write correctly. It is also not compatible
 with CTFE.
CTFE does support things like: double d; int i = *(cast(int*)&d); as a special case, specifically to support bit manipulation of floating point types.
the special case it supports if cast(uint*)&float; and cast(ulong*)&double.
Thanks for the clarification. Hence, removing such casts in Phobos would break it in CTFE, rather than fix it.
May 17 2017
prev sibling parent reply tsbockman <thomas.bockman gmail.com> writes:
On Wednesday, 17 May 2017 at 15:30:29 UTC, Stefan Koch wrote:
 the special case it supports if cast(uint*)&float;
 and cast(ulong*)&double.
What about casting from real* when real.sizeof > double.sizeof?
May 17 2017
parent reply Stefan Koch <uplink.coder googlemail.com> writes:
On Wednesday, 17 May 2017 at 19:26:32 UTC, tsbockman wrote:
 On Wednesday, 17 May 2017 at 15:30:29 UTC, Stefan Koch wrote:
 the special case it supports if cast(uint*)&float;
 and cast(ulong*)&double.
What about casting from real* when real.sizeof > double.sizeof?
unsupported. The code in ctfeExpr (paintFloatInt) explicitly checks for size == 4 || size == 8
May 17 2017
next sibling parent Dominikus Dittes Scherkl <Dominikus.Scherkl continental-corporation.com> writes:
On Wednesday, 17 May 2017 at 20:25:26 UTC, Stefan Koch wrote:
 On Wednesday, 17 May 2017 at 19:26:32 UTC, tsbockman wrote:
 On Wednesday, 17 May 2017 at 15:30:29 UTC, Stefan Koch wrote:
 the special case it supports if cast(uint*)&float;
 and cast(ulong*)&double.
What about casting from real* when real.sizeof > double.sizeof?
unsupported. The code in ctfeExpr (paintFloatInt) explicitly checks for size == 4 || size == 8
we finally need support for ucent!
May 18 2017
prev sibling parent tsbockman <thomas.bockman gmail.com> writes:
On Wednesday, 17 May 2017 at 20:25:26 UTC, Stefan Koch wrote:
 On Wednesday, 17 May 2017 at 19:26:32 UTC, tsbockman wrote:
 On Wednesday, 17 May 2017 at 15:30:29 UTC, Stefan Koch wrote:
 the special case it supports if cast(uint*)&float;
 and cast(ulong*)&double.
What about casting from real* when real.sizeof > double.sizeof?
unsupported. The code in ctfeExpr (paintFloatInt) explicitly checks for size == 4 || size == 8
I have opened a DMD PR to fix this: https://github.com/dlang/dmd/pull/6811
May 18 2017