JIT: More type equality opts #14317

AndyAyersMS · 2017-10-04T07:09:21Z

Handle some type equality cases we couldn't get previously, involving runtime lookups.

Not ready for merging just yet, but want to get some test exposure.

AndyAyersMS · 2017-10-04T07:14:23Z

@dotnet-bot test Windows_NT x64 corefx_baseline

Jit-diffs here are interesting (using checked to show off which generics got impacted, diffs are similar in release):

Summary:
(Note: Lower is better)

Total bytes of diff: -6546 (-0.18 % 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):
       -6546 : System.Private.CoreLib.dasm (-0.18 % of base)

1 total files with size differences (1 improved, 0 regressed), 0 unchanged.

Top method improvements by size (bytes):
       -3160 : System.Private.CoreLib.dasm - System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1[__Canon][System.__Canon]:AwaitUnsafeOnCompleted(byref,byref):this (3 methods)
       -1595 : System.Private.CoreLib.dasm - System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1[VoidTaskResult][System.Threading.Tasks.VoidTaskResult]:AwaitUnsafeOnCompleted(byref,byref):this (9 methods)
       -1585 : System.Private.CoreLib.dasm - System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1[Boolean][System.Boolean]:AwaitUnsafeOnCompleted(byref,byref):this (2 methods)
        -103 : System.Private.CoreLib.dasm - System.Runtime.InteropServices.WindowsRuntime.EnumeratorToIteratorAdapter`1[KeyValuePair`2][System.Collections.Generic.KeyValuePair`2[System.__Canon,System.__Canon]]:GetMany(ref):int:this
        -103 : System.Private.CoreLib.dasm - System.Runtime.InteropServices.WindowsRuntime.EnumeratorToIteratorAdapter`1[KeyValuePair`2][System.Collections.Generic.KeyValuePair`2[System.__Canon,System.Int32]]:GetMany(ref):int:this

5 total methods with size differences (5 improved, 0 regressed), 25217 unchanged.

The huge decreases in the first method set comes about because the new interface method knows that __Canon cannot represent a value type, so the shared instances of these methods can now rule out many cases.

omariom · 2017-10-04T14:08:52Z

👍
What cases are handled now?

AndyAyersMS · 2017-10-04T18:34:17Z

@omariom does this help?

using System;

struct Wrap1<T> {}

struct Wrap2<T> {}

class C
{
    static bool IsInt<T>()
    {
        return (typeof(T) == typeof(int));
    }

    static bool IsInt<T>(T t)
    {
        return (t.GetType() == typeof(int));
    }

    static bool IsString<T>()
    {
        return (typeof(T) == typeof(string));
    }

    static bool IsString<T>(T t)
    {
        return (t.GetType() == typeof(string));
    }

    static bool IsIntArray<T>()
    {
        return (typeof(T) == typeof(int[]));
    }

    static bool IsStringArray<T>()
    {
        return (typeof(T) == typeof(string[]));
    }

    static bool IsWrap1<T,U>()
    {
        return (typeof(U) == typeof(Wrap1<T>));
    }

    static bool IsWrap1<T,U>(U u)
    {
        return (u.GetType() == typeof(Wrap1<T>));
    }

    public static int Main()
    {
        // Fully optimized
        bool c1 = IsInt<int>();
        bool c2 = IsInt<string>();
        bool c3 = IsString<int>();

        // Partially optimized
        bool c4 = IsString<string>();

        // Not yet optimized (#14304)
        bool d1 = IsInt<int>(3);
        bool d2 = IsInt<string>("three");
        bool d3 = IsString<int>(3);
        bool d4 = IsString<string>("three");

        // Partially optimized
        // Note all these calls share one method body
        bool e1 = IsIntArray<int[]>();
        bool e2 = IsIntArray<string[]>();
        bool e3 = IsStringArray<int[]>();
        bool e4 = IsStringArray<string[]>();

        // Fully optimized
        bool f1 = IsWrap1<int, Wrap1<int>>();
        bool f2 = IsWrap1<int, Wrap2<int>>();

        // Not yet optimized (#14304)
        bool f3 = IsWrap1<int, Wrap2<int>>(new Wrap2<int>());
        bool f4 = IsWrap1<int, Wrap1<int>>(new Wrap1<int>());

        bool pos = c1 & c4 & d1 & d4 & e1 & e4 & f1 & f4;
        bool neg = c2 & c3 & d2 & d3 & e2 & e3 & f2 & f3;

        return pos & !neg ? 100 : 0;
    }
}

AndyAyersMS · 2017-10-04T18:34:54Z

OSX failure may be relevant, but looks odd, so will retry

@dotnet-bot retest OSX10.12 x64 Checked Build and Test

AndyAyersMS · 2017-10-04T18:35:35Z

Hmm, can't trigger retest with edit. retrying my retry

@dotnet-bot retest OSX10.12 x64 Checked Build and Test

AndyAyersMS · 2017-10-04T20:23:13Z

This is getting closer to being ready. Still need t work on SPMI and related changes and the desktop side updates.

@jkotas could you look over the logic in the new jit interface method compareTypesForEquality? Note I'm also introducing a method for optimizing casts but haven't yet implemented it or called it from the jit; that will come in future PRs.

jkotas · 2017-10-04T20:27:05Z

could you look over the logic in the new jit interface method compareTypesForEquality?

LGTM

jkotas · 2017-10-04T20:38:46Z

src/vm/jitinterface.cpp

+        // be equal unless the type defs are the same
+        if (hnd1.IsValueType() || hnd2.IsValueType())
+        {
+            if (!hnd1.AsMethodTable()->HasSameTypeDefAs(hnd2.AsMethodTable()))


There are "native value types" for which IsValueType() returns true, but they are not backed by MethodTable. You may want to check for !IsTypeDesc() before calling AsMethodTable() here.

How about:

if ((hnd1.IsValueType() && !hnd1.IsNativeValueType()) || (hnd2.IsValueType() && !hnd2.IsNativeValueType()))

This can be still true if one of them is native value type -> AsMethodTable won't work. It may be best to handle this by calling GetMethodTable instead of AsMethodTable. Something like:

if (hnd1.IsValueType() || hnd2.IsValueType()) { if (!hnd1.GetMethodTable()->HasSameTypeDefAs(hnd2.GetMethodTable()))

So there are classes that aren't value types but are native value types? Interesting. Is there some simple way to create these kinds of classes?

Any extra complications on desktop? I need to implement at least part of this there...

The situation that I was worried about does not actually need to involve native value types. For example: hnd1 is value type and hnd2 is array. if (hnd1.IsValueType() || hnd2.IsValueType()) will be true, and we would end up calling AsMethodTable on array TypeDesc.

So there are classes that aren't value types but are native value types?

All native value types are value types too.

jkotas · 2017-10-04T20:40:38Z

src/vm/jitinterface.cpp

+            TypeHandle canonHnd = TypeHandle(g_pCanonMethodTableClass);
+            if ((hnd1 != canonHnd) && (hnd2 != canonHnd))
+            {
+                // Arrays... ?


This should work fine for arrays, I think.

Thanks, removed the comment.

AndyAyersMS · 2017-10-04T22:09:38Z

@briansull PTAL
cc @dotnet/jit-contrib

AndyAyersMS · 2017-10-04T22:10:26Z

Note this changes the jit interface so will need desktop changes too. I will get working on those.

briansull

Looks Good
(with suggestions for better enum names)

briansull · 2017-10-04T23:47:03Z

src/inc/corinfo.h

+{
+    MustNot = -1, // types are not equal
+    May = 0,      // types may be equal (must test at runtime)
+    Must = 1,     // type are equal


Another possibility here is:

Incongruent,
Unknown,
Congruent,

In particular the usage of May is confusing as it represents the lack of knowledge.

I forgot to mention this when we were chatting offline: these names match the ones used a in similar .NetNative extension of the jit interface. So am going to keep them as is...

AndyAyersMS · 2017-10-05T01:57:11Z

Arm legs look like they likely going to time out. Will retry them later.

Work based on the plan outlined in #14305. Introduce a new unary node type GT_RUNTIMELOOKUP that wraps existing runtime lookup trees created in `impLookupToTree` and holds onto the handle that inspired the lookup. Note there are other importer paths that create lookups directly that we might also want to wrap, though wrapping is not a requirement for correctness. Keep this node type around through morph, then unwrap and just use the wrapped node after that.

The jit is now able to optimize some type equality checks in shared method instances where one or both of the types require runtime lookup, for instance comparing `T` to a value type, or comparing two different value types `V1<T>` and `V2<T>`. Add two new jit interface methods, one for testing for type equality and the other for type casting. These return Must/MustNot/Maybe results depending on whether or not the equality or cast can be resolved at jit time. Implement the equality check. Use this to enhance the type equality opts in the jit for both direct comparison and the checking done by unbox.any.

AndyAyersMS · 2017-10-05T05:19:47Z

Latest update includes suggested fix for the type desc case. Also rebased and squashed down to two commits.

AndyAyersMS · 2017-10-05T14:08:49Z

Am going to retry the arm tests since the HW should be more or less idle now.

@dotnet-bot retest Windows_NT arm Cross Checked Build and Test
@dotnet-bot retest Windows_NT armlb Cross Checked Build and Test

AndyAyersMS · 2017-10-05T18:31:29Z

Am going to give up trying to get arm runs through; there's nothing arch-specific about this change.

AndyAyersMS added the * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) label Oct 4, 2017

dnfclas added the cla-already-signed label Oct 4, 2017

jkotas reviewed Oct 4, 2017

View reviewed changes

AndyAyersMS removed the * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) label Oct 4, 2017

AndyAyersMS changed the title ~~WIP More type equality opts~~ JIT: More type equality opts Oct 4, 2017

briansull approved these changes Oct 4, 2017

View reviewed changes

AndyAyersMS added 2 commits October 4, 2017 22:09

AndyAyersMS force-pushed the MoreTypeEqualityOpts branch from 508b1bd to 9fc2875 Compare October 5, 2017 05:18

AndyAyersMS merged commit b085bfa into dotnet:master Oct 5, 2017

AndyAyersMS deleted the MoreTypeEqualityOpts branch October 5, 2017 18:32

AndyAyersMS mentioned this pull request Jul 24, 2022

JIT: teach VN to fold type comparisons dotnet/runtime#72136

Merged

JIT: More type equality opts #14317

JIT: More type equality opts #14317

Uh oh!

Conversation

AndyAyersMS commented Oct 4, 2017

Uh oh!

AndyAyersMS commented Oct 4, 2017

Uh oh!

omariom commented Oct 4, 2017

Uh oh!

AndyAyersMS commented Oct 4, 2017

Uh oh!

AndyAyersMS commented Oct 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AndyAyersMS commented Oct 4, 2017

Uh oh!

AndyAyersMS commented Oct 4, 2017

Uh oh!

jkotas commented Oct 4, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jkotas Oct 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AndyAyersMS commented Oct 4, 2017

Uh oh!

AndyAyersMS commented Oct 4, 2017

Uh oh!

briansull left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AndyAyersMS commented Oct 5, 2017

Uh oh!

AndyAyersMS commented Oct 5, 2017

Uh oh!

AndyAyersMS commented Oct 5, 2017

Uh oh!

AndyAyersMS commented Oct 5, 2017

Uh oh!

Uh oh!

AndyAyersMS commented Oct 4, 2017 •

edited

Loading

jkotas Oct 5, 2017 •

edited

Loading