RyuJIT: Don't emit cast helpers for (T)array.Clone()

[EgorBo]

Closes #45302

Example:

int[] CloneArray(int[] a) => (int[])a.Clone();

master:

G_M38234_IG01:
       sub      rsp, 40
G_M38234_IG02:
       cmp      dword ptr [rdx], edx
       mov      rcx, rdx
       call     System.Object:MemberwiseClone():System.Object:this
       mov      rdx, rax
       mov      rcx, 0xD1FFAB1E
       call     CORINFO_HELP_CHKCASTARRAY
       nop      
G_M38234_IG03:
       add      rsp, 40
       ret

PR:

G_M38234_IG01:
       sub      rsp, 40
G_M38234_IG02:
       cmp      dword ptr [rdx], edx
       mov      rcx, rdx
       call     System.Object:MemberwiseClone():System.Object:this
       nop      
G_M38234_IG03:
       add      rsp, 40
       ret

Jit-diff (--pmi):

C:\prj>jit-diff diff --output C:\prj\jit-diffs --cctors -f --core_root runtime\artifacts\tests\coreclr\windows.x64.Release\Tests\Core_Root --base C:\prj\runtime\artifacts\bin\coreclr\windows.x64.Checked_base --diff C:\prj\runtime\artifacts\bin\coreclr\windows.x64.Checked --pmi
Beginning PMI CodeSize Diffs for System.Private.CoreLib.dll, framework assemblies
/ Finished 268/268 Base 268/268 Diff [508.4 sec]
Completed PMI CodeSize Diffs for System.Private.CoreLib.dll, framework assemblies [invoking .cctors] in 508.59s
Diffs (if any) can be viewed by comparing: C:\prj\jit-diffs\dasmset_5\base C:\prj\jit-diffs\dasmset_5\diff

git diff --no-index --diff-filter=M --exit-code --numstat C:\prj\jit-diffs\dasmset_5\diff C:\prj\jit-diffs\dasmset_5\base

Analyzing CodeSize diffs...
Found 36 files with textual diffs.

PMI CodeSize Diffs for System.Private.CoreLib.dll, framework assemblies [invoking .cctors] for  default jit

Summary of Code Size diffs:
(Lower is better)

Total bytes of base: 49740193
Total bytes of diff: 49736462
Total bytes of delta: -3731 (-0.01% of base)
    diff is an improvement.

Top file improvements (bytes):
        -947 : System.Private.CoreLib.dasm (-0.02% of base)
        -576 : FSharp.Core.dasm (-0.02% of base)
        -378 : System.Numerics.Tensors.dasm (-0.15% of base)
        -288 : System.Security.Cryptography.Algorithms.dasm (-0.09% of base)
        -224 : Microsoft.CodeAnalysis.dasm (-0.01% of base)
        -108 : System.Collections.Immutable.dasm (-0.01% of base)
        -108 : System.ComponentModel.TypeConverter.dasm (-0.04% of base)
        -108 : System.Threading.Tasks.Dataflow.dasm (-0.01% of base)
         -90 : System.Security.Cryptography.Primitives.dasm (-0.25% of base)
         -90 : System.Web.HttpUtility.dasm (-0.83% of base)
         -72 : Microsoft.VisualBasic.Core.dasm (-0.02% of base)
         -72 : System.Private.Xml.dasm (-0.00% of base)
         -72 : System.Runtime.Numerics.dasm (-0.10% of base)
         -54 : System.Security.Claims.dasm (-0.24% of base)
         -54 : System.Security.Cryptography.Csp.dasm (-0.10% of base)
         -54 : System.Security.Cryptography.Xml.dasm (-0.03% of base)
         -40 : System.Reflection.Metadata.dasm (-0.01% of base)
         -36 : System.CodeDom.dasm (-0.02% of base)
         -36 : System.DirectoryServices.AccountManagement.dasm (-0.01% of base)
         -36 : System.Reflection.MetadataLoadContext.dasm (-0.02% of base)

36 total files with Code Size differences (36 improved, 0 regressed), 232 unchanged.

