Skip to content

Simplify arithmetic operations on registry and memory in ssa#125885

Open
pedrobsaila wants to merge 10 commits intodotnet:mainfrom
pedrobsaila:125300-ssa
Open

Simplify arithmetic operations on registry and memory in ssa#125885
pedrobsaila wants to merge 10 commits intodotnet:mainfrom
pedrobsaila:125300-ssa

Conversation

@pedrobsaila
Copy link
Contributor

@pedrobsaila pedrobsaila commented Mar 21, 2026 •
edited
Loading

Fixes #125300

Alternate version of #125559 (cc @EgorBo )

Before :

Program:AddReg(int):int (FullOpts):
       lea      eax, [rdi+2*rdi]
       ret      

Program:AddMem(byref):int (FullOpts):
       mov      eax, dword ptr [rdi]
       lea      ecx, [rax+rax]
       add      eax, ecx
       ret      

Program:SubReg(int):int (FullOpts):
       xor      eax, eax
       sub      eax, edi
       ret      

Program:SubMem(byref):int (FullOpts):
       mov      eax, dword ptr [rdi]
       mov      ecx, eax
       sub      ecx, eax
       sub      ecx, eax
       mov      eax, ecx
       ret

After:

Program:AddReg(int):int (FullOpts):
       lea      eax, [rdi+2*rdi]
       ret      

Program:AddMem(byref):int (FullOpts):
       mov      eax, dword ptr [rdi]
       lea      eax, [rax+2*rax]
       ret      

Program:SubReg(int):int (FullOpts):
       mov      eax, ecx
       neg      eax
       ret      

Program:SubMem(byref):int (FullOpts):
       mov      eax, dword ptr [rdi]
       neg      eax
       ret

@github-actions github-actions bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 21, 2026
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Mar 21, 2026
@dotnet-policy-service
Copy link
Contributor

dotnet-policy-service bot commented Mar 21, 2026

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

@pedrobsaila pedrobsaila mentioned this pull request Mar 21, 2026
Open
@EgorBo
Copy link
Member

EgorBo commented Mar 21, 2026

I haven't looked into correctness of these changes, but the diffs look like a PerfScore regression accross all collections?

@pedrobsaila
Copy link
Contributor Author

pedrobsaila commented Mar 21, 2026 •
edited
Loading

One of the things that I don't understand in asm diffs is this :

       lea      eax, [rax+rax+0x03]
						;; size=29 bbWeight=1 PerfScore 5.25

less size than this :

       lea      eax, [2*rax+0x03]
						;; size=32 bbWeight=1 PerfScore 4.75

Is there a logical explanation for it ?

For the perf score diffs I'm working on fixing them

EgorBo
EgorBo reviewed Mar 21, 2026
View reviewed changes
src/coreclr/jit/assertionprop.cpp
op1->AsOp()->gtOp1->AsOp()->gtOp1->AsLclVar()->GetLclNum() == lclNum)) &&
op1->AsOp()->gtOp2->OperIs(GT_CNS_INT)))
{
return gtNewOperNode(GT_MUL, tree->TypeGet(), op1->AsOp()->gtOp1,
Copy link
Member

@EgorBo EgorBo Mar 21, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

TBH, I am not sure what SSA in the PR title means - you just moved morph logic to this place which doesn't make sense to me, morph is supposed to be called from this phase (if it is not, we can experiment calling it unconditionally as today it might be called only if we constant folded something or changed based on assertions).

this function is a good place to, in theory, to rely on SSA, but the current PR doesn't do it.

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

Reviewers

@EgorBo EgorBo EgorBo left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

JIT: Simplify arithmetic operations on memory, as is already done for register

2 participants

@pedrobsaila @EgorBo