Skip to content

Conversation

@jonathandavies-arm
Copy link
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
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
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
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

Copy link
Member

@EgorBo EgorBo left a comment

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
Member

@EgorBo if this is ready can we merge?

@xtqqczze
Copy link
Contributor

@MihuBot -arm

@EgorBo EgorBo merged commit 5fc165a into dotnet:main Oct 20, 2025
118 checks passed
@xtqqczze
Copy link
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
Copy link
Member

EgorBo commented Oct 20, 2025

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
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 join this conversation on GitHub. Already have an account? Sign in to comment

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