Skip to content

Conversation

@dakersnar
Copy link
Owner

This PR should include everything from #4 but with additions for Decimal64. I'll be pivoting to Decimal64 development from here forward.

…sing

Ieee decimal basic setup and parsing
@dakersnar
Copy link
Owner Author

This commit: 7cbca98 is the only real "new" thing compared to the Decimal32 PR. I grouped all logic updates in this one commit to make reviewing easier.

IMinMaxValue<Decimal64>
{

private const NumberStyles DefaultParseStyle = NumberStyles.Float | NumberStyles.AllowThousands; // TODO is this correct?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be correct, yes.

// c. Trailing significand. Can be used to encode a payload, to distinguish different NaNs.
// Note: Canonical NaN has G6-G12 as 0 and the encoding of the payload also canonical.

private const ulong QNanBits = 0x7C00_0000_0000_0000; // TODO I used a "positive" NaN here, should it be negative?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd probably keep it as positive for the "bit mask"

But we should probably return -QNaN for the "canonincal" version, so it matches what happens for the binary QNaN

Copy link
Owner Author

@dakersnar dakersnar Feb 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, although I'm curious how we arrived at that decision. I think the intel library has positive QNaN as their "canonical" NaN, for example, this early return in bid64_add:
image

Despite this, is it still reasonable to return -QNaN for cases like this?

Copy link

@tannergooding tannergooding Feb 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because x86 hardware returns -QNaN for binary floats in hardware, so it's been the historical "norm" that people see

@dakersnar
Copy link
Owner Author

@tannergooding I updated most of the HalfTests unit tests for Decimal64. I structured my commits such that that you can look at ef6a876 to get a readable diff of the updates.

@dakersnar
Copy link
Owner Author

I was planning on doing the same strategy for HalfTests.GenericMath, pushing the "starting point" here: 864d939

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants