DivRem 64by32 and 32by16 refactor in Decimal.DecCalc

[lilinus]

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.

[danmoseley]

I don't know how to compare benchmarks thought, to see there are no regressions in performance.

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.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-numerics
See info in area-owners.md if you want to be subscribed.

Issue Details

---

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.

Author:	lilinus
Assignees:	-
Labels:	area-System.Numerics, needs-area-label
Milestone:	-

[tannergooding]

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 ulong / uint and similar are areas that the JIT should be able to recognize and optimize itself, allowing this to light up anywhere its used and not require customized logic that only works for System.Decimal.

This would be particularly relevant for Arm64 which doesn't have specialized instructions allowing uint128 / uint64 or similar.

[lilinus]

These really seems like some basic JIT optimizations we're missing and not something we should be adding hardcoded helpers around. Cases of ulong / uint and similar are areas that the JIT should be able to recognize and optimize itself, allowing this to light up anywhere its used and not require customized logic that only works for System.Decimal.

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:

• Make code more readable and easier to follow / maintain using the helpers. IMO it's just easier to follow what is going on with the divide operators with carries with these introduced helpers.
• Homogenous DivRem methods already exists in Math, but these non-homogenous also (hopefully) conveys the inputs' and outputs' sizes of the operations better than before.
• Operations and flow in existing methods stays the same, just using the new helper methods to (hopefully) improve maintainability.

[dotnet-policy-service]

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

[lilinus]

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.

[danmoseley]

I don't know how to compare benchmarks thought, to see there are no regressions in performance.

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.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-numerics
See info in area-owners.md if you want to be subscribed.

Issue Details

---

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.

Author:	lilinus
Assignees:	-
Labels:	area-System.Numerics, needs-area-label
Milestone:	-

[tannergooding]

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 ulong / uint and similar are areas that the JIT should be able to recognize and optimize itself, allowing this to light up anywhere its used and not require customized logic that only works for System.Decimal.

This would be particularly relevant for Arm64 which doesn't have specialized instructions allowing uint128 / uint64 or similar.

[lilinus]

These really seems like some basic JIT optimizations we're missing and not something we should be adding hardcoded helpers around. Cases of ulong / uint and similar are areas that the JIT should be able to recognize and optimize itself, allowing this to light up anywhere its used and not require customized logic that only works for System.Decimal.

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:

• Make code more readable and easier to follow / maintain using the helpers. IMO it's just easier to follow what is going on with the divide operators with carries with these introduced helpers.
• Homogenous DivRem methods already exists in Math, but these non-homogenous also (hopefully) conveys the inputs' and outputs' sizes of the operations better than before.
• Operations and flow in existing methods stays the same, just using the new helper methods to (hopefully) improve maintainability.

[dotnet-policy-service]

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

[lilinus]

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.

[danmoseley]

I don't know how to compare benchmarks thought, to see there are no regressions in performance.

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.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-numerics
See info in area-owners.md if you want to be subscribed.

Issue Details

---

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.

Author:	lilinus
Assignees:	-
Labels:	area-System.Numerics, needs-area-label
Milestone:	-

[tannergooding]

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 ulong / uint and similar are areas that the JIT should be able to recognize and optimize itself, allowing this to light up anywhere its used and not require customized logic that only works for System.Decimal.

This would be particularly relevant for Arm64 which doesn't have specialized instructions allowing uint128 / uint64 or similar.

[lilinus]

These really seems like some basic JIT optimizations we're missing and not something we should be adding hardcoded helpers around. Cases of ulong / uint and similar are areas that the JIT should be able to recognize and optimize itself, allowing this to light up anywhere its used and not require customized logic that only works for System.Decimal.

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:

• Make code more readable and easier to follow / maintain using the helpers. IMO it's just easier to follow what is going on with the divide operators with carries with these introduced helpers.
• Homogenous DivRem methods already exists in Math, but these non-homogenous also (hopefully) conveys the inputs' and outputs' sizes of the operations better than before.
• Operations and flow in existing methods stays the same, just using the new helper methods to (hopefully) improve maintainability.

[dotnet-policy-service]

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

[lilinus]

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.

[danmoseley]

I don't know how to compare benchmarks thought, to see there are no regressions in performance.

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.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-numerics
See info in area-owners.md if you want to be subscribed.

Issue Details

---

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.

Author:	lilinus
Assignees:	-
Labels:	area-System.Numerics, needs-area-label
Milestone:	-

[tannergooding]

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 ulong / uint and similar are areas that the JIT should be able to recognize and optimize itself, allowing this to light up anywhere its used and not require customized logic that only works for System.Decimal.

This would be particularly relevant for Arm64 which doesn't have specialized instructions allowing uint128 / uint64 or similar.

[lilinus]

These really seems like some basic JIT optimizations we're missing and not something we should be adding hardcoded helpers around. Cases of ulong / uint and similar are areas that the JIT should be able to recognize and optimize itself, allowing this to light up anywhere its used and not require customized logic that only works for System.Decimal.

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:

• Make code more readable and easier to follow / maintain using the helpers. IMO it's just easier to follow what is going on with the divide operators with carries with these introduced helpers.
• Homogenous DivRem methods already exists in Math, but these non-homogenous also (hopefully) conveys the inputs' and outputs' sizes of the operations better than before.
• Operations and flow in existing methods stays the same, just using the new helper methods to (hopefully) improve maintainability.

