Remove decimal alignment quirk from the type loader #38603

jkotas · 2020-06-30T04:58:07Z

System.Decimal fields did not match Win32 DECIMAL type for historic reasons. It required type loader to have a quirk to artifically inflate System.Decimal alignment to match the alignment of Win32 DECIMAL type to make interop work well.

This change is fixing the System.Decimal fields to match Win32 DECIMAL type and removing the quirk from the type loader since it does not belong there.

The downsides are:

Slightly lower code quality on 32-bit platforms. 32-bit platforms are not our high performance targets anymore.
Explicit implementation of ISerializable is required on System.Decimal for binary serialization compatibility.

Fixes #38390

Dotnet-GitSync-Bot · 2020-06-30T04:58:11Z

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

jkotas · 2020-06-30T05:18:18Z

src/libraries/System.Private.CoreLib/src/System/Decimal.cs

+        // match Win32 DECIMAL type.
+        private readonly int _flags;
+        private readonly uint _hi32;
+        private readonly ulong _lo64;

        // Constructs a Decimal from an integer value.
        //
        public Decimal(int value)


Begin 00007FFE9E193740, size 1e

00007ffe`9e193740 85d2 test edx,edx 00007ffe`9e193742 7c06 jl System_Private_CoreLib!System.Decimal..ctor(Int32)+0xffffffff`a273ea2a (00007ffe`9e19374a) 00007ffe`9e193744 33c0 xor eax,eax 00007ffe`9e193746 8901 mov dword ptr [rcx],eax 00007ffe`9e193748 eb08 jmp System_Private_CoreLib!System.Decimal..ctor(Int32)+0xffffffff`a273ea32 (00007ffe`9e193752) 00007ffe`9e19374a c70100000080 mov dword ptr [rcx],80000000h 00007ffe`9e193750 f7da neg edx 00007ffe`9e193752 895108 mov dword ptr [rcx+8],edx 00007ffe`9e193755 33c0 xor eax,eax 00007ffe`9e193757 89410c mov dword ptr [rcx+0Ch],eax 00007ffe`9e19375a 894104 mov dword ptr [rcx+4],eax 00007ffe`9e19375d c3 ret

After:

One less memory access

Instruction count and code size is the same

Begin 00007FFE94E4B6B0, size 1e 00007ffe`94e4b6b0 85d2 test edx,edx 00007ffe`94e4b6b2 7c06 jl System_Private_CoreLib!System.Decimal..ctor(Int32)+0x6a (00007ffe`94e4b6ba) 00007ffe`94e4b6b4 33c0 xor eax,eax 00007ffe`94e4b6b6 8901 mov dword ptr [rcx],eax 00007ffe`94e4b6b8 eb08 jmp System_Private_CoreLib!System.Decimal..ctor(Int32)+0x72 (00007ffe`94e4b6c2) 00007ffe`94e4b6ba c70100000080 mov dword ptr [rcx],80000000h 00007ffe`94e4b6c0 f7da neg edx 00007ffe`94e4b6c2 8bc2 mov eax,edx 00007ffe`94e4b6c4 48894108 mov qword ptr [rcx+8],rax 00007ffe`94e4b6c8 33c0 xor eax,eax 00007ffe`94e4b6ca 894104 mov dword ptr [rcx+4],eax 00007ffe`94e4b6cd c3 ret

jkotas · 2020-06-30T05:24:14Z

src/libraries/System.Private.CoreLib/src/System/Decimal.cs

-                    mid = bits[1];
-                    hi = bits[2];
-                    flags = f;
+                    _lo64 = (uint)bits[0] + ((ulong)(uint)bits[1] << 32);


Before:

Begin 00007FFE9E155DD0, size a5 00007ffe`9e155dd0 56 push rsi 00007ffe`9e155dd1 4883ec20 sub rsp,20h 00007ffe`9e155dd5 488b02 mov rax,qword ptr [rdx] 00007ffe`9e155dd8 8b5208 mov edx,dword ptr [rdx+8] 00007ffe`9e155ddb 83fa04 cmp edx,4 00007ffe`9e155dde 7546 jne System_Private_CoreLib!System.Decimal..ctor(System.ReadOnlySpan`1<Int32>)+0xffffffff`a0ec0ec6 (00007ffe`9e155e26) 00007ffe`9e155de0 8b500c mov edx,dword ptr [rax+0Ch] 00007ffe`9e155de3 f7c2ffff007f test edx,7F00FFFFh 00007ffe`9e155de9 753b jne System_Private_CoreLib!System.Decimal..ctor(System.ReadOnlySpan`1<Int32>)+0xffffffff`a0ec0ec6 (00007ffe`9e155e26) 00007ffe`9e155deb 448bc2 mov r8d,edx 00007ffe`9e155dee 4181e00000ff00 and r8d,0FF0000h 00007ffe`9e155df5 4181f800001c00 cmp r8d,1C0000h 00007ffe`9e155dfc 410f96c0 setbe r8b 00007ffe`9e155e00 450fb6c0 movzx r8d,r8b 00007ffe`9e155e04 4585c0 test r8d,r8d 00007ffe`9e155e07 741d je System_Private_CoreLib!System.Decimal..ctor(System.ReadOnlySpan`1<Int32>)+0xffffffff`a0ec0ec6 (00007ffe`9e155e26) 00007ffe`9e155e09 448b00 mov r8d,dword ptr [rax] 00007ffe`9e155e0c 44894108 mov dword ptr [rcx+8],r8d 00007ffe`9e155e10 448b4004 mov r8d,dword ptr [rax+4] 00007ffe`9e155e14 4489410c mov dword ptr [rcx+0Ch],r8d 00007ffe`9e155e18 8b4008 mov eax,dword ptr [rax+8] 00007ffe`9e155e1b 894104 mov dword ptr [rcx+4],eax 00007ffe`9e155e1e 8911 mov dword ptr [rcx],edx 00007ffe`9e155e20 4883c420 add rsp,20h 00007ffe`9e155e24 5e pop rsi 00007ffe`9e155e25 c3 ret 00007ffe`9e155e26 48b998fb1d9efe7f0000 mov rcx,7FFE9E1DFB98h (MT: System.ArgumentException) 00007ffe`9e155e30 e8aba6bf5f call coreclr!JIT_TrialAllocSFastMP_InlineGetThread (00007ffe`fdd504e0) 00007ffe`9e155e35 488bf0 mov rsi,rax 00007ffe`9e155e38 b9113b0000 mov ecx,3B11h 00007ffe`9e155e3d 48ba2040019efe7f0000 mov rdx,7FFE9E014020h 00007ffe`9e155e47 e89477b25f call coreclr!JIT_StrCns (00007ffe`fdc7d5e0) 00007ffe`9e155e4c 488bc8 mov rcx,rax 00007ffe`9e155e4f 48ba6030f1c0d0020000 mov rdx,2D0C0F13060h 00007ffe`9e155e59 488b12 mov rdx,qword ptr [rdx] 00007ffe`9e155e5c e8cffcffff call CLRStub[MethodDescPrestub]@7ffe9e155b30 (00007ffe`9e155b30) (System.SR.GetResourceString(System.String, System.String), mdToken: 00000000060013F8) 00007ffe`9e155e61 488bd0 mov rdx,rax 00007ffe`9e155e64 488bce mov rcx,rsi 00007ffe`9e155e67 e80c1effff call CLRStub[MethodDescPrestub]@7ffe9e147c78 (00007ffe`9e147c78) (System.ArgumentException..ctor(System.String), mdToken: 0000000006000AFE) 00007ffe`9e155e6c 488bce mov rcx,rsi 00007ffe`9e155e6f e86c36b95f call coreclr!IL_Throw (00007ffe`fdce94e0) 00007ffe`9e155e74 cc int 3

After:

One less memory access

One extra instruction

Begin 00007FFE9B89BE10, size a8 00007ffe`9b89be10 56 push rsi 00007ffe`9b89be11 4883ec20 sub rsp,20h 00007ffe`9b89be15 488b02 mov rax,qword ptr [rdx] 00007ffe`9b89be18 8b5208 mov edx,dword ptr [rdx+8] 00007ffe`9b89be1b 83fa04 cmp edx,4 00007ffe`9b89be1e 7549 jne System_Private_CoreLib!System.Decimal..ctor(System.ReadOnlySpan`1<Int32>)+0x6b9 (00007ffe`9b89be69) 00007ffe`9b89be20 8b500c mov edx,dword ptr [rax+0Ch] 00007ffe`9b89be23 f7c2ffff007f test edx,7F00FFFFh 00007ffe`9b89be29 753e jne System_Private_CoreLib!System.Decimal..ctor(System.ReadOnlySpan`1<Int32>)+0x6b9 (00007ffe`9b89be69) 00007ffe`9b89be2b 448bc2 mov r8d,edx 00007ffe`9b89be2e 4181e00000ff00 and r8d,0FF0000h 00007ffe`9b89be35 4181f800001c00 cmp r8d,1C0000h 00007ffe`9b89be3c 410f96c0 setbe r8b 00007ffe`9b89be40 450fb6c0 movzx r8d,r8b 00007ffe`9b89be44 4585c0 test r8d,r8d 00007ffe`9b89be47 7420 je System_Private_CoreLib!System.Decimal..ctor(System.ReadOnlySpan`1<Int32>)+0x6b9 (00007ffe`9b89be69) 00007ffe`9b89be49 448b00 mov r8d,dword ptr [rax] 00007ffe`9b89be4c 448b4804 mov r9d,dword ptr [rax+4] 00007ffe`9b89be50 49c1e120 shl r9,20h 00007ffe`9b89be54 4d03c1 add r8,r9 00007ffe`9b89be57 4c894108 mov qword ptr [rcx+8],r8 00007ffe`9b89be5b 8b4008 mov eax,dword ptr [rax+8] 00007ffe`9b89be5e 894104 mov dword ptr [rcx+4],eax 00007ffe`9b89be61 8911 mov dword ptr [rcx],edx 00007ffe`9b89be63 4883c420 add rsp,20h 00007ffe`9b89be67 5e pop rsi 00007ffe`9b89be68 c3 ret 00007ffe`9b89be69 48b9b082919bfe7f0000 mov rcx,7FFE9B9182B0h (MT: System.ArgumentException) 00007ffe`9b89be73 e8782ebd5f call CoreCLR!JIT_TrialAllocSFastMP_InlineGetThread (00007ffe`fb46ecf0) 00007ffe`9b89be78 488bf0 mov rsi,rax 00007ffe`9b89be7b b9e6390000 mov ecx,39E6h 00007ffe`9b89be80 48ba2040749bfe7f0000 mov rdx,7FFE9B744020h 00007ffe`9b89be8a e8815abb5f call CoreCLR!JIT_StrCns (00007ffe`fb451910) 00007ffe`9b89be8f 488bc8 mov rcx,rax 00007ffe`9b89be92 48ba60307a9140010000 mov rdx,140917A3060h 00007ffe`9b89be9c 488b12 mov rdx,qword ptr [rdx] 00007ffe`9b89be9f e884e4feff call CLRStub[MethodDescPrestub]@7ffe9b88a328 (00007ffe`9b88a328) (System.SR.GetResourceString(System.String, System.String), mdToken: 0000000006001453) 00007ffe`9b89bea4 488bd0 mov rdx,rax 00007ffe`9b89bea7 488bce mov rcx,rsi 00007ffe`9b89beaa e849c0feff call CLRStub[MethodDescPrestub]@7ffe9b887ef8 (00007ffe`9b887ef8) (System.ArgumentException..ctor(System.String), mdToken: 0000000006000AFD) 00007ffe`9b89beaf 488bce mov rcx,rsi 00007ffe`9b89beb2 e88905b55f call CoreCLR!IL_Throw (00007ffe`fb3ec440) 00007ffe`9b89beb7 cc int 3

jkotas · 2020-06-30T05:33:10Z

The perf impact on 64-bit platforms should be in the noise level. I have shared a few examples above. The general pattern is that that are fewer memory accesses and more bit manipulations.

jkotas · 2020-06-30T05:36:41Z

cc @dotnet/crossgen-contrib

System.Decimal fields did not match Win32 DECIMAL type for historic reasons. It required type loader to have a quirk to artifically inflate System.Decimal alignment to match the alignment of Win32 DECIMAL type to make interop work well. This change is fixing the System.Decimal fields to match Win32 DECIMAL type and removing the quirk from the type loader since it does not belong there. The downsides are: - Slightly lower code quality on 32-bit platforms. 32-bit platforms are not our high performance targets anymore. - Explicit implementation of ISerializable is required on System.Decimal for binary serialization compatibility. Fixes dotnet#38390

AaronRobinsonMSFT · 2020-06-30T05:41:44Z

src/libraries/System.Private.CoreLib/src/System/Decimal.cs

@@ -182,6 +176,28 @@ public Decimal(double value)
            DecCalc.VarDecFromR8(value, out AsMutable(ref this));
        }

+        private Decimal(SerializationInfo info, StreamingContext context)


Do we have a Decimal serialization test?

Yep, we have number of cases for decimal in https://raw.githubusercontent.com/dotnet/runtime/master/src/libraries/System.Runtime.Serialization.Formatters/tests/BinaryFormatterTestData.cs . They were failing before this part of the fix.

src/coreclr/src/vm/methodtablebuilder.cpp

Dotnet-GitSync-Bot added area-VM-coreclr new-api-needs-documentation labels Jun 30, 2020

jkotas commented Jun 30, 2020

View reviewed changes

jkotas mentioned this pull request Jun 30, 2020

Fix System.Decimal aligment on ARM32 with crossgen2 #38390

Closed

jkotas removed the new-api-needs-documentation label Jun 30, 2020

jkotas force-pushed the decimal branch from 4726cf3 to f3a2d46 Compare June 30, 2020 05:38

AaronRobinsonMSFT approved these changes Jun 30, 2020

View reviewed changes

MichalStrehovsky reviewed Jun 30, 2020

View reviewed changes

src/coreclr/src/vm/methodtablebuilder.cpp Show resolved Hide resolved

jkotas added 3 commits June 30, 2020 01:32

Feedback

ddb2a30

Delete more unused names

0954618

Merge branch 'master' into decimal

3b0d832

jaredpar mentioned this pull request Jun 30, 2020

Errors installing the SDK during builds #34015

Closed

jkotas merged commit 1ea4166 into dotnet:master Jun 30, 2020

jkotas deleted the decimal branch June 30, 2020 16:45

ghost locked as resolved and limited conversation to collaborators Dec 8, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove decimal alignment quirk from the type loader #38603

Remove decimal alignment quirk from the type loader #38603

jkotas commented Jun 30, 2020

Dotnet-GitSync-Bot commented Jun 30, 2020

jkotas Jun 30, 2020

jkotas Jun 30, 2020

jkotas commented Jun 30, 2020

jkotas commented Jun 30, 2020

AaronRobinsonMSFT Jun 30, 2020

jkotas Jun 30, 2020 •

edited

Loading

Remove decimal alignment quirk from the type loader #38603

Remove decimal alignment quirk from the type loader #38603

Conversation

jkotas commented Jun 30, 2020

Dotnet-GitSync-Bot commented Jun 30, 2020

jkotas Jun 30, 2020

Choose a reason for hiding this comment

jkotas Jun 30, 2020

Choose a reason for hiding this comment

jkotas commented Jun 30, 2020

jkotas commented Jun 30, 2020

AaronRobinsonMSFT Jun 30, 2020

Choose a reason for hiding this comment

jkotas Jun 30, 2020 • edited Loading

Choose a reason for hiding this comment

jkotas Jun 30, 2020 •

edited

Loading