Use Unsafe.BitCast for Interlocked.{Compare}Exchange of float/double #87166

huoyaoyuan · 2023-06-06T11:17:10Z

Codegen for coreclr:

        private static float x;
        private static int i;

        public float Variable(float a, float b) => Interlocked.CompareExchange(ref x, a, b);

        public float Constant() => Interlocked.CompareExchange(ref x, 1.0f, 2.0f);

        public float IntField() => Interlocked.CompareExchange(ref Unsafe.As<int, float>(ref i), 1.0f, 2.0f);

; Method CSPlayground.Program:Variable(float,float):float:this
G_M23030_IG01:  ;; offset=0000H
       vzeroupper 
						;; size=3 bbWeight=1 PerfScore 1.00

G_M23030_IG02:  ;; offset=0003H
       mov      rdx, 0x7FF8C0D428B8      ; data for CSPlayground.Program:x
       vmovd    ecx, xmm1
       vmovd    eax, xmm2
       lock     
       cmpxchg  dword ptr [rdx], ecx
       vmovd    xmm0, eax
						;; size=26 bbWeight=1 PerfScore 24.25

G_M23030_IG03:  ;; offset=001DH
       ret

; Method CSPlayground.Program:Constant():float:this
G_M63042_IG01:  ;; offset=0000H
       vzeroupper 
						;; size=3 bbWeight=1 PerfScore 1.00

G_M63042_IG02:  ;; offset=0003H
       mov      rdx, 0x7FF8C0D328B8      ; data for CSPlayground.Program:x
       mov      ecx, 0x3F800000
       mov      eax, 0x40000000
       lock     
       cmpxchg  dword ptr [rdx], ecx
       vmovd    xmm0, eax
						;; size=28 bbWeight=1 PerfScore 20.75

G_M63042_IG03:  ;; offset=001FH
       ret      
						;; size=1 bbWeight=1 PerfScore 1.00

; Method CSPlayground.Program:IntField():float:this
G_M13357_IG01:  ;; offset=0000H
       vzeroupper 
						;; size=3 bbWeight=1 PerfScore 1.00

G_M13357_IG02:  ;; offset=0003H
       mov      rdx, 0x7FF8C0D328BC      ; data for CSPlayground.Program:i
       mov      ecx, 0x3F800000
       mov      eax, 0x40000000
       lock     
       cmpxchg  dword ptr [rdx], ecx
       vmovd    xmm0, eax
						;; size=28 bbWeight=1 PerfScore 20.75

G_M13357_IG03:  ;; offset=001FH
       ret

I don't know how to see codegen for mono/nativeaot, but as long as they implement Unsafe.As/BitCast correctly this would be very simple.

ghost · 2023-06-06T11:17:32Z

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

Issue Details

Codegen for coreclr:

        private static float x;
        private static int i;

        public float Variable(float a, float b) => Interlocked.CompareExchange(ref x, a, b);

        public float Constant() => Interlocked.CompareExchange(ref x, 1.0f, 2.0f);

        public float IntField() => Interlocked.CompareExchange(ref Unsafe.As<int, float>(ref i), 1.0f, 2.0f);

; Method CSPlayground.Program:Variable(float,float):float:this
G_M23030_IG01:  ;; offset=0000H
       vzeroupper 
						;; size=3 bbWeight=1 PerfScore 1.00

G_M23030_IG02:  ;; offset=0003H
       mov      rdx, 0x7FF8C0D428B8      ; data for CSPlayground.Program:x
       vmovd    ecx, xmm1
       vmovd    eax, xmm2
       lock     
       cmpxchg  dword ptr [rdx], ecx
       vmovd    xmm0, eax
						;; size=26 bbWeight=1 PerfScore 24.25

G_M23030_IG03:  ;; offset=001DH
       ret

; Method CSPlayground.Program:Constant():float:this
G_M63042_IG01:  ;; offset=0000H
       vzeroupper 
						;; size=3 bbWeight=1 PerfScore 1.00

G_M63042_IG02:  ;; offset=0003H
       mov      rdx, 0x7FF8C0D328B8      ; data for CSPlayground.Program:x
       mov      ecx, 0x3F800000
       mov      eax, 0x40000000
       lock     
       cmpxchg  dword ptr [rdx], ecx
       vmovd    xmm0, eax
						;; size=28 bbWeight=1 PerfScore 20.75

G_M63042_IG03:  ;; offset=001FH
       ret      
						;; size=1 bbWeight=1 PerfScore 1.00

; Method CSPlayground.Program:IntField():float:this
G_M13357_IG01:  ;; offset=0000H
       vzeroupper 
						;; size=3 bbWeight=1 PerfScore 1.00

G_M13357_IG02:  ;; offset=0003H
       mov      rdx, 0x7FF8C0D328BC      ; data for CSPlayground.Program:i
       mov      ecx, 0x3F800000
       mov      eax, 0x40000000
       lock     
       cmpxchg  dword ptr [rdx], ecx
       vmovd    xmm0, eax
						;; size=28 bbWeight=1 PerfScore 20.75

