Skip to content

std.math: Use DOUBLEPAIR_MSB/LSB for accessing high/low parts of ibmExtended format#5823

Merged
dlang-bot merged 1 commit intodlang:masterfrom
ibuclaw:ppcmsb
Mar 26, 2018
Merged

std.math: Use DOUBLEPAIR_MSB/LSB for accessing high/low parts of ibmExtended format#5823
dlang-bot merged 1 commit intodlang:masterfrom
ibuclaw:ppcmsb

Conversation

@ibuclaw
Copy link
Member

@ibuclaw ibuclaw commented Oct 29, 2017

A continuation of #5822 (or a prelude, works both ways).

When running the unittests for PPC64/LE, found many failures due to it checking the wrong part (MANTISSA_MSB value is the least significant index on LittleEndian).

Fixed places where Big/Little endian would otherwise differ, but actually should do the same thing.

@ibuclaw ibuclaw requested a review from dnadlinger as a code owner October 29, 2017 12:13
@dlang-bot
Copy link
Contributor

Thanks for your pull request, @ibuclaw!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

@ibuclaw ibuclaw added the math label Oct 29, 2017
@ibuclaw
Copy link
Member Author

ibuclaw commented Oct 29, 2017

Maybe its time to resurrect #4336

@redstar
Copy link
Contributor

redstar commented Mar 14, 2018

Very nice - I found this, too. I will have a closer look this evening. One point: you are enabling doubledouble support on little endian. This should also be reflected in the version check at the beginning of the file (https://github.com/dlang/phobos/blob/master/std/math.d#L233).

@ibuclaw
Copy link
Member Author

ibuclaw commented Mar 14, 2018

@redstar, oh and there's also #5825 which fixes this using an entirely different approach.

@redstar
Copy link
Contributor

redstar commented Mar 14, 2018

Ok, this is the simple approach. :-) Looks good to me. There are no overlooked instance, so it should work out of the box.

@ibuclaw
Copy link
Member Author

ibuclaw commented Mar 17, 2018

Rebased.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants