std.conv: ieeeQuadruple (AArch64) support for parse!real #5965
Conversation
|
Thanks for your pull request, @jpf91! We are looking forward to reviewing it, and you should be hearing from a maintainer soon. Some tips to help speed things up:
Bear in mind that large or tricky changes may require multiple rounds of review and revision. Please see CONTRIBUTING.md for more information. Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. |
std/conv.d
Outdated
| * 1) Parse the textual input into msdec and exp variables: | ||
| * input is 0xaaaaa...p+000... where aaaa is the mantissa in hex and | ||
| * 000 is the exponent in decimal format. | ||
| * 2) Rounding / sticky bit, whatever? |
There was a problem hiding this comment.
That was a note that I do not totally understand what the algorithm is doing with these guard bits... Anyway, forgot to remove that note :-)
std/conv.d
Outdated
| *msw = quad_mant[0]; | ||
| *lsw = quad_mant[1]; | ||
|
|
||
| //Store exponent, now overwriting implicit bit |
Currently limited to ieeeExtended precision by the parser. This just adds code to put the parsed value into a ieeeQuadruple format real.
| // Stuff mantissa directly into double | ||
| // (first including implicit bit) | ||
| *msw = quad_mant[0]; | ||
| *lsw = quad_mant[1]; |
There was a problem hiding this comment.
Would using a union here be better? I'm a bit weary of the optimizer here, incase it just sees this as dead code (msw/lsw are set but not read).
Would probably be more clearer as well that you are changing the value of ldval directly.
There was a problem hiding this comment.
Probably not a problem however. I just have a slight distaste for using pointers for bit manipulation in std.math that carries over here.
There was a problem hiding this comment.
The IEEE extended and double branches used pointers as well and I'm not sure if this is relevant for CTFE-ability? I think the optimizer shouldn't be much of a problem, as the pointers are used to write to the value?
There was a problem hiding this comment.
I've seen it go wrong before when I implemented floorImpl. Actual details are hazy, but that function is doing a lot more bit setting than here.
Backport from upstream phobos 2.079 for AArch64. Reviewed-on: dlang/phobos#5965 git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@266238 138bc75d-0d04-0410-961f-82ee72b054a4
Backport from upstream phobos 2.079 for AArch64. Reviewed-on: dlang/phobos#5965 From-SVN: r266238
I've added some code to support ieeeQuadruple reals in
parse!real. I've also documented some of the parser. As I do not fully understand the internals of the parser, I didn't want to extend the parser to allow for higher precision. We'd have to use cent (or, as there is no cent type, two longs for a manual 128 bit in implementation) which probably makes the code slower. So I leave that to somebody who fully understands the parser code. (It would also make more sense to limit precision in the parser for double precision reals. Currently we parse at higher precision and remove precision when converting to the final value. This just adds unnecessary overhead).I've added code to stuff the parse values into an ieeeQuadruple real though. This just means parse!real on ieeeQuadruple targets is limited to the same precision as on X86_64. Note that I had to implement bit-shifts for 128bit integers using two ulongs, which doesn't exactly make the code more readable...