JIT: Strange redunant 'cmp' in Unsafe.As<TFrom, TTo>() codegen #64532
Comments
dotnet-issue-labeler
bot
added
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT Issue DetailsSpotted in .NET 6.0.* and .NET 7.0.100-preview.2.22078.1: Sample code: public unsafe struct Test
{
private int[] A;
public int Sum_GetArrayDataReferenceCalled()
{
var s = 0;
var l = (nuint)A.Length;
ref var ar = ref MemoryMarshal.GetArrayDataReference(A);
for(nuint i = 0; i < l; i++)
{
s += Unsafe.Add(ref ar, i);
}
return s;
}
// This version is just the same as above but has a manually inlined 'MemoryMarshal.GetArrayDataReference'
// allowing us to narrow down (I believe) the source of the issue
public int Sum_GetArrayDataReferenceInlined()
{
var s = 0;
var l = (nuint)A.Length;
var a = A;
ref var d = ref Unsafe.As<int[], RawArrayData>(ref a); // <---- ???
ref var ar = ref Unsafe.As<byte, int>(ref d.Data);
for(nuint i = 0; i < l; i++)
{
s += Unsafe.Add(ref ar, i);
}
return s;
}
private sealed class RawArrayData
{
public uint Length;
public uint Padding;
public byte Data;
}
}Result codegen: ; ================================================================================
; Test.Sum_GetArrayDataReferenceCalled()
; 35 (0x23) bytes
; 13 (0xD) instructions
; var s = 0;
xor eax,eax
; var l = (nuint)A.Length;
mov rdx,[rcx]
mov ecx,[rdx+8]
; ref var ar = ref MemoryMarshal.GetArrayDataReference(A);
cmp [rdx],edx
add rdx,10h
; for(nuint i = 0; i < l; i++)
; ^^^^^^^^^^^
xor r8d,r8d
; for(nuint i = 0; i < l; i++)
; ^^^^^
test rcx,rcx
jbe LBL_1
LBL_0:
; s += Unsafe.Add(ref ar, i);
add eax,[rdx+r8*4]
; for(nuint i = 0; i < l; i++)
; ^^^
inc r8
; for(nuint i = 0; i < l; i++)
; ^^^^^
cmp r8,rcx
jb LBL_0
LBL_1:
; return s;
ret
; ================================================================================
; Test.Sum_GetArrayDataReferenceInlined()
; 35 (0x23) bytes
; 13 (0xD) instructions
; var s = 0;
xor eax,eax
; var l = (nuint)A.Length;
mov rdx,[rcx]
mov ecx,[rdx+8]
; ref var d = ref Unsafe.As<int[], RawArrayData>(ref a); // <---- ???
; ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
cmp [rdx],edx
add rdx,10h
; for(nuint i = 0; i < l; i++)
; ^^^^^^^^^^^
xor r8d,r8d
; for(nuint i = 0; i < l; i++)
; ^^^^^
test rcx,rcx
jbe LBL_1
LBL_0:
; s += Unsafe.Add(ref ar, i);
add eax,[rdx+r8*4]
; for(nuint i = 0; i < l; i++)
; ^^^
inc r8
; for(nuint i = 0; i < l; i++)
; ^^^^^
cmp r8,rcx
jb LBL_0
LBL_1:
; return s;
ret
|
|
This is an interesting issue. A bit smaller reproduction: [MethodImpl(MethodImplOptions.NoInlining)]
private ref int Problem(ClassWithFields a)
{
_ = a.ObjWithField.First;
return ref a.ObjWithField.First;
}
sealed class ClassWithFields
{
public int First;
public ClassWithFields ObjWithField;
}First, we start in morph, where we have to add a null check for Fortunately, VN does work with indirections, and gives both ***** BB01
STMT00000 ( 0x000[E-] ... 0x00B )
N013 ( 8, 8) [000004] -A-XG------- * COMMA void <l:$301, c:$300>
N011 ( 8, 8) [000002] -A-XG------- +--* IND int <l:$244, c:$243>
N010 ( 6, 6) [000011] -A-XG--N---- | \--* ADD byref <l:$144, c:$143>
N008 ( 5, 5) [000030] -A-XG------- | +--* COMMA ref <l:$203, c:$202>
N006 ( 4, 4) [000028] -A-XG---R--- | | +--* ASG ref $VN.Void
N005 ( 1, 1) [000027] D------N---- | | | +--* LCL_VAR ref V04 cse0 d:1 <l:$203, c:$202>
N004 ( 4, 4) [000001] ---XG------- | | | \--* IND ref <l:$203, c:$202>
N003 ( 2, 2) [000013] -------N---- | | | \--* ADD byref $140
N001 ( 1, 1) [000000] ------------ | | | +--* LCL_VAR ref V01 arg1 u:1 $80
N002 ( 1, 1) [000012] ------------ | | | \--* CNS_INT long 16 field offset Fseq[ObjWithField] $100
N007 ( 1, 1) [000029] ------------ | | \--* LCL_VAR ref V04 cse0 u:1 <l:$203, c:$202>
N009 ( 1, 1) [000010] ------------ | \--* CNS_INT long 32 field offset Fseq[First] $101
N012 ( 0, 0) [000003] ------------ \--* NOP void $2c0
***** BB01
STMT00001 ( 0x00C[E-] ... 0x017 )
N011 ( 7, 9) [000009] -A-XG------- * RETURN byref <l:$210, c:$20e>
N010 ( 6, 8) [000022] -A-XG--N---- \--* COMMA byref <l:$147, c:$146>
N006 ( 3, 5) [000025] -A-XG------- +--* COMMA void <l:$210, c:$20e>
N003 ( 1, 3) [000015] -A--G---R--- | +--* ASG ref <l:$203, c:$208>
N002 ( 1, 1) [000014] D------N---- | | +--* LCL_VAR ref V03 tmp1 d:1 <l:$200, c:$85>
N001 ( 1, 1) [000031] ------------ | | \--* LCL_VAR ref V04 cse0 u:1 <l:$200, c:$81>
N005 ( 2, 2) [000017] ---X---N---- | \--* NULLCHECK byte <l:$20c, c:$20b>
N004 ( 1, 1) [000016] ------------ | \--* LCL_VAR ref V03 tmp1 u:1 <l:$200, c:$85>
N009 ( 3, 3) [000021] ------------ \--* ADD byref <l:$142, c:$145>
N007 ( 1, 1) [000019] ------------ +--* LCL_VAR ref V03 tmp1 u:1 (last use) <l:$200, c:$85>
N008 ( 1, 1) [000020] ------------ \--* CNS_INT long 32 field offset Fseq[First] $101
-------------------------------------------------------------------------------------------------------------------
GenTreeNode creates assertion:
N004 ( 4, 4) [000001] ---XG------- * IND ref <l:$203, c:$202>
In BB01 New Global Constant Assertion: ($80,$0) V01.01 != null, index = #01
GenTreeNode creates assertion:
N005 ( 2, 2) [000017] ---X---N---- * NULLCHECK byte <l:$20c, c:$20b>
In BB01 New Global Constant Assertion: ($85,$0) V03.01 != null, index = #02Ultimately, the issue is that CSE "improves" the transient closure of conservative VNs for the expressions it replaces, and we do not take advantage of that (except in some specific cases with array lengths and bounds checks). It is not clear to me if there's a trivial fix for this. |
|
cc @jakobbotsch for VN and CSE related issue. |
JulieLeeMSFT
removed
the
untriaged
|
Was this fixed due intrinsification of |
Spotted in .NET 6.0.* and .NET 7.0.100-preview.2.22078.1:
Sample code:
Result codegen:
https://sharplab.io/#v2:EYLgxg9gTgpgtADwGwBYA0AXEBDAzgWwB8ABAJgEYBYAKGIAYACY8gOgBkBLAOwEcBuGvSbkUA2o2YsASgFcuGDvhgsAwhHwAHDgBsYUAMp6AbhzAxcYoZNnzFygJLy9EDYagmzFwRNYA5GUpQpl7iwtJyCkosjhhBXLjBLAAaABxIlj7htlExcQlgIYIAzAxyuNgAZjAMuLEyYBgMACrmGDQA3gCQNJ0aQUbYGNXcGADaALoMAIJi3dSdxCUjDPoBAPoA4jAYU1BQ2ACeACKD2FIwVbBcZirY2roAJgAUAJQMPe09nQNQNQwAvAw6LNvthftoAQwnlwZCMXlN2DAuABzDAACxBsAqDB+DDBkKxDAAsjB8NADkSwbg0XcWFsdntDicMGcLnokWYnlMXiCKtBobD5AwOJDgcKGAAeBjaPjCgDUcreX0+806nVwDDlgIAqvFKsopg9noSwWhhTyvgBfL7EADsNVm1vmPQA9C7mmiOBqjHoEhAuMKNQArGS1Bjo6rlJR4jXYYAQH0MYAyRo02MMfDYGF3bQHYVcbTcGAPBgAchJZKgFKpNO0dO2u32x1O50uHJgpfe8zdePuEAA7txkaUNRgIAwuGCoAOGA8BwGnvYkzBCzAfW8IzUIDIoGYGBBsZuvbgZDAeot841VvhNg3Gc2Wa32dcYI5C1xi68u50VWrcRrAWBL5cQhQEBThBE2CRVEMWA/FsEhGYvkJXES0BQldXKKoWCmXAJRGCYzSkbB+0bJlTgAPieE0eQYHtJTgRi4AYAB+NjkIuHF4N+dDOMw/UcLw4ADiGM0RiowkHhYZlsAtVUvj5KBwKFEVANlEUpRleVFW/H8vnVTUdT1bDDWNTjTXNEEnTVO0HR6ayej6DgBiGGoYDuYsmFIBhiNI+8ZI+G0SkFRooJRdEQQvEKGAABWwI0h0ikphNcmTHRoS0gA
category:cq
theme:codegen
skill-level:beginner
cost:small
impact:medium
The text was updated successfully, but these errors were encountered: