Give a proper VN to GT_BOX

[mikedn]

GT_BOX doesn't do anything so it should just get the VN of its sole operand. This allows assertion prop to see that the result of BOX is not null (since the BOX operand is the result of an object allocation which is never null).

[mikedn]

jit-diff summary:

Total bytes of diff: -36733 (-0.16% of base)
    diff is an improvement.
Total byte diff includes 0 bytes from reconciling methods
        Base had    0 unique methods,        0 unique bytes
        Diff had    0 unique methods,        0 unique bytes
Top file improvements by size (bytes):
      -36662 : System.Private.CoreLib.dasm (-1.08% of base)
         -37 : System.Private.Xml.dasm (0.00% of base)
         -15 : System.Private.DataContractSerialization.dasm (0.00% of base)
         -14 : System.Data.Common.dasm (0.00% of base)
          -5 : NuGet.Versioning.dasm (-0.02% of base)
5 total files with size differences (5 improved, 0 regressed), 125 unchanged.
Top method regessions by size (bytes):
           5 : System.Data.Common.dasm - SqlDoubleStorage:ConvertXmlToObject(ref):ref:this
           2 : System.Data.Common.dasm - SqlCharsStorage:ConvertXmlToObject(ref):ref:this
           2 : System.Data.Common.dasm - SqlDecimalStorage:ConvertXmlToObject(ref):ref:this
           2 : System.Data.Common.dasm - SqlStringStorage:ConvertXmlToObject(ref):ref:this
