Re: Replacing std.math raw pointer arithmetic with a union type
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*) and cast(ulong*) 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
Re: Replacing std.math raw pointer arithmetic with a union type
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*) and cast(ulong*) 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!
Re: Replacing std.math raw pointer arithmetic with a union type
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*) and cast(ulong*) What about casting from real* when real.sizeof > double.sizeof? unsupported. The code in ctfeExpr (paintFloatInt) explicitly checks for size == 4 || size == 8
Re: Replacing std.math raw pointer arithmetic with a union type
On Wednesday, 17 May 2017 at 15:30:29 UTC, Stefan Koch wrote: the special case it supports if cast(uint*) and cast(ulong*) What about casting from real* when real.sizeof > double.sizeof?
Re: Replacing std.math raw pointer arithmetic with a union type
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*)); as a special case, specifically to support bit manipulation of floating point types. the special case it supports if cast(uint*) and cast(ulong*) Thanks for the clarification. Hence, removing such casts in Phobos would break it in CTFE, rather than fix it.
Re: Replacing std.math raw pointer arithmetic with a union type
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*)); as a special case, specifically to support bit manipulation of floating point types. the special case it supports if cast(uint*) and cast(ulong*)
Re: Replacing std.math raw pointer arithmetic with a union type
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*)); as a special case, specifically to support bit manipulation of floating point types.
Re: Replacing std.math raw pointer arithmetic with a union type
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?
Re: Replacing std.math raw pointer arithmetic with a union type
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. A while ago I created Phobos #4336 (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
Re: Replacing std.math raw pointer arithmetic with a union type
On Wed, May 17, 2017 at 03:02:02AM +, 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. > > A while ago I created Phobos #4336 > (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
