-
Notifications
You must be signed in to change notification settings - Fork 5.3k
DivRem 64by32 and 32by16 refactor in Decimal.DecCalc #99196
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
src/libraries/System.Private.CoreLib/src/System/Decimal.DecCalc.cs
Outdated
Show resolved
Hide resolved
Have you looked at the docs in dotnet/performance? They explain how to test private changes like this. If there's not coverage, you can add a test there. |
|
Tagging subscribers to this area: @dotnet/area-system-numerics Issue DetailsTried to make the code easier to read with all the DivRem stuff going on. It will also be much easier to leverage intrinsics in the helper methods (such as I don't know how to compare benchmarks thought, to see there are no regressions in performance.
|
|
Same general comments that I gave on #99212 apply here. These really seems like some basic JIT optimizations we're missing and not something we should be adding hardcoded helpers around. Cases of This would be particularly relevant for |
I agree with the missing JIT optimizations. The purpose of this PR was not to optimize, rather refactor a bit without impacting performance negatively. Like you mentioned in #99212 this has to do with the DivRem operations. Thism PR just moves the workaround of calculating remainder/varries as difference from being spread out in the code to the helpers. Main purpose of this PR is:
|
|
Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it. |
Tried to make the code easier to read with all the DivRem stuff going on. It will also be much easier to leverage intrinsics in the helper methods (such as
X86Base.DivRem) when they can be used properly.I don't know how to compare benchmarks thought, to see there are no regressions in performance.