Skip to content
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

Optimize nullcheck for Interlocked.* operations #87297

Closed
wants to merge 4 commits into from

Conversation

EgorBo
Copy link
Member

@EgorBo EgorBo commented Jun 8, 2023

Closes #44087

int _field;

void Test() => Interlocked.Increment(ref _field);
; Assembly listing for method Program:Test():this
-      cmp      byte  ptr [rcx], cl
       add      rcx, 8
       lock     
       add      dword ptr [rcx], 1
       ret      
-; Total bytes of code 11
+; Total bytes of code 9

No need for an extra nullcheck when interlocked operation is an implicit nullcheck itself. Follows #44087 (comment) advice

@ghost ghost assigned EgorBo Jun 8, 2023
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jun 8, 2023
@ghost
Copy link

ghost commented Jun 8, 2023

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

Issue Details
int _field;

void Test() => Interlocked.Increment(ref _field);
; Assembly listing for method Program:Test():this
-      cmp      byte  ptr [rcx], cl
       add      rcx, 8
       lock     
       add      dword ptr [rcx], 1
       ret      
-; Total bytes of code 9
+; Total bytes of code 11

No need for extra nullcheck when interlocked operation is an implicit nullcheck itself. Follows #44087 (comment) advice

Author: EgorBo
Assignees: EgorBo
Labels:

area-CodeGen-coreclr

Milestone: -

@AndyAyersMS
Copy link
Member

How hard would it be to build an address mode here too?

@AndyAyersMS
Copy link
Member

Also maybe take a look at this regression? Look like maybe we don't propagate non-null assertions from interlocked ops or something?

;; x64 libraries_tests.pmi
+45 (+7.85%) : 314641.dasm - System.Reflection.Internal.Tests.AbstractMemoryBlockTests+<>c__DisplayClass6_0:<DisposeThreadSafety>b__0():this

@EgorBo
Copy link
Member Author

EgorBo commented Jul 16, 2023

Will take a look this week

@EgorBo EgorBo added this to the 9.0.0 milestone Aug 5, 2023
@EgorBo EgorBo closed this Aug 27, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Sep 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RyuJIT] Unnecessary nullcheck in Exchange_long benchmark
2 participants