G_M13357_IG03:  ;; offset=001FH
       ret

I don't know how to see codegen for mono/nativeaot, but as long as they implement Unsafe.As/BitCast correctly this would be very simple.

Author:	huoyaoyuan
Assignees:	-
Labels:	`area-System.Threading`, `community-contribution`
Milestone:	-

huoyaoyuan · 2023-06-06T11:18:04Z

src/libraries/System.Private.CoreLib/src/System/Threading/Interlocked.cs

+        /// <param name="comparand">The value that is compared to the value at <paramref name="location1"/>.</param>
+        /// <returns>The original value in <paramref name="location1"/>.</returns>
+        /// <exception cref="NullReferenceException">The address of <paramref name="location1"/> is a null pointer.</exception>
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]


AggressiveInlining wasn't shown necessary in my first explorations, added just for consistency with uint/ulong ones.

huoyaoyuan · 2023-06-06T11:24:01Z

Codegen for ARM64:

        public float Constant() => Interlocked.CompareExchange(ref x, 1.0f, 2.0f);

        public int ConstantInteger() => Interlocked.CompareExchange(ref i, 1, 2);

; Method CSPlayground.Program:ConstantInteger():int:this
G_M53319_IG01:  ;; offset=0000H
            stp     fp, lr, [sp, #-0x10]!
            mov     fp, sp
						;; size=8 bbWeight=1 PerfScore 1.50

G_M53319_IG02:  ;; offset=0008H
            movz    x0, #0x28BC      // data for CSPlayground.Program:i
            movk    x0, #0xC0D2 LSL #16
            movk    x0, #0x7FF8 LSL #32
            mov     w1, #1
            mov     w2, #2
            casal   w2, w1, [x0]
            mov     w0, w2
						;; size=28 bbWeight=1 PerfScore 6.00

G_M53319_IG03:  ;; offset=0024H
            ldp     fp, lr, [sp], #0x10
            ret     lr
						;; size=8 bbWeight=1 PerfScore 2.00
; Total bytes of code: 44

; Method CSPlayground.Program:Constant():float:this
G_M63042_IG01:  ;; offset=0000H
            stp     fp, lr, [sp, #-0x10]!
            mov     fp, sp
						;; size=8 bbWeight=1 PerfScore 1.50

G_M63042_IG02:  ;; offset=0008H
            movz    x0, #0x28B8      // data for CSPlayground.Program:x
            movk    x0, #0xC0D3 LSL #16
            movk    x0, #0x7FF8 LSL #32
            mov     w1, #0x3F800000
            mov     w2, #0x40000000
            casal   w2, w1, [x0]
            fmov    s0, w2
						;; size=28 bbWeight=1 PerfScore 6.50

G_M63042_IG03:  ;; offset=0024H
            ldp     fp, lr, [sp], #0x10
            ret     lr
						;; size=8 bbWeight=1 PerfScore 2.00
; Total bytes of code: 44

I don't really understand ARM asm, but since the patterns are almost the same, I think there's no issue.

vargaz · 2023-06-06T12:55:19Z

The mono changes look ok.

EgorBo · 2023-06-06T12:58:27Z

I'd check codegen for x86/arm32 for the double one.
Although, should be fine

huoyaoyuan · 2023-06-06T13:31:51Z

Double codegen for x86:

G_M6963_IG01:  ;; offset=0000H
       push     ebp
       mov      ebp, esp
       sub      esp, 32
       vzeroupper 
						;; size=9 bbWeight=1 PerfScore 2.50

G_M6963_IG02:  ;; offset=0009H
       vmovsd   xmm0, qword ptr [ebp+10H]
       vmovsd   qword ptr [ebp-08H], xmm0
       mov      ecx, dword ptr [ebp-08H]
       mov      eax, dword ptr [ebp-04H]
       vmovsd   xmm0, qword ptr [ebp+08H]
       vmovsd   qword ptr [ebp-10H], xmm0
       push     eax
       push     ecx
       push     dword ptr [ebp-0CH]
       push     dword ptr [ebp-10H]
       mov      ecx, 0x7FF8BFC728B8      ; data for CSPlayground.Program:x
       call     System.Threading.Interlocked:CompareExchange(byref,long,long):long
       mov      dword ptr [ebp-18H], eax
       mov      dword ptr [ebp-14H], edx
       vmovsd   xmm0, qword ptr [ebp-18H]
       vmovsd   qword ptr [ebp-20H], xmm0
       fld      qword ptr [ebp-20H]
						;; size=63 bbWeight=1 PerfScore 21.75

G_M6963_IG03:  ;; offset=0048H
       mov      esp, ebp
       pop      ebp
       ret      16
						;; size=6 bbWeight=1 PerfScore 2.75

; Method CSPlayground.Program:Constant():double:this
G_M36327_IG01:  ;; offset=0000H
       push     ebp
       mov      ebp, esp
       sub      esp, 16
       vzeroupper 
						;; size=9 bbWeight=1 PerfScore 2.50

G_M36327_IG02:  ;; offset=0009H
       push     0x3FF00000
       push     0
       push     0x40000000
       push     0
       mov      ecx, 0x7FF8BFC528B8      ; data for CSPlayground.Program:x
       call     System.Threading.Interlocked:CompareExchange(byref,long,long):long
       mov      dword ptr [ebp-08H], eax
       mov      dword ptr [ebp-04H], edx
       vmovsd   xmm0, qword ptr [ebp-08H]
       vmovsd   qword ptr [ebp-10H], xmm0
       fld      qword ptr [ebp-10H]
						;; size=43 bbWeight=1 PerfScore 11.75

G_M36327_IG03:  ;; offset=0034H
       mov      esp, ebp
       pop      ebp
       ret      
						;; size=4 bbWeight=1 PerfScore 1.75
; Total bytes of code: 56

The call to CompareExchange isn't inlined, since it's an FCall?

jkotas

Thank you

EgorBo · 2023-06-06T16:40:01Z

CI is not feeling well

The call to CompareExchange isn't inlined, since it's an FCall?

yes, that's expected

MichalPetryka · 2023-06-06T18:01:50Z

I'd check codegen for x86/arm32 for the double one. Although, should be fine

double codegen will likely change with #85562 since that makes it partially handled as an intrinsic in bitcast.

src/libraries/System.Private.CoreLib/src/System/Threading/Interlocked.cs

src/mono/mono/metadata/threads.c

huoyaoyuan added 6 commits June 6, 2023 14:00

Update {Compare}Exchange in coreclr to use Unsafe.As

1372442

Remove corresponding FCalls

91bf45c

Share with mono

c7673f0

Remove Mono icalls

35a6b88

Add AggressiveInlining

a9bbe6d

nativeaot

c4b9f5f

huoyaoyuan requested review from lateralusX, lambdageek, vargaz, thaystg, marek-safar and MichalStrehovsky as code owners June 6, 2023 11:17

ghost added the community-contribution Indicates that the PR has been added by a community member label Jun 6, 2023

dotnet-issue-labeler bot added the area-System.Threading label Jun 6, 2023

huoyaoyuan commented Jun 6, 2023

View reviewed changes

jkotas approved these changes Jun 6, 2023

View reviewed changes

EgorBo approved these changes Jun 6, 2023

View reviewed changes

build-analysis bot mentioned this pull request Jun 6, 2023

Tracking issue for CI build timeouts #76454

Closed

tannergooding reviewed Jun 6, 2023

View reviewed changes

src/libraries/System.Private.CoreLib/src/System/Threading/Interlocked.cs Show resolved Hide resolved

jkotas reviewed Jun 7, 2023

View reviewed changes

src/mono/mono/metadata/threads.c Show resolved Hide resolved

jkotas reviewed Jun 7, 2023

View reviewed changes

src/mono/mono/metadata/threads.c Show resolved Hide resolved

Remove unused union type in mono

0358183

jkotas merged commit 1ade285 into dotnet:main Jun 7, 2023

huoyaoyuan deleted the interlocked-bitcast branch June 8, 2023 01:21

ghost locked as resolved and limited conversation to collaborators Jul 8, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use Unsafe.BitCast for Interlocked.{Compare}Exchange of float/double #87166

Use Unsafe.BitCast for Interlocked.{Compare}Exchange of float/double #87166

huoyaoyuan commented Jun 6, 2023

ghost commented Jun 6, 2023

huoyaoyuan Jun 6, 2023

huoyaoyuan commented Jun 6, 2023

vargaz commented Jun 6, 2023

EgorBo commented Jun 6, 2023 •

edited

Loading

huoyaoyuan commented Jun 6, 2023

jkotas left a comment

EgorBo commented Jun 6, 2023

MichalPetryka commented Jun 6, 2023

Use Unsafe.BitCast for Interlocked.{Compare}Exchange of float/double #87166

Use Unsafe.BitCast for Interlocked.{Compare}Exchange of float/double #87166

Conversation

huoyaoyuan commented Jun 6, 2023

ghost commented Jun 6, 2023

huoyaoyuan Jun 6, 2023

Choose a reason for hiding this comment

huoyaoyuan commented Jun 6, 2023

vargaz commented Jun 6, 2023

EgorBo commented Jun 6, 2023 • edited Loading

huoyaoyuan commented Jun 6, 2023

jkotas left a comment

Choose a reason for hiding this comment

EgorBo commented Jun 6, 2023

MichalPetryka commented Jun 6, 2023

EgorBo commented Jun 6, 2023 •

edited

Loading