ARM64 - Optimizing a % b operations

[TIHan]

Addressing part of this issue: #34937

Description

There are various ways to optimize % for integers on ARM64.

a % b can be transformed into a & (b - 1) if they are unsigned integers and b is a constant with the power of 2.

Acceptance Criteria

• Add Tests (asmdiffs cover this)

[ghost]

Tagging subscribers to this area: @JulieLeeMSFT
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Addressing this issue: #34937

Description

There are various ways to optimize % for integers on ARM64.

Example:
a % b can be transformed into a & (b - 1) if they are unsigned integers and b is a constant with the power of 2.

Acceptance Criteria

• Add signed int mod optimization with known constant
• Add signed int mod optimization without a known constant
• Add Tests

Author:	TIHan
Assignees:	-
Labels:	area-CodeGen-coreclr
Milestone:	-

[EgorBo]

Shouldn't we just remove early expansion of UMOD/MOD for arm64 from morph and use the shared (with x86) impl in lower instead?

[TIHan]

@EgorBo are you referring to Lowering::LowerConstIntDivOrMod ?

[EgorBo]

LowerConstIntDivOrMod

yeah, and just move mod to a % b = a - (a / b) * b; there - thus, we won't have to re-implement the already existing optimization.

One potential problem with this approach that it might produce regressions where a - (a / b) * b previously led to more CSE opportunities, e.g. with a / b next to a % b

[TIHan]

It makes sense that we should do it there so the earlier phases don't screw it up.

[TIHan]

I did some work to see if I could move the existing mod optimizations to lowering, but it might be a bit much for what the PR is trying to accomplish.

[tannergooding]

Shouldn't we just remove early expansion of UMOD/MOD for arm64 from morph and use the shared (with x86) impl in lower instead

@EgorBo, I would've thought it was better to do it early in morph so other things can more easily take advantage of the optimization (we don't optimize around div/rem in many cases)?

[kunalspathak]

@EgorBo, I would've thought it was better to do it early in morph so other things can more easily take advantage of the optimization (we don't optimize around div/rem in many cases)?

I agree.

[EgorBo]

Shouldn't we just remove early expansion of UMOD/MOD for arm64 from morph and use the shared (with x86) impl in lower instead

@EgorBo, I would've thought it was better to do it early in morph so other things can more easily take advantage of the optimization (we don't optimize around div/rem in many cases)?

cc @kunalspathak @tannergooding

I personally think it's not, for any non-leaf X in X % Y we have to introduce a new local (ASG node) instead of just keeping a simple x mod y expression, e.g (x + 1) % y early in morph is converted into:

[000015] -A-X-+--R---              \--*  SUB       int   
[000012] -----+------                 +--*  LCL_VAR   int    V03 tmp1         
[000014] -A-X-+------                 \--*  MUL       int   
[000004] -A-X-+------                    +--*  DIV       int   
[000011] -A---+------                    |  +--*  COMMA     int   
[000009] -A---+------                    |  |  +--*  ASG       int   
[000008] D----+-N----                    |  |  |  +--*  LCL_VAR   int    V03 tmp1         
[000002] -----+------                    |  |  |  \--*  ADD       int   
[000000] -----+------                    |  |  |     +--*  LCL_VAR   int    V00 arg0         
[000001] -----+------                    |  |  |     \--*  CNS_INT   int    1
[000010] -----+------                    |  |  \--*  LCL_VAR   int    V03 tmp1         
[000003] -----+------                    |  \--*  LCL_VAR   int    V01 arg1         
[000013] -----+------                    \--*  LCL_VAR   int    V01 arg1         

instead of just:

[000004] ---X--------              \--*  MOD       int   
[000002] ------------                 +--*  ADD       int   
[000000] ------------                 |  +--*  LCL_VAR   int    V00 arg0         
[000001] ------------                 |  \--*  CNS_INT   int    1
[000003] ------------                 \--*  LCL_VAR   int    V01 arg1         

E.g. it makes it non-hoistable for ARM64, e.g. see this:

loops are highlighted for both x64 and arm64

[tannergooding]

@EgorBo, I was referring specifically to the x % SomePow2 optimization being introduced here.

It should be a clear improvement to recognize and replace x % SomePow2 with x & (SomePow2 - 1) since that's the same number of nodes, still a constant, but also AND is better understood and optimized than DIV or MOD

[EgorBo]

@EgorBo, I was referring specifically to the x % SomePow2 optimization being introduced here.

It should be a clear improvement to recognize and replace x % SomePow2 with x & (SomePow2 - 1) since that's the same number of nodes, still a constant, but also AND is better understood and optimized than DIV or MOD

I'm fine with doing x umod POT early in morph - it makes sense and doesn't produce additional local, I was referring to my suggestion to remove the early expansion of general X [u]mod Y

[TIHan]

@kunalspathak @echesakovMSFT This is ready.

Will try to restart CI.

[kunalspathak]

LGTM with nice diffs.