[dotnet-policy-service]

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

[lilinus]

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.

[danmoseley]

I don't know how to compare benchmarks thought, to see there are no regressions in performance.

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.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-numerics
See info in area-owners.md if you want to be subscribed.

Issue Details

---

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.

Author:	lilinus
Assignees:	-
Labels:	area-System.Numerics, needs-area-label
Milestone:	-

[tannergooding]

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 ulong / uint and similar are areas that the JIT should be able to recognize and optimize itself, allowing this to light up anywhere its used and not require customized logic that only works for System.Decimal.

This would be particularly relevant for Arm64 which doesn't have specialized instructions allowing uint128 / uint64 or similar.

[lilinus]

These really seems like some basic JIT optimizations we're missing and not something we should be adding hardcoded helpers around. Cases of ulong / uint and similar are areas that the JIT should be able to recognize and optimize itself, allowing this to light up anywhere its used and not require customized logic that only works for System.Decimal.

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:

• Make code more readable and easier to follow / maintain using the helpers. IMO it's just easier to follow what is going on with the divide operators with carries with these introduced helpers.
• Homogenous DivRem methods already exists in Math, but these non-homogenous also (hopefully) conveys the inputs' and outputs' sizes of the operations better than before.
• Operations and flow in existing methods stays the same, just using the new helper methods to (hopefully) improve maintainability.

[dotnet-policy-service]

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

[lilinus]

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.

[danmoseley]

I don't know how to compare benchmarks thought, to see there are no regressions in performance.

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.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-numerics
See info in area-owners.md if you want to be subscribed.

Issue Details

---

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.

Author:	lilinus
Assignees:	-
Labels:	area-System.Numerics, needs-area-label
Milestone:	-

[tannergooding]

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 ulong / uint and similar are areas that the JIT should be able to recognize and optimize itself, allowing this to light up anywhere its used and not require customized logic that only works for System.Decimal.

This would be particularly relevant for Arm64 which doesn't have specialized instructions allowing uint128 / uint64 or similar.

[lilinus]

These really seems like some basic JIT optimizations we're missing and not something we should be adding hardcoded helpers around. Cases of ulong / uint and similar are areas that the JIT should be able to recognize and optimize itself, allowing this to light up anywhere its used and not require customized logic that only works for System.Decimal.

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:

• Make code more readable and easier to follow / maintain using the helpers. IMO it's just easier to follow what is going on with the divide operators with carries with these introduced helpers.
• Homogenous DivRem methods already exists in Math, but these non-homogenous also (hopefully) conveys the inputs' and outputs' sizes of the operations better than before.
• Operations and flow in existing methods stays the same, just using the new helper methods to (hopefully) improve maintainability.

[dotnet-policy-service]

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

[lilinus]

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.

[danmoseley]

I don't know how to compare benchmarks thought, to see there are no regressions in performance.

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.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-numerics
See info in area-owners.md if you want to be subscribed.

Issue Details

---

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.

Author:	lilinus
Assignees:	-
Labels:	area-System.Numerics, needs-area-label
Milestone:	-

[tannergooding]

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 ulong / uint and similar are areas that the JIT should be able to recognize and optimize itself, allowing this to light up anywhere its used and not require customized logic that only works for System.Decimal.

This would be particularly relevant for Arm64 which doesn't have specialized instructions allowing uint128 / uint64 or similar.

[lilinus]

These really seems like some basic JIT optimizations we're missing and not something we should be adding hardcoded helpers around. Cases of ulong / uint and similar are areas that the JIT should be able to recognize and optimize itself, allowing this to light up anywhere its used and not require customized logic that only works for System.Decimal.

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:

• Make code more readable and easier to follow / maintain using the helpers. IMO it's just easier to follow what is going on with the divide operators with carries with these introduced helpers.
• Homogenous DivRem methods already exists in Math, but these non-homogenous also (hopefully) conveys the inputs' and outputs' sizes of the operations better than before.
• Operations and flow in existing methods stays the same, just using the new helper methods to (hopefully) improve maintainability.

[dotnet-policy-service]

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

[lilinus]

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.

[danmoseley]

I don't know how to compare benchmarks thought, to see there are no regressions in performance.

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.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-numerics
See info in area-owners.md if you want to be subscribed.

Issue Details

---

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.

Author:	lilinus
Assignees:	-
Labels:	area-System.Numerics, needs-area-label
Milestone:	-

[tannergooding]

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 ulong / uint and similar are areas that the JIT should be able to recognize and optimize itself, allowing this to light up anywhere its used and not require customized logic that only works for System.Decimal.

This would be particularly relevant for Arm64 which doesn't have specialized instructions allowing uint128 / uint64 or similar.

[lilinus]

These really seems like some basic JIT optimizations we're missing and not something we should be adding hardcoded helpers around. Cases of ulong / uint and similar are areas that the JIT should be able to recognize and optimize itself, allowing this to light up anywhere its used and not require customized logic that only works for System.Decimal.

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:

• Make code more readable and easier to follow / maintain using the helpers. IMO it's just easier to follow what is going on with the divide operators with carries with these introduced helpers.
• Homogenous DivRem methods already exists in Math, but these non-homogenous also (hopefully) conveys the inputs' and outputs' sizes of the operations better than before.
• Operations and flow in existing methods stays the same, just using the new helper methods to (hopefully) improve maintainability.

[dotnet-policy-service]

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.