JIT should optimize away the cast that occurs after Array.Clone

[GrabYourPitchforks]

The typical way to clone an array is to use Array.Clone, which returns an object that needs to be cast back to the original array type.

string[] originalArray = GetArray();
string[] clonedArray = (string[])originalArray.Clone();

After the call to Clone, the JIT emits a call to the ChkCastAny helper to ensure that the cast is legitimate.

public static object[] M(object[] o) {
    return (Object[])o.Clone();
}

M(System.Object[])
    L0000: sub rsp, 0x28
    L0004: mov eax, [rcx]
    L0006: call 0x00007ffb1bc0e0c0  ;  Array.Clone()
    L000b: mov rdx, rax
    L000e: mov rcx, 0x7ffabc1af762  ;  type handle for object[]
    L0018: call 0x00007ffb1bc16750  ;  ChkCastAny
    L001d: nop
    L001e: add rsp, 0x28
    L0022: ret

There is an opportunity for improvement here. If the target of the Array.Clone call is known by the JIT to be a local of type T, and if the return value of Clone is immediately cast to T or type U (where reinterpret_cast<U>(T) is legal), then the JIT should no-op the cast. In the sample above, that means labels L000b - L001d would disappear.

Some examples:

string[] stringArray;
string[] clone1 = (string[])stringArray.Clone(); // JIT should elide check
object[] clone2 = (object[])stringArray.Clone(); // JIT should elide check

object[] objectArray;
string[] clone3 = (string[])objectArray.Clone(); // check should take place, could succeed or fail at runtime
object[] clone4 = (object[])objectArray.Clone(); // JIT should elide check

int[] intArray;
int[] clone5 = (int[])intArray.Clone(); // JIT should elide check
uint[] clone6 = (uint[])intArray.Clone(); // JIT should elide check
object[] clone7 = (object[])intArray.Clone(); // check should take place and will fail at runtime

[sharwell]

Can we not use covariant return types here, to make the return type of string[].Clone be string[] instead of just object?

[benaadams]

Can we not use covariant return types here,

Array is a bit of a funny type and doesn't have any generic instance methods to change the return type on?

[EgorBo]

I think it should be easy to fix in JIT, given a tree:

\--*  CALL help ref    HELPER.CORINFO_HELP_CHKCASTARRAY
   +--*  CNS_INT(h) long   0xd1ffab1e class
   \--*  CALL      ref    System.Object.MemberwiseClone  ;;; should be marked as [Intrinsic]
      \--*  LCL_VAR   ref    V00 arg0

drop the helper if arg0's type and that CNS_INT handle return TypeCompareState::Must via info.compCompHnd->compareTypesForCast (if it doesn't give up on arrays)

[EgorBo]

Possible fix: EgorBo@43282cd

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: 49740090
Total bytes of diff: 49736642
Total bytes of delta: -3448 (-0.01% of base)
    diff is an improvement.

Top file improvements (bytes):
        -857 : System.Private.CoreLib.dasm (-0.02% of base)
        -558 : 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)
        -121 : Microsoft.CodeAnalysis.dasm (-0.01% of base)
        -108 : System.Collections.Immutable.dasm (-0.01% of base)
        -108 : System.Threading.Tasks.Dataflow.dasm (-0.01% of base)
         -90 : System.ComponentModel.TypeConverter.dasm (-0.03% 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.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)
         -18 : Microsoft.CSharp.dasm (-0.00% of base)

35 total files with Code Size differences (35 improved, 0 regressed), 233 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: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
         -36 (-24.32% of base) : System.Security.Cryptography.Algorithms.dasm - Rijndael:.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) : 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
         -18 (-40.00% of base) : System.Security.Cryptography.Algorithms.dasm - Helpers:CloneByteArray(ref):ref

[GrabYourPitchforks]

Why would the intrinsic annotation be on MemberwiseClone? I realize Array.Copy is just a dumb wrapper around it, so I guess inlining has already taken place at this point?