Simplify arithmetic operations on registry and memory

[pedrobsaila]

Fixes #125300

Lacks tests, working on it

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 

[dotnet-policy-service]

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

[EgorBo]

haven't checked the correctness of these changes, but the diffs imply that +300 LOC to JIT are not worth it.

[pedrobsaila]

There is room to reduce LOC, I just wrote it fast to see whether their will be asm diffs or not. I'll roll more compact version in a few hours. But I agree the diffs are not that impressive.

[EgorBo]

There is room to reduce LOC, I just wrote it fast to see whether their will be asm diffs or not. I'll roll more compact version in a few hours. But I agree the diffs are not that impressive.

Right. Generally, we don't accept PRs like that unless there is a strong justification. We see it as an extra risk that is not worth the impact. To be honest, all kinds of x + x + x .. transformation should be handled elsewhere, on VN/SSA level.

[pedrobsaila]

To be honest, all kinds of x + x + x .. transformation should be handled elsewhere, on VN/SSA level.

Sorry to bother you, but never worked on VN/SSA level, I need to ask you about which file the transformation should be valuenum.cpp/ssabuilder.cpp or elsewhere ?

[EgorBo]

To be honest, all kinds of x + x + x .. transformation should be handled elsewhere, on VN/SSA level.

Sorry to bother you, but never worked on VN/SSA level, I need to ask you about which file the transformation should be valuenum.cpp/ssabuilder.cpp or elsewhere ?

I just tried myself with VN and it didn't find diffs either (mostly because we don't materialize VNs today). With SSA it's probably a more advanced task. probably like in copyprop or assertionprop, but it might also not worth it - hard to tell in advance.

[tannergooding]

Do we have any info on whether the lack of diffs is because this is pattern is uncommon in all code or simply uncommon in the code that we have SPMI diffs around (i.e. corelib and partner frameworks/apps that tend to be a bit more explicit about our patterns)?

[pedrobsaila]

/azp run runtime-coreclr superpmi-diffs

[azure-pipelines]

Commenter does not have sufficient privileges for PR 125559 in repo dotnet/runtime

[pedrobsaila]

@EgorBo can you tell me if I'm going in the right direction this time ? I know I still have 4 failing tests, just want to see if I'm not doing something wrong before investing more time.