Top method improvements by size (bytes):
      -11582 : System.Private.CoreLib.dasm - ValueTuple`8:ToString():ref:this (15 methods)
      -11389 : System.Private.CoreLib.dasm - ValueTuple`8:System.IValueTupleInternal.ToStringEnd():ref:this (15 methods)
       -7630 : System.Private.CoreLib.dasm - ValueTuple`8:GetHashCode():int:this (15 methods)
       -3737 : System.Private.CoreLib.dasm - ValueTuple`8:GetHashCodeCore(ref):int:this (15 methods)
       -1884 : System.Private.CoreLib.dasm - ValueTuple`8:System.Runtime.CompilerServices.ITuple.get_Item(int):ref:this (15 methods)
43 total methods with size differences (39 improved, 4 regressed), 129976 unchanged.

[mikedn]

A complete (mostly, unfortunately different generic instantiations are reported as one method) list of affected methods is interesting to see where a certain pattern occurs - boxing followed by a null/type check:

-11582 : System.Private.CoreLib.dasm - ValueTuple`8:ToString():ref:this (15 methods)
-11389 : System.Private.CoreLib.dasm - ValueTuple`8:System.IValueTupleInternal.ToStringEnd():ref:this (15 methods)
 -7630 : System.Private.CoreLib.dasm - ValueTuple`8:GetHashCode():int:this (15 methods)
 -3737 : System.Private.CoreLib.dasm - ValueTuple`8:GetHashCodeCore(ref):int:this (15 methods)
 -1884 : System.Private.CoreLib.dasm - ValueTuple`8:System.Runtime.CompilerServices.ITuple.get_Item(int):ref:this (15 methods)
  -251 : System.Private.CoreLib.dasm - ValueTuple`8:System.Runtime.CompilerServices.ITuple.get_Length():int:this (15 methods)
   -28 : System.Private.CoreLib.dasm - List`1:IsCompatibleObject(ref):bool (26 methods)
   -23 : System.Private.CoreLib.dasm - SignatureHelper:ToString():ref:this
   -21 : System.Private.CoreLib.dasm - ReadOnlyCollection`1:IsCompatibleObject(ref):bool (26 methods)
   -15 : System.Private.CoreLib.dasm - Tuple`2:System.ITupleInternal.ToString(ref):ref:this (3 methods)
   -15 : System.Private.CoreLib.dasm - EncodingTable:GetEncodings():ref
   -15 : System.Private.DataContractSerialization.dasm - CriticalHelper:WriteMembers(ref,ref,ref):int:this (2 methods)
   -14 : System.Private.CoreLib.dasm - ObjectEqualityComparer`1:LastIndexOf(ref,struct,int,int):int:this (10 methods)
   -14 : System.Private.CoreLib.dasm - ObjectEqualityComparer`1:IndexOf(ref,struct,int,int):int:this (10 methods)
   -10 : System.Private.CoreLib.dasm - ObjectEqualityComparer`1:IndexOf(ref,long,int,int):int:this
   -10 : System.Private.CoreLib.dasm - ObjectEqualityComparer`1:IndexOf(ref,int,int,int):int:this (3 methods)
    -9 : System.Private.CoreLib.dasm - RuntimeType:CacheEquals(ref):bool:this
    -9 : System.Private.CoreLib.dasm - CustomAttribute:GetCustomAttributes(ref,int,int,ref,bool,ref):ref
    -9 : System.Private.CoreLib.dasm - ObjectEqualityComparer`1:Equals(int,int):bool:this (3 methods)
    -9 : System.Private.Xml.dasm - XmlLoader:LoadDocumentType(ref,ref):this
    -7 : System.Private.Xml.dasm - QilGenerator:GetNsVar(ref):ref:this
    -7 : System.Private.Xml.dasm - XmlILOptimizerVisitor:IsConstructedExpression(ref):bool:this
    -7 : System.Private.Xml.dasm - XmlILOptimizerVisitor:AreLiteralArgs(ref):bool:this
    -7 : System.Private.Xml.dasm - XmlILOptimizerVisitor:HasNestedSequence(ref):bool:this
    -5 : NuGet.Versioning.dasm - FloatRange:GetHashCode():int:this
    -5 : System.Private.CoreLib.dasm - ObjectEqualityComparer`1:LastIndexOf(ref,long,int,int):int:this
    -5 : System.Private.CoreLib.dasm - ObjectEqualityComparer`1:LastIndexOf(ref,int,int,int):int:this (3 methods)
    -3 : System.Data.Common.dasm - SqlDateTimeStorage:ConvertXmlToObject(ref):ref:this
    -3 : System.Data.Common.dasm - SqlInt64Storage:ConvertXmlToObject(ref):ref:this
    -3 : System.Data.Common.dasm - SqlMoneyStorage:ConvertXmlToObject(ref):ref:this
    -2 : System.Data.Common.dasm - SqlGuidStorage:ConvertXmlToObject(ref):ref:this
    -2 : System.Data.Common.dasm - SqlInt16Storage:ConvertXmlToObject(ref):ref:this
    -2 : System.Data.Common.dasm - SqlInt32Storage:ConvertXmlToObject(ref):ref:this
    -2 : System.Data.Common.dasm - SqlSingleStorage:ConvertXmlToObject(ref):ref:this
    -2 : System.Data.Common.dasm - SqlBinaryStorage:ConvertXmlToObject(ref):ref:this
    -2 : System.Data.Common.dasm - SqlBooleanStorage:ConvertXmlToObject(ref):ref:this
    -2 : System.Data.Common.dasm - SqlBytesStorage:ConvertXmlToObject(ref):ref:this
    -2 : System.Data.Common.dasm - SqlByteStorage:ConvertXmlToObject(ref):ref:this
    -2 : System.Private.CoreLib.dasm - ResourceManager:ReleaseAllResources():this

• I've yet to figure out what's going on with ValueTuple'8, how come there are 15 instantiations in corelib?!
• IsCompatibleObject causes a bit of bloat. Might be worth a look to see what can be done better.
• ObjectEqualityComparer's Equals/IndexOf/LastIndexOf - not sure why those are instantiated.
• Something in corelib appears to be using Tuple<,>. Maybe it's worth having a look an replacing with ValueTuple<,>.

[mikedn]

I've yet to figure out what's going on with ValueTuple'8, how come there are 15 instantiations in corelib?!

It looks like TupleExtensions.ToTuple forces the instantiation of System.ValueTuple'8[__Canon,__Canon,__Canon,__Canon,__Canon,__Canon,__Canon,ValueTuple'1] & co. For such instantiations the JIT now throws away a lot of dead code from methods like ValueTuple'8:ToString:

coreclr/src/mscorlib/shared/System/ValueTuple.cs

Lines 2259 to 2268 in 64ea348

	IValueTupleInternal rest = Rest as IValueTupleInternal;
	if (rest == null)
	{
	return "(" + Item1?.ToString() + ", " + Item2?.ToString() + ", " + Item3?.ToString() + ", " + Item4?.ToString() + ", " + Item5?.ToString() + ", " + Item6?.ToString() + ", " + Item7?.ToString() + ", " + Rest.ToString() + ")";
	}
	else
	{
	return "(" + Item1?.ToString() + ", " + Item2?.ToString() + ", " + Item3?.ToString() + ", " + Item4?.ToString() + ", " + Item5?.ToString() + ", " + Item6?.ToString() + ", " + Item7?.ToString() + ", " + rest.ToStringEnd();
	}
	}

Hrm, this ValueTuple stuff is pretty horrible in terms of code size.

[AndyAyersMS]

Thanks for spotting this. Looks good.

[BruceForstall]

@AndyAyersMS @mikedn Looks like this caused an assert in armlb testing: https://ci.dot.net/job/dotnet_coreclr/job/master/job/armlb_cross_checked_windows_nt_flow/365/

TEST_CMD_LINE: JIT\Performance\CodeQuality\SIMD\SeekUnroll\SeekUnroll> C:\Users\robox\j\workspace\armlb_cross_c---052e33bb\bin\tests\Windows_NT.armlb.Checked\JIT\Performance\CodeQuality\SIMD\SeekUnroll\SeekUnroll\SeekUnroll.cmd 

BEGIN EXECUTION
 "C:\Users\robox\j\workspace\armlb_cross_c---052e33bb\bin\tests\Windows_NT.armlb.Checked\Tests\Core_Root\corerun.exe" SeekUnroll.exe 
Warning: no iteration count specified; defaulting to 1 iteration per case
To use multiple iterations per case, pass the desired number of iterations as the first command-line argument to this test

Assert failure(PID 15844 [0x00003de4], Thread: 10584 [0x2958]): Assertion failed 'varDsc->lvOnFrame' in 'Xunit.Sdk.AssertEqualityComparer`1[Int32][System.Int32]:Equals(int,int):bool:this' (IL size 358)

    File: d:\j\workspace\armlb_cross_c---17d9d8e9\src\jit\lclvars.cpp Line: 6425
    Image: C:\Users\robox\j\workspace\armlb_cross_c---052e33bb\bin\tests\Windows_NT.armlb.Checked\Tests\Core_Root\CoreRun.exe

Expected: 100
Actual: 123456789
END EXECUTION - FAILED
FAILED

[mikedn]

Hmm, armlb is the legacy backend, right?

[BruceForstall]

Correct.

[AndyAyersMS]

Saw something similar over on desktop with the x86 frankenjit:

Assert failure(PID 4264 [0x000010a8], Thread: 114056 [0x1bd88]): Assertion failed 'varDsc->lvOnFrame'
in 'System.Collections.Generic.Dictionary`2[__Canon,Nullable`1] System.__Canon,System.Nullable`1[System.Boolean]]:get_Item(ref):struct:this'

[mikedn]

Thanks, I've reproduced locally with legacynonjit.dll and the following code:

Dictionary<string, bool?> d = new Dictionary<string, bool?>();
d.Add("foo", true);
return d["foo"];

I'll investigate more this evening (your morning-afternoon). Heading out to work...