digitalmars.D - Replacing std.math raw pointer arithmetic with a union type
- tsbockman (18/18) May 16 2017 std.math contains a lot of raw pointer arithmetic for accessing
- H. S. Teoh via Digitalmars-d (20/41) May 16 2017 Nowhere near a floating-point "expert" here, but I think one of the big
- Simen =?UTF-8?B?S2rDpnLDpXM=?= (20/38) May 17 2017 I had a look-see, and it certainly is more readable and less
- tsbockman (8/18) May 17 2017 Sadly, no. I don't think that it's possible to share the same
- Walter Bright (5/8) May 17 2017 CTFE does support things like:
- Stefan Koch (3/14) May 17 2017 the special case it supports if cast(uint*)&float;
- Walter Bright (3/18) May 17 2017 Thanks for the clarification.
- tsbockman (2/4) May 17 2017 What about casting from real* when real.sizeof > double.sizeof?
- Stefan Koch (3/7) May 17 2017 unsupported. The code in ctfeExpr (paintFloatInt) explicitly
- Dominikus Dittes Scherkl (2/10) May 18 2017 we finally need support for ucent!
- tsbockman (3/11) May 18 2017 I have opened a DMD PR to fix this:
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
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
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
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. -- SimenThanks for taking a look! Would you mind leaving a "Looks good to me" comment on the Github page?
May 17 2017
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
On Wednesday, 17 May 2017 at 15:15:04 UTC, Walter Bright wrote:On 5/16/2017 8:02 PM, tsbockman wrote:the special case it supports if cast(uint*)&float; and cast(ulong*)&double.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
On 5/17/2017 8:30 AM, Stefan Koch wrote:On Wednesday, 17 May 2017 at 15:15:04 UTC, Walter Bright wrote:Thanks for the clarification. Hence, removing such casts in Phobos would break it in CTFE, rather than fix it.On 5/16/2017 8:02 PM, tsbockman wrote:the special case it supports if cast(uint*)&float; and cast(ulong*)&double.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
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
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:unsupported. The code in ctfeExpr (paintFloatInt) explicitly checks for size == 4 || size == 8the 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
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:we finally need support for ucent!On Wednesday, 17 May 2017 at 15:30:29 UTC, Stefan Koch wrote:unsupported. The code in ctfeExpr (paintFloatInt) explicitly checks for size == 4 || size == 8the special case it supports if cast(uint*)&float; and cast(ulong*)&double.What about casting from real* when real.sizeof > double.sizeof?
May 18 2017
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:I have opened a DMD PR to fix this: https://github.com/dlang/dmd/pull/6811On Wednesday, 17 May 2017 at 15:30:29 UTC, Stefan Koch wrote:unsupported. The code in ctfeExpr (paintFloatInt) explicitly checks for size == 4 || size == 8the special case it supports if cast(uint*)&float; and cast(ulong*)&double.What about casting from real* when real.sizeof > double.sizeof?
May 18 2017