Cleanup some math helpers in DecimalCalc#76118
Cleanup some math helpers in DecimalCalc#76118huoyaoyuan wants to merge 2 commits intodotnet:mainfrom
Conversation
ghost
added
the
community-contribution
|
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
|
Tagging subscribers to this area: @dotnet/area-system-numerics Issue DetailsBigMul(uint) and DivRem are not included.
|
| // | ||
| const uint SNGBIAS = 126; | ||
| int exp = (int)(GetExponent(input) - SNGBIAS); | ||
| int exp = input.Exponent + 1; |
There was a problem hiding this comment.
The change is correct in that it matches current behavior, but there isn't anything explaining why its "off by one".
That is Single.Exponent and Double.Exponent return the exact and correct exponent, while this is the adjusting it to be one higher than expected. I've not sat down to work through "why" the adjustment is needed for decimal's scale but this is the type of scenario where there should really be a comment explaining it.
There was a problem hiding this comment.
The logic looks slightly different for float. The original code lacks &mask, I'm not sure if the failure is related since VS debugging is somehow broken for me. But I don't expect the logic of float to be different from double.
There was a problem hiding this comment.
These helpers should be fixed once this PR gets merged. We tabled it for .NET 7 but now that we are starting to work on .NET 8 it should be revisited.
There was a problem hiding this comment.
Thanks. Then I think it's better to wait for that PR.
|
Looks like there are some test failures that need to be looked into |
ghost
closed this
|
Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it. |
ghost
locked as resolved and limited conversation to collaborators
3 participants
BigMul(uint) and DivRem are not included.