Top method improvements (bytes):
        -234 (-12.09% of base) : System.Private.CoreLib.dasm - HashSet`1:ConstructFrom(HashSet`1):this (7 methods)
        -126 (-3.37% of base) : FSharp.Core.dasm - Array:stableSortWithKeysAndComparer(IComparer`1,IComparer`1,ref,ref) (7 methods)
        -126 (-5.53% of base) : System.Numerics.Tensors.dasm - CompressedSparseTensor`1:.ctor(ReadOnlySpan`1,int,bool):this (7 methods)
        -126 (-9.09% of base) : System.Numerics.Tensors.dasm - CompressedSparseTensor`1:.ctor(Memory`1,Memory`1,Memory`1,int,ReadOnlySpan`1,bool):this (7 methods)
        -126 (-1.53% of base) : System.Numerics.Tensors.dasm - CompressedSparseTensor`1:.ctor(Array,bool):this (7 methods)
        -108 (-14.84% of base) : FSharp.Core.dasm - ArrayModule:Copy(ref):ref (7 methods)
        -108 (-4.27% of base) : FSharp.Core.dasm - SeqModule:ToArray(IEnumerable`1):ref (7 methods)
        -108 (-8.34% of base) : FSharp.Core.dasm - Array:stableSortInPlace(ref) (7 methods)
        -108 (-10.41% of base) : FSharp.Core.dasm - Array:stableSortInPlaceWith(FSharpFunc`2,ref) (7 methods)
        -108 (-17.20% of base) : System.Collections.Immutable.dasm - ImmutableArrayExtensions:ToArray(ImmutableArray`1):ref (7 methods)
        -108 (-21.34% of base) : System.Threading.Tasks.Dataflow.dasm - ImmutableArray`1:ToArray():ref:this (7 methods)
        -103 (-12.50% of base) : Microsoft.CodeAnalysis.dasm - SmallDictionary`2:LeftComplex(AvlNode):AvlNode (8 base, 7 diff methods)
        -103 (-12.50% of base) : Microsoft.CodeAnalysis.dasm - SmallDictionary`2:RightComplex(AvlNode):AvlNode (8 base, 7 diff methods)
         -54 (-8.44% of base) : System.ComponentModel.TypeConverter.dasm - PropertyTabAttribute:InitializeArrays(ref,ref,ref):this
         -54 (-0.52% of base) : System.Private.CoreLib.dasm - DefaultBinder:BindToMethod(int,ref,byref,ref,CultureInfo,ref,byref):MethodBase:this
         -40 (-23.26% of base) : System.Reflection.Metadata.dasm - BlobHeap:GetVirtualBlobBytes(BlobHandle,bool):ref:this
         -36 (-13.53% of base) : System.Private.CoreLib.dasm - DateTimeFormatInfo:GetMergedPatterns(ref,String):ref
         -36 (-23.08% of base) : System.Security.Cryptography.Algorithms.dasm - Aes:.ctor():this
         -36 (-24.32% of base) : System.Security.Cryptography.Algorithms.dasm - DES:.ctor():this
         -36 (-24.32% of base) : System.Security.Cryptography.Algorithms.dasm - RC2:.ctor():this

Top method improvements (percentages):
         -18 (-51.43% of base) : System.Security.Cryptography.Algorithms.dasm - KeySizeHelpers:CloneKeySizesArray(ref):ref
         -18 (-51.43% of base) : System.Security.Cryptography.Csp.dasm - KeySizeHelpers:CloneKeySizesArray(ref):ref
         -18 (-46.15% of base) : FSharp.Core.dasm - CompilationArgumentCountsAttribute:get_Counts():IEnumerable`1:this
         -18 (-46.15% of base) : Microsoft.CSharp.dasm - ComTypeEnumDesc:GetMemberNames():ref:this
         -18 (-46.15% of base) : System.CodeDom.dasm - CompilerInfo:GetLanguages():ref:this
         -18 (-46.15% of base) : System.CodeDom.dasm - CompilerInfo:GetExtensions():ref:this
         -18 (-46.15% of base) : System.Net.NetworkInformation.dasm - PhysicalAddress:GetAddressBytes():ref:this
         -18 (-46.15% of base) : System.Private.CoreLib.dasm - ApplicationId:get_PublicKeyToken():ref:this
         -18 (-46.15% of base) : System.Private.CoreLib.dasm - NumberFormatInfo:get_CurrencyGroupSizes():ref:this
         -18 (-46.15% of base) : System.Private.CoreLib.dasm - NumberFormatInfo:get_NumberGroupSizes():ref:this
         -18 (-46.15% of base) : System.Private.CoreLib.dasm - NumberFormatInfo:get_PercentGroupSizes():ref:this
         -18 (-46.15% of base) : System.Private.CoreLib.dasm - SortKey:get_KeyData():ref:this
         -18 (-46.15% of base) : System.Private.CoreLib.dasm - SignatureConstructedGenericType:get_GenericTypeArguments():ref:this
         -18 (-46.15% of base) : System.Security.Cryptography.Algorithms.dasm - ECDiffieHellmanPublicKey:ToByteArray():ref:this
         -18 (-46.15% of base) : System.Security.Cryptography.Primitives.dasm - AsymmetricAlgorithm:get_LegalKeySizes():ref:this
         -18 (-46.15% of base) : System.Security.Cryptography.Primitives.dasm - SymmetricAlgorithm:get_LegalBlockSizes():ref:this
         -18 (-46.15% of base) : System.Security.Cryptography.Primitives.dasm - SymmetricAlgorithm:get_LegalKeySizes():ref:this
         -18 (-42.86% of base) : System.Private.CoreLib.dasm - NumberFormatInfo:get_NativeDigits():ref:this
         -18 (-41.86% of base) : System.Reflection.Context.dasm - VirtualPropertyBase:GetIndexParameters():ref:this
         -18 (-40.00% of base) : System.Private.CoreLib.dasm - DateTimeFormatInfo:get_MonthGenitiveNames():ref:this

120 total methods with Code Size differences (120 improved, 0 regressed), 258751 unchanged.
Completed analysis in 29.25s

/cc @GrabYourPitchforks @VSadov

[EgorBo]

Failures are not related (#45317)

[VSadov]

// op1 - value to cast

Nit: should rename to "tree" now?

---

Refers to: src/coreclr/src/jit/importer.cpp:10719 in ce5ac35. [](commit_id = ce5ac357154f11e14ad6d4889ae215e49456b44f, deletion_comment = False)

[VSadov]

LGTM

[ViktorHofer]

// Auto-generated message

69e114c which was merged 12/7 removed the intermediate src/coreclr/src/ folder. This PR needs to be updated as it touches files in that directory which causes conflicts.

To update your commits you can use this bash script: https://gist.github.com/ViktorHofer/6d24f62abdcddb518b4966ead5ef3783. Feel free to use the comment section of the gist to improve the script for others.

[EgorBo]

@dotnet/jit-contrib can this be merged?

[AndyAyersMS]

Perhaps teach gtGetClassHandle about this intrinsic instead? That might convey some extra benefits...

[AndyAyersMS]

Are you going to re-run diffs?

[EgorBo]

Perhaps teach gtGetClassHandle about this intrinsic instead? That might convey some extra benefits...

Good point, it looks nicer now

Are you going to re-run diffs?

Yeah just finished - the diff is exactly the same.

[AndyAyersMS]

Looks good -- just one small comment.

[AndyAyersMS]

Testing hit another case of the xunit assertion #11063.

Unhandled exception. System.InvalidOperationException: Collection was modified after the enumerator was instantiated.   at System.Collections.Generic.Stack`1.Enumerator.MoveNext()
   at Xunit.Sdk.DisposalTracker.Dispose() in C:\Dev\xunit\xunit\src\xunit.execution\Sdk\DisposalTracker.cs:line 26

am going to rerun these.

[AndyAyersMS]

Thanks Egor!

[GrabYourPitchforks]

A random question about this line:

objClass = gtGetClassHandle(call->gtCallThisArg->GetNode(), pIsExact, pIsNonNull);

I assume it's ok that objClass might technically be a lie? For example...

object[] arr1 = GetArray(); // actually a string[]!
object[] clone1 = (object[])arr1.Clone(); // also actually a string[]!

int[] arr2 = GetArray(); // actually a uint[]!
int[] clone2 = (int[])arr2.Clone(); // also actually a uint[]!

// etc.

If I then call clone2.GetType(), the JIT won't try to over-optimize this, and it'll correctly return typeof(uint[]) instead of typeof(int[])?

[pentp]

Could the same logic be applied to MemberwiseClone also?

[AndyAyersMS]

I assume it's ok that objClass might technically be a lie?

We're fairly cautious when reasoning about types unless we know the exact type (say from seeing a constructor call or an exact type test). And even when we do know the exact type we're cautious with arrays of primitive types:

runtime/src/coreclr/jit/importer.cpp

Lines 21629 to 21663 in fcf0b8b

	//------------------------------------------------------------------------
	// impIsClassExact: check if a class handle can only describe values
	// of exactly one class.
	//
	// Arguments:
	// classHnd - handle for class in question
	//
	// Returns:
	// true if class is final and not subject to special casting from
	// variance or similar.
	//
	// Note:
	// We are conservative on arrays of primitive types here.
	bool Compiler::impIsClassExact(CORINFO_CLASS_HANDLE classHnd)
	{
	DWORD flags = info.compCompHnd->getClassAttribs(classHnd);
	DWORD flagsMask = CORINFO_FLG_FINAL | CORINFO_FLG_VARIANCE | CORINFO_FLG_ARRAY;
	if ((flags & flagsMask) == CORINFO_FLG_FINAL)
	{
	return true;
	}
	if ((flags & flagsMask) == (CORINFO_FLG_FINAL | CORINFO_FLG_ARRAY))
	{
	CORINFO_CLASS_HANDLE arrayElementHandle = nullptr;
	CorInfoType type = info.compCompHnd->getChildType(classHnd, &arrayElementHandle);
	if ((type == CORINFO_TYPE_CLASS) || (type == CORINFO_TYPE_VALUECLASS))
	{
	return impIsClassExact(arrayElementHandle);
	}
	}
	return false;
	}

In the above we would not know the exact types for arr1 and arr2 and so partial knowledge of those types would not allow optimizing a subsequent GetType().

[AndyAyersMS]

Could the same logic be applied to MemberwiseClone also?

Seems like it could, yes. @EgorBo want to follow up on this?