Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

arm64: Optimise GT/GE/LT/LE comparisons to constant zero - #119758

#119758
Merged
EgorBo merged 10 commits into
dotnet:maindotnet/runtime:mainfrom
jonathandavies-arm:upstream/ce/flag-zerojonathandavies-arm/runtime:upstream/ce/flag-zeroCopy head branch name to clipboard
Oct 20, 2025
Merged

arm64: Optimise GT/GE/LT/LE comparisons to constant zero#119758
EgorBo merged 10 commits into
dotnet:maindotnet/runtime:mainfrom
jonathandavies-arm:upstream/ce/flag-zerojonathandavies-arm/runtime:upstream/ce/flag-zeroCopy head branch name to clipboard

Conversation

@jonathandavies-arm

Copy link
Copy Markdown
Contributor

No description provided.

@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 Sep 16, 2025
@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Sep 16, 2025
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@jonathandavies-arm
jonathandavies-arm marked this pull request as ready for review September 29, 2025 09:57
@jonathandavies-arm jonathandavies-arm changed the title [DRAFT] arm64: Optimise GT/GE/LT/LE comparisons to constant zero arm64: Optimise GT/GE/LT/LE comparisons to constant zero Sep 29, 2025
@jonathandavies-arm

Copy link
Copy Markdown
Contributor Author

Please can I have a review @EgorBo?
There was an old PR with this work (#112863) and this is mostly the same except that I fixed the issue with the tests.

@dotnet/arm64-contrib

@xtqqczze

Copy link
Copy Markdown
Contributor

Diffs show a small number of regressions, e.g.:

; System.IO.Tests.File_NotifyFilter_Tests:FileSystemWatcher_File_NotifyFilter_Size(int):this (FullOpts)
@@ -160,11 +160,13 @@ G_M49668_IG03:        ; bbWeight=0.50, gcVars=0000000000008000 {V03}, gcrefRegs=
             movk    x0, #1 LSL #32
             str     x0, [x19, #0x18]
             mov     w23, wzr
-            and     w0, w20, #124
             cmp     w20, #8
             ccmp    w20, #16, z, ne
-            ccmp    w0, #0, 0, ne
-            bgt     G_M49668_IG04
+            cset    x0, eq
+            ands    w1, w20, #124
+            cset    x1, gt
+            orr     w0, w0, w1
+            cbnz    w0, G_M49668_IG04
             movz    x0, #0xD1FFAB1E      // code for System.PlatformDetection:get_IsWindows7():bool
             movk    x0, #0xD1FFAB1E LSL #16
             movk    x0, #1 LSL #32
@@ -174,7 +176,7 @@ G_M49668_IG03:        ; bbWeight=0.50, gcVars=0000000000008000 {V03}, gcrefRegs=
             cbz     w0, G_M49668_IG05
             cmp     w20, #4
             bne     G_M49668_IG05
-						;; size=132 bbWeight=0.50 PerfScore 12.75
+						;; size=140 bbWeight=0.50 PerfScore 13.25

@EgorBo EgorBo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for adding tests!

@AndyAyersMS

Copy link
Copy Markdown
Member

@EgorBo if this is ready can we merge?

@xtqqczze

Copy link
Copy Markdown
Contributor

@MihuBot -arm

@EgorBo
EgorBo merged commit 5fc165a into dotnet:main Oct 20, 2025
118 checks passed
@xtqqczze

Copy link
Copy Markdown
Contributor

Diffs show a small number of regressions, e.g.:

; System.IO.Tests.File_NotifyFilter_Tests:FileSystemWatcher_File_NotifyFilter_Size(int):this (FullOpts)
@@ -160,11 +160,13 @@ G_M49668_IG03:        ; bbWeight=0.50, gcVars=0000000000008000 {V03}, gcrefRegs=
             movk    x0, #1 LSL #32
             str     x0, [x19, #0x18]
             mov     w23, wzr
-            and     w0, w20, #124
             cmp     w20, #8
             ccmp    w20, #16, z, ne
-            ccmp    w0, #0, 0, ne
-            bgt     G_M49668_IG04
+            cset    x0, eq
+            ands    w1, w20, #124
+            cset    x1, gt
+            orr     w0, w0, w1
+            cbnz    w0, G_M49668_IG04
             movz    x0, #0xD1FFAB1E      // code for System.PlatformDetection:get_IsWindows7():bool
             movk    x0, #0xD1FFAB1E LSL #16
             movk    x0, #1 LSL #32
@@ -174,7 +176,7 @@ G_M49668_IG03:        ; bbWeight=0.50, gcVars=0000000000008000 {V03}, gcrefRegs=
             cbz     w0, G_M49668_IG05
             cmp     w20, #4
             bne     G_M49668_IG05
-						;; size=132 bbWeight=0.50 PerfScore 12.75
+						;; size=140 bbWeight=0.50 PerfScore 13.25

@EgorBo Which optimization is being missed here, TryLowerAndOrToCCMP?

@EgorBo

EgorBo commented Oct 20, 2025

Copy link
Copy Markdown
Member

Diffs show a small number of regressions, e.g.:

; System.IO.Tests.File_NotifyFilter_Tests:FileSystemWatcher_File_NotifyFilter_Size(int):this (FullOpts)
@@ -160,11 +160,13 @@ G_M49668_IG03:        ; bbWeight=0.50, gcVars=0000000000008000 {V03}, gcrefRegs=
             movk    x0, #1 LSL #32
             str     x0, [x19, #0x18]
             mov     w23, wzr
-            and     w0, w20, #124
             cmp     w20, #8
             ccmp    w20, #16, z, ne
-            ccmp    w0, #0, 0, ne
-            bgt     G_M49668_IG04
+            cset    x0, eq
+            ands    w1, w20, #124
+            cset    x1, gt
+            orr     w0, w0, w1
+            cbnz    w0, G_M49668_IG04
             movz    x0, #0xD1FFAB1E      // code for System.PlatformDetection:get_IsWindows7():bool
             movk    x0, #0xD1FFAB1E LSL #16
             movk    x0, #1 LSL #32
@@ -174,7 +176,7 @@ G_M49668_IG03:        ; bbWeight=0.50, gcVars=0000000000008000 {V03}, gcrefRegs=
             cbz     w0, G_M49668_IG05
             cmp     w20, #4
             bne     G_M49668_IG05
-						;; size=132 bbWeight=0.50 PerfScore 12.75
+						;; size=140 bbWeight=0.50 PerfScore 13.25

@EgorBo Which optimization is being missed here, TryLowerAndOrToCCMP?

hm.. i do not recall this, last round of SPMI was a clean improvement if my memory is not failing me?

@xtqqczze

Copy link
Copy Markdown
Contributor

hm.. i do not recall this, last round of SPMI was a clean improvement if my memory is not failing me?

I think there were 3-4 similar regressions, the build has since been deleted.

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 community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.