From 27fe171d9d3a2f004987ad88cf926018cab1113c Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Tue, 12 Jul 2022 11:23:14 -0700 Subject: [PATCH 01/13] JIT: teach VN to fold type comparisons Teach VN to fold some comparisons involving calls to `TypeHandleToRuntimeType`. Closes #71909. --- src/coreclr/jit/valuenum.cpp | 60 ++++++++++++++++++++++++++++++++++++ src/coreclr/jit/valuenum.h | 3 ++ 2 files changed, 63 insertions(+) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index ea63129080e904..de3f9c6c557a2a 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -2135,6 +2135,15 @@ ValueNum ValueNumStore::VNForFunc(var_types typ, VNFunc func, ValueNum arg0VN, V ValueNum resultVN = NoVN; + // When both operands are runtime types we can sometimes also fold. + // This is the VN analog of gtFoldTypeCompare + // + resultVN = VNEvalFoldTypeCompare(typ, func, arg0VN, arg1VN); + if (resultVN != NoVN) + { + return resultVN; + } + // We canonicalize commutative operations. // (Perhaps should eventually handle associative/commutative [AC] ops -- but that gets complicated...) if (VNFuncIsCommutative(func)) @@ -3651,6 +3660,57 @@ ValueNum ValueNumStore::EvalBitCastForConstantArgs(var_types dstType, ValueNum a } } +//------------------------------------------------------------------------ +// VNEvalFoldTypeCompare: +// +// Arguments: +// type - The result type +// func - The function +// arg0VN - VN of the first argument +// arg1VN - VN of the second argument +// +// Return Value: +// NoVN if this is not a foldable type compare +// Simplified (perhaps constant) VN if it is foldable. +// +// Notes: +// Value number counterpart to gtFoldTypeCompare +// Doesn't handle all the cases (yet). +// +// (EQ/NE (TypeHandleToRuntimeType x) (TypeHandleToRuntimeType y)) == (EQ/NE x y) +// +ValueNum ValueNumStore::VNEvalFoldTypeCompare(var_types type, VNFunc func, ValueNum arg0VN, ValueNum arg1VN) +{ + if (func >= VNF_Boundary) + { + return NoVN; + } + + const genTreeOps oper = genTreeOps(func); + if (!GenTree::StaticOperIs(oper, GT_EQ, GT_NE)) + { + return NoVN; + } + + VNFuncApp arg0Func; + const bool arg0IsFunc = GetVNFunc(arg0VN, &arg0Func); + + if (!arg0IsFunc || (arg0Func.m_func != VNF_TypeHandleToRuntimeType)) + { + return NoVN; + } + + VNFuncApp arg1Func; + const bool arg1IsFunc = GetVNFunc(arg1VN, &arg1Func); + + if (!arg1IsFunc || (arg1Func.m_func != VNF_TypeHandleToRuntimeType)) + { + return NoVN; + } + + return VNForFunc(type, func, arg0Func.m_args[0], arg1Func.m_args[0]); +} + //------------------------------------------------------------------------ // VNEvalCanFoldBinaryFunc: Can the given binary function be constant-folded? // diff --git a/src/coreclr/jit/valuenum.h b/src/coreclr/jit/valuenum.h index 650ffd62508bbe..8fba842c78d96c 100644 --- a/src/coreclr/jit/valuenum.h +++ b/src/coreclr/jit/valuenum.h @@ -314,6 +314,9 @@ class ValueNumStore // Returns "true" iff "vnf" should be folded by evaluating the func with constant arguments. bool VNEvalShouldFold(var_types typ, VNFunc func, ValueNum arg0VN, ValueNum arg1VN); + // Value number a type comparison + ValueNum VNEvalFoldTypeCompare(var_types type, VNFunc func, ValueNum arg0VN, ValueNum arg1VN); + // return vnf(v0) template static T EvalOp(VNFunc vnf, T v0); From e4f8bcb7d26974ac749abea74e635101c75544c5 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Thu, 14 Jul 2022 19:55:30 -0700 Subject: [PATCH 02/13] add test case --- .../JIT/opt/ValueNumbering/TypeTestFolding.cs | 170 ++++++++++++++++++ .../opt/ValueNumbering/TypeTestFolding.csproj | 13 ++ 2 files changed, 183 insertions(+) create mode 100644 src/tests/JIT/opt/ValueNumbering/TypeTestFolding.cs create mode 100644 src/tests/JIT/opt/ValueNumbering/TypeTestFolding.csproj diff --git a/src/tests/JIT/opt/ValueNumbering/TypeTestFolding.cs b/src/tests/JIT/opt/ValueNumbering/TypeTestFolding.cs new file mode 100644 index 00000000000000..4c46605c264ef9 --- /dev/null +++ b/src/tests/JIT/opt/ValueNumbering/TypeTestFolding.cs @@ -0,0 +1,170 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; +using System.Runtime.CompilerServices; + +public enum Enum1 : int { A } +public enum Enum2 : uint { A } + +class TypeTestFolding +{ + [MethodImpl(MethodImplOptions.NoInlining)] + static void SideEffect() { } + + static bool True0() => typeof(delegate*) == typeof(delegate* unmanaged); + static bool True1() + { + var t0 = typeof(delegate*); + SideEffect(); + var t1 = typeof(delegate* unmanaged); + return t0 == t1; + } + + static bool True2() => typeof(TypeTestFolding) == typeof(TypeTestFolding); + static bool True3() + { + var t0 = typeof(TypeTestFolding); + SideEffect(); + var t1 = typeof(TypeTestFolding); + return t0 == t1; + } + + static bool True4() => typeof(ValueTuple) == typeof(ValueTuple); + static bool True5() + { + var t0 = typeof(ValueTuple); + SideEffect(); + var t1 = typeof(ValueTuple); + return t0 == t1; + } + + static bool True6() => typeof(delegate*) == typeof(nint); + static bool True7() + { + var t0 = typeof(delegate*); + SideEffect(); + var t1 = typeof(nint); + return t0 == t1; + } + + static bool False0() => typeof(List) == typeof(List); + static bool False1() + { + var t0 = typeof(List); + SideEffect(); + var t1 = typeof(List); + return t0 == t1; + } + + static bool False2() => typeof(int) == typeof(Enum1); + static bool False3() + { + var t0 = typeof(int); + SideEffect(); + var t1 = typeof(Enum1); + return t0 == t1; + } + + static bool False4() => typeof(Enum1) == typeof(Enum2); + static bool False5() + { + var t0 = typeof(Enum1); + SideEffect(); + var t1 = typeof(Enum2); + return t0 == t1; + } + + static bool False6() => typeof(int?) == typeof(uint?); + static bool False7() + { + var t0 = typeof(int?); + SideEffect(); + var t1 = typeof(uint?); + return t0 == t1; + } + + static bool False8() => typeof(int?) == typeof(Enum1?); + static bool False9() + { + var t0 = typeof(int?); + SideEffect(); + var t1 = typeof(Enum1?); + return t0 == t1; + } + + static bool False10() => typeof(ValueTuple) == typeof(ValueTuple); + static bool False11() + { + var t0 = typeof(ValueTuple); + SideEffect(); + var t1 = typeof(ValueTuple); + return t0 == t1; + } + + static bool False12() => typeof(delegate*[]) == typeof(delegate*[]); + static bool False13() + { + var t0 = typeof(delegate*[]); + SideEffect(); + var t1 = typeof(delegate*[]); + return t0 == t1; + } + + static bool False14() => typeof(int[]) == typeof(uint[]); + static bool False15() + { + var t0 = typeof(int[]); + SideEffect(); + var t1 = typeof(uint[]); + return t0 == t1; + } + + static bool False16() => typeof(delegate*) == typeof(UIntPtr); + static bool False17() + { + var t0 = typeof(delegate*); + SideEffect(); + var t1 = typeof(UIntPtr); + return t0 == t1; + } + + unsafe static int Main() + { + delegate*[] trueFuncs = new delegate*[] { &True0, &True1, &True2, &True3, &True4, &True5, &True6, &True7 }; + delegate*[] falseFuncs = new delegate*[] { &False0, &False1, &False2, &False3, &False4, &False5, + &False6, &False7, &False8, &False9, &False10, &False11, + &False12, &False13, &False14, &False15, &False16, &False17 }; + + int result = 100; + int trueCount = 0; + int falseCount = 0; + + foreach(var tf in trueFuncs) + { + if (!tf()) + { + Console.WriteLine($"True{trueCount} failed"); + result++; + } + trueCount++; + } + + foreach(var ff in falseFuncs) + { + if (ff()) + { + Console.WriteLine($"False{falseCount} failed"); + result++; + } + falseCount++; + } + + Console.WriteLine($"Ran {trueCount + falseCount} tests; result {result}"); + return result; + } +} + + + diff --git a/src/tests/JIT/opt/ValueNumbering/TypeTestFolding.csproj b/src/tests/JIT/opt/ValueNumbering/TypeTestFolding.csproj new file mode 100644 index 00000000000000..bf6f589eb325bb --- /dev/null +++ b/src/tests/JIT/opt/ValueNumbering/TypeTestFolding.csproj @@ -0,0 +1,13 @@ + + + Exe + + + None + True + True + + + + + From 31d16fa3791aed5b5dd64f57e07c835fa3c775f3 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Fri, 15 Jul 2022 10:07:25 -0700 Subject: [PATCH 03/13] handle function pointer case --- src/coreclr/jit/gentree.cpp | 22 +++++++++++++++------- src/coreclr/jit/valuenum.cpp | 22 ++++++++++++++++++++++ src/coreclr/vm/jitinterface.cpp | 29 ++++++++++++++++++++++++++--- 3 files changed, 63 insertions(+), 10 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index b15378b8e9b979..79e832c671348e 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -12910,18 +12910,26 @@ GenTree* Compiler::gtFoldTypeCompare(GenTree* tree) // // Find out how we can compare the two handles. // NOTE: We're potentially passing NO_CLASS_HANDLE, but the runtime knows what to do with it here. - CorInfoInlineTypeCheck inliningKind = + // + CorInfoInlineTypeCheck inliningKind1 = info.compCompHnd->canInlineTypeCheck(cls1Hnd, CORINFO_INLINE_TYPECHECK_SOURCE_TOKEN); + CorInfoInlineTypeCheck inliningKind2 = + info.compCompHnd->canInlineTypeCheck(cls2Hnd, CORINFO_INLINE_TYPECHECK_SOURCE_TOKEN); - // If the first type needs helper, check the other type: it might be okay with a simple compare. - if (inliningKind == CORINFO_INLINE_TYPECHECK_USE_HELPER) + // If either type needs a comparison helper, just bail on the optimization. + // The helper no longer exists. + // + if ((inliningKind1 == CORINFO_INLINE_TYPECHECK_USE_HELPER) || + (inliningKind2 == CORINFO_INLINE_TYPECHECK_USE_HELPER)) { - inliningKind = info.compCompHnd->canInlineTypeCheck(cls2Hnd, CORINFO_INLINE_TYPECHECK_SOURCE_TOKEN); + JITDUMP("Comparison requires helper, not optimizing\n"); + return tree; } - assert(inliningKind == CORINFO_INLINE_TYPECHECK_PASS || inliningKind == CORINFO_INLINE_TYPECHECK_USE_HELPER); - - GenTree* compare = gtCreateHandleCompare(oper, op1ClassFromHandle, op2ClassFromHandle, inliningKind); + assert(inliningKind1 == CORINFO_INLINE_TYPECHECK_PASS); + assert(inliningKind2 == CORINFO_INLINE_TYPECHECK_PASS); + GenTree* compare = + gtCreateHandleCompare(oper, op1ClassFromHandle, op2ClassFromHandle, CORINFO_INLINE_TYPECHECK_PASS); // Drop any now-irrelvant flags compare->gtFlags |= tree->gtFlags & (GTF_RELOP_JMP_USED | GTF_DONT_CSE); diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index de3f9c6c557a2a..2cba29f59cef4b 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -3708,6 +3708,28 @@ ValueNum ValueNumStore::VNEvalFoldTypeCompare(var_types type, VNFunc func, Value return NoVN; } + // Only re-express as handle equality when we have known + // class handles and the VM agrees comparing these gives the same + // result as comparing the runtime types. + // + ValueNum handle0 = arg0Func.m_args[0]; + ValueNum handle1 = arg1Func.m_args[0]; + if (!IsVNHandle(handle0) || !IsVNHandle(handle1)) + { + return NoVN; + } + assert(GetHandleFlags(handle0) == GTF_ICON_CLASS_HDL); + assert(GetHandleFlags(handle1) == GTF_ICON_CLASS_HDL); + CORINFO_CLASS_HANDLE cls0Hnd = CORINFO_CLASS_HANDLE(ConstantValue(handle0)); + CORINFO_CLASS_HANDLE cls1Hnd = CORINFO_CLASS_HANDLE(ConstantValue(handle1)); + + TypeCompareState s = m_pComp->info.compCompHnd->compareTypesForEquality(cls0Hnd, cls1Hnd); + + if (s == TypeCompareState::May) + { + return NoVN; + } + return VNForFunc(type, func, arg0Func.m_args[0], arg1Func.m_args[0]); } diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index a6bd0d8ea26c33..87f207b0f912f1 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -3543,8 +3543,24 @@ bool CEEInfo::isValueClass(CORINFO_CLASS_HANDLE clsHnd) // This will enable to use directly the typehandle instead of going through getClassByHandle CorInfoInlineTypeCheck CEEInfo::canInlineTypeCheck(CORINFO_CLASS_HANDLE clsHnd, CorInfoInlineTypeCheckSource source) { - LIMITED_METHOD_CONTRACT; - return CORINFO_INLINE_TYPECHECK_PASS; + CONTRACTL { + NOTHROW; + GC_NOTRIGGER; + MODE_PREEMPTIVE; + } CONTRACTL_END; + + CorInfoInlineTypeCheck result = CORINFO_INLINE_TYPECHECK_PASS; + + JIT_TO_EE_TRANSITION_LEAF(); + + if (TypeHandle(clsHnd).IsFnPtrType()) + { + result = CORINFO_INLINE_TYPECHECK_USE_HELPER; + } + + EE_TO_JIT_TRANSITION_LEAF(); + + return result; } /*********************************************************************/ @@ -4221,7 +4237,14 @@ TypeCompareState CEEInfo::compareTypesForEquality( // If neither type is a canonical subtype, type handle comparison suffices if (!hnd1.IsCanonicalSubtype() && !hnd2.IsCanonicalSubtype()) { - result = (hnd1 == hnd2 ? TypeCompareState::Must : TypeCompareState::MustNot); + if (hnd1.IsFnPtrType() || hnd2.IsFnPtrType()) + { + result = TypeCompareState::May; + } + else + { + result = (hnd1 == hnd2 ? TypeCompareState::Must : TypeCompareState::MustNot); + } } // If either or both types are canonical subtypes, we can sometimes prove inequality. else From 768915ec3ba40330fe8796004fd3fbfff1cf8322 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Fri, 15 Jul 2022 11:38:19 -0700 Subject: [PATCH 04/13] crossgen2 support --- src/coreclr/jit/valuenum.cpp | 5 ++++- .../tools/Common/JitInterface/CorInfoImpl.cs | 21 ++++++++++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 2cba29f59cef4b..d7686dd1d2a0c0 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -3730,7 +3730,10 @@ ValueNum ValueNumStore::VNEvalFoldTypeCompare(var_types type, VNFunc func, Value return NoVN; } - return VNForFunc(type, func, arg0Func.m_args[0], arg1Func.m_args[0]); + // At this point we know enough to determine if the resulting compare will + // be true or false, but we'll let normal const folding take over. + // + return VNForFunc(type, func, handle0, handle1); } //------------------------------------------------------------------------ diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index cdcb74e5a6b0e6..6e7ce6fc6a0f1b 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -1923,6 +1923,17 @@ private CorInfoInlineTypeCheck canInlineTypeCheck(CORINFO_CLASS_STRUCT_* cls, Co { // TODO: when we support multiple modules at runtime, this will need to do more work // NOTE: cls can be null + + // Block the jit from optimizing type tests involving function pointers. + if (cls != null) + { + TypeDesc type = HandleToObject(cls); + if (type.IsFunctionPointer) + { + return CorInfoInlineTypeCheck.CORINFO_INLINE_TYPECHECK_USE_HELPER; + } + } + return CorInfoInlineTypeCheck.CORINFO_INLINE_TYPECHECK_PASS; } @@ -2590,7 +2601,15 @@ private TypeCompareState compareTypesForEquality(CORINFO_CLASS_STRUCT_* cls1, CO // If neither type is a canonical subtype, type handle comparison suffices if (!type1.IsCanonicalSubtype(CanonicalFormKind.Any) && !type2.IsCanonicalSubtype(CanonicalFormKind.Any)) { - result = (type1 == type2 ? TypeCompareState.Must : TypeCompareState.MustNot); + // FunctionPointer types are special. Avoid letting the jit optimize them for now. + if (type1.IsFunctionPointer || type2.IsFunctionPointer) + { + result = TypeCompareState.May; + } + else + { + result = (type1 == type2 ? TypeCompareState.Must : TypeCompareState.MustNot); + } } // If either or both types are canonical subtypes, we can sometimes prove inequality. else From 0b633462eda33d8e2c54c49d8fd4659b015184bb Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Fri, 15 Jul 2022 15:44:19 -0700 Subject: [PATCH 05/13] leave fnptrs broken for now --- src/coreclr/jit/gentree.cpp | 22 ++-- .../tools/Common/JitInterface/CorInfoImpl.cs | 21 +--- src/coreclr/vm/jitinterface.cpp | 29 +---- .../JIT/opt/ValueNumbering/TypeTestFolding.cs | 104 +++++++++--------- 4 files changed, 63 insertions(+), 113 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 79e832c671348e..b15378b8e9b979 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -12910,26 +12910,18 @@ GenTree* Compiler::gtFoldTypeCompare(GenTree* tree) // // Find out how we can compare the two handles. // NOTE: We're potentially passing NO_CLASS_HANDLE, but the runtime knows what to do with it here. - // - CorInfoInlineTypeCheck inliningKind1 = + CorInfoInlineTypeCheck inliningKind = info.compCompHnd->canInlineTypeCheck(cls1Hnd, CORINFO_INLINE_TYPECHECK_SOURCE_TOKEN); - CorInfoInlineTypeCheck inliningKind2 = - info.compCompHnd->canInlineTypeCheck(cls2Hnd, CORINFO_INLINE_TYPECHECK_SOURCE_TOKEN); - // If either type needs a comparison helper, just bail on the optimization. - // The helper no longer exists. - // - if ((inliningKind1 == CORINFO_INLINE_TYPECHECK_USE_HELPER) || - (inliningKind2 == CORINFO_INLINE_TYPECHECK_USE_HELPER)) + // If the first type needs helper, check the other type: it might be okay with a simple compare. + if (inliningKind == CORINFO_INLINE_TYPECHECK_USE_HELPER) { - JITDUMP("Comparison requires helper, not optimizing\n"); - return tree; + inliningKind = info.compCompHnd->canInlineTypeCheck(cls2Hnd, CORINFO_INLINE_TYPECHECK_SOURCE_TOKEN); } - assert(inliningKind1 == CORINFO_INLINE_TYPECHECK_PASS); - assert(inliningKind2 == CORINFO_INLINE_TYPECHECK_PASS); - GenTree* compare = - gtCreateHandleCompare(oper, op1ClassFromHandle, op2ClassFromHandle, CORINFO_INLINE_TYPECHECK_PASS); + assert(inliningKind == CORINFO_INLINE_TYPECHECK_PASS || inliningKind == CORINFO_INLINE_TYPECHECK_USE_HELPER); + + GenTree* compare = gtCreateHandleCompare(oper, op1ClassFromHandle, op2ClassFromHandle, inliningKind); // Drop any now-irrelvant flags compare->gtFlags |= tree->gtFlags & (GTF_RELOP_JMP_USED | GTF_DONT_CSE); diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index 6e7ce6fc6a0f1b..cdcb74e5a6b0e6 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -1923,17 +1923,6 @@ private CorInfoInlineTypeCheck canInlineTypeCheck(CORINFO_CLASS_STRUCT_* cls, Co { // TODO: when we support multiple modules at runtime, this will need to do more work // NOTE: cls can be null - - // Block the jit from optimizing type tests involving function pointers. - if (cls != null) - { - TypeDesc type = HandleToObject(cls); - if (type.IsFunctionPointer) - { - return CorInfoInlineTypeCheck.CORINFO_INLINE_TYPECHECK_USE_HELPER; - } - } - return CorInfoInlineTypeCheck.CORINFO_INLINE_TYPECHECK_PASS; } @@ -2601,15 +2590,7 @@ private TypeCompareState compareTypesForEquality(CORINFO_CLASS_STRUCT_* cls1, CO // If neither type is a canonical subtype, type handle comparison suffices if (!type1.IsCanonicalSubtype(CanonicalFormKind.Any) && !type2.IsCanonicalSubtype(CanonicalFormKind.Any)) { - // FunctionPointer types are special. Avoid letting the jit optimize them for now. - if (type1.IsFunctionPointer || type2.IsFunctionPointer) - { - result = TypeCompareState.May; - } - else - { - result = (type1 == type2 ? TypeCompareState.Must : TypeCompareState.MustNot); - } + result = (type1 == type2 ? TypeCompareState.Must : TypeCompareState.MustNot); } // If either or both types are canonical subtypes, we can sometimes prove inequality. else diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 87f207b0f912f1..a6bd0d8ea26c33 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -3543,24 +3543,8 @@ bool CEEInfo::isValueClass(CORINFO_CLASS_HANDLE clsHnd) // This will enable to use directly the typehandle instead of going through getClassByHandle CorInfoInlineTypeCheck CEEInfo::canInlineTypeCheck(CORINFO_CLASS_HANDLE clsHnd, CorInfoInlineTypeCheckSource source) { - CONTRACTL { - NOTHROW; - GC_NOTRIGGER; - MODE_PREEMPTIVE; - } CONTRACTL_END; - - CorInfoInlineTypeCheck result = CORINFO_INLINE_TYPECHECK_PASS; - - JIT_TO_EE_TRANSITION_LEAF(); - - if (TypeHandle(clsHnd).IsFnPtrType()) - { - result = CORINFO_INLINE_TYPECHECK_USE_HELPER; - } - - EE_TO_JIT_TRANSITION_LEAF(); - - return result; + LIMITED_METHOD_CONTRACT; + return CORINFO_INLINE_TYPECHECK_PASS; } /*********************************************************************/ @@ -4237,14 +4221,7 @@ TypeCompareState CEEInfo::compareTypesForEquality( // If neither type is a canonical subtype, type handle comparison suffices if (!hnd1.IsCanonicalSubtype() && !hnd2.IsCanonicalSubtype()) { - if (hnd1.IsFnPtrType() || hnd2.IsFnPtrType()) - { - result = TypeCompareState::May; - } - else - { - result = (hnd1 == hnd2 ? TypeCompareState::Must : TypeCompareState::MustNot); - } + result = (hnd1 == hnd2 ? TypeCompareState::Must : TypeCompareState::MustNot); } // If either or both types are canonical subtypes, we can sometimes prove inequality. else diff --git a/src/tests/JIT/opt/ValueNumbering/TypeTestFolding.cs b/src/tests/JIT/opt/ValueNumbering/TypeTestFolding.cs index 4c46605c264ef9..3041e558c9a22b 100644 --- a/src/tests/JIT/opt/ValueNumbering/TypeTestFolding.cs +++ b/src/tests/JIT/opt/ValueNumbering/TypeTestFolding.cs @@ -13,14 +13,14 @@ class TypeTestFolding [MethodImpl(MethodImplOptions.NoInlining)] static void SideEffect() { } - static bool True0() => typeof(delegate*) == typeof(delegate* unmanaged); - static bool True1() - { - var t0 = typeof(delegate*); - SideEffect(); - var t1 = typeof(delegate* unmanaged); - return t0 == t1; - } + //static bool True0() => typeof(delegate*) == typeof(delegate* unmanaged); + //static bool True1() + //{ + // var t0 = typeof(delegate*); + // SideEffect(); + // var t1 = typeof(delegate* unmanaged); + // return t0 == t1; + //} static bool True2() => typeof(TypeTestFolding) == typeof(TypeTestFolding); static bool True3() @@ -40,108 +40,108 @@ static bool True5() return t0 == t1; } - static bool True6() => typeof(delegate*) == typeof(nint); - static bool True7() - { - var t0 = typeof(delegate*); - SideEffect(); - var t1 = typeof(nint); - return t0 == t1; - } + //static bool True6() => typeof(delegate*) == typeof(nint); + //static bool True7() + //{ + // var t0 = typeof(delegate*); + // SideEffect(); + // var t1 = typeof(nint); + // return t0 == t1; + //} static bool False0() => typeof(List) == typeof(List); - static bool False1() + static bool False1() { var t0 = typeof(List); SideEffect(); var t1 = typeof(List); return t0 == t1; - } + } static bool False2() => typeof(int) == typeof(Enum1); - static bool False3() + static bool False3() { var t0 = typeof(int); SideEffect(); var t1 = typeof(Enum1); return t0 == t1; - } + } static bool False4() => typeof(Enum1) == typeof(Enum2); - static bool False5() + static bool False5() { var t0 = typeof(Enum1); SideEffect(); var t1 = typeof(Enum2); return t0 == t1; - } - + } + static bool False6() => typeof(int?) == typeof(uint?); - static bool False7() + static bool False7() { var t0 = typeof(int?); SideEffect(); var t1 = typeof(uint?); return t0 == t1; - } + } static bool False8() => typeof(int?) == typeof(Enum1?); - static bool False9() + static bool False9() { var t0 = typeof(int?); SideEffect(); var t1 = typeof(Enum1?); return t0 == t1; - } + } static bool False10() => typeof(ValueTuple) == typeof(ValueTuple); - static bool False11() + static bool False11() { var t0 = typeof(ValueTuple); SideEffect(); var t1 = typeof(ValueTuple); return t0 == t1; - } + } - static bool False12() => typeof(delegate*[]) == typeof(delegate*[]); - static bool False13() - { - var t0 = typeof(delegate*[]); - SideEffect(); - var t1 = typeof(delegate*[]); - return t0 == t1; - } + //static bool False12() => typeof(delegate*[]) == typeof(delegate*[]); + //static bool False13() + //{ + // var t0 = typeof(delegate*[]); + // SideEffect(); + // var t1 = typeof(delegate*[]); + // return t0 == t1; + //} static bool False14() => typeof(int[]) == typeof(uint[]); - static bool False15() + static bool False15() { var t0 = typeof(int[]); SideEffect(); var t1 = typeof(uint[]); return t0 == t1; - } + } - static bool False16() => typeof(delegate*) == typeof(UIntPtr); - static bool False17() - { - var t0 = typeof(delegate*); - SideEffect(); - var t1 = typeof(UIntPtr); - return t0 == t1; - } + //static bool False16() => typeof(delegate*) == typeof(IntPtr); + //static bool False17() + //{ + // var t0 = typeof(delegate*); + // SideEffect(); + // var t1 = typeof(UIntPtr); + // return t0 == t1; + //} unsafe static int Main() { - delegate*[] trueFuncs = new delegate*[] { &True0, &True1, &True2, &True3, &True4, &True5, &True6, &True7 }; - delegate*[] falseFuncs = new delegate*[] { &False0, &False1, &False2, &False3, &False4, &False5, + delegate*[] trueFuncs = new delegate*[] { &True2, &True3, &True4, &True5 }; + delegate*[] falseFuncs = new delegate*[] { &False0, &False1, &False2, &False3, &False4, &False5, &False6, &False7, &False8, &False9, &False10, &False11, - &False12, &False13, &False14, &False15, &False16, &False17 }; + &False14, &False15 }; int result = 100; int trueCount = 0; int falseCount = 0; - foreach(var tf in trueFuncs) + foreach (var tf in trueFuncs) { if (!tf()) { @@ -151,7 +151,7 @@ unsafe static int Main() trueCount++; } - foreach(var ff in falseFuncs) + foreach (var ff in falseFuncs) { if (ff()) { From 0092cb504bb71d45a4a00085f341fc49318263e0 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Mon, 18 Jul 2022 18:43:04 -0700 Subject: [PATCH 06/13] map from embedded handle to compile time handle and fully evaluate VN --- src/coreclr/jit/smallhash.h | 18 +++++++++++++ src/coreclr/jit/valuenum.cpp | 49 ++++++++++++++++++++++++++++-------- src/coreclr/jit/valuenum.h | 8 ++++++ 3 files changed, 64 insertions(+), 11 deletions(-) diff --git a/src/coreclr/jit/smallhash.h b/src/coreclr/jit/smallhash.h index ff4445eb456248..ff2af50d137bd4 100644 --- a/src/coreclr/jit/smallhash.h +++ b/src/coreclr/jit/smallhash.h @@ -75,6 +75,24 @@ struct HashTableInfo } }; +//------------------------------------------------------------------------ +// HashTableInfo: specialized version of HashTableInfo for ssize_t- +// typed keys. +template <> +struct HashTableInfo +{ + static bool Equals(ssize_t x, ssize_t y) + { + return x == y; + } + + static unsigned GetHashCode(ssize_t key) + { + // Return the key itself + return (unsigned)key; + } +}; + //------------------------------------------------------------------------ // HashTableBase: base type for HashTable and SmallHashTable. This class // provides the vast majority of the implementation. The diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index d7686dd1d2a0c0..9c51010f11ec14 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -439,6 +439,7 @@ ValueNumStore::ValueNumStore(Compiler* comp, CompAllocator alloc) , m_intCnsMap(nullptr) , m_longCnsMap(nullptr) , m_handleMap(nullptr) + , m_embeddedToCompileTimeHandleMap(alloc) , m_floatCnsMap(nullptr) , m_doubleCnsMap(nullptr) , m_byrefCnsMap(nullptr) @@ -3712,28 +3713,48 @@ ValueNum ValueNumStore::VNEvalFoldTypeCompare(var_types type, VNFunc func, Value // class handles and the VM agrees comparing these gives the same // result as comparing the runtime types. // + // Note the VN actually tracks the value of embedded handle; + // we need to pass the VM the associated the compile time handles. + // ValueNum handle0 = arg0Func.m_args[0]; ValueNum handle1 = arg1Func.m_args[0]; + if (!IsVNHandle(handle0) || !IsVNHandle(handle1)) { return NoVN; } assert(GetHandleFlags(handle0) == GTF_ICON_CLASS_HDL); assert(GetHandleFlags(handle1) == GTF_ICON_CLASS_HDL); - CORINFO_CLASS_HANDLE cls0Hnd = CORINFO_CLASS_HANDLE(ConstantValue(handle0)); - CORINFO_CLASS_HANDLE cls1Hnd = CORINFO_CLASS_HANDLE(ConstantValue(handle1)); - TypeCompareState s = m_pComp->info.compCompHnd->compareTypesForEquality(cls0Hnd, cls1Hnd); + const ssize_t handleVal0 = ConstantValue(handle0); + const ssize_t handleVal1 = ConstantValue(handle1); + ssize_t compileTimeHandle0; + ssize_t compileTimeHandle1; + + // These mappings should always exist. + // + const bool found0 = m_embeddedToCompileTimeHandleMap.TryGetValue(handleVal0, &compileTimeHandle0); + const bool found1 = m_embeddedToCompileTimeHandleMap.TryGetValue(handleVal1, &compileTimeHandle1); + assert(found0 && found1); + + JITDUMP("Asking runtime to compare %p (%s) and %p (%s) for equality\n", dspPtr(compileTimeHandle0), + m_pComp->eeGetClassName(CORINFO_CLASS_HANDLE(compileTimeHandle0)), dspPtr(compileTimeHandle1), + m_pComp->eeGetClassName(CORINFO_CLASS_HANDLE(compileTimeHandle1))); - if (s == TypeCompareState::May) + ValueNum result = NoVN; + const TypeCompareState s = + m_pComp->info.compCompHnd->compareTypesForEquality(CORINFO_CLASS_HANDLE(compileTimeHandle0), + CORINFO_CLASS_HANDLE(compileTimeHandle1)); + if (s != TypeCompareState::May) { - return NoVN; + const bool typesAreEqual = (s == TypeCompareState::Must); + const bool operatorIsEQ = (oper == GT_EQ); + const int compareResult = operatorIsEQ ^ typesAreEqual ? 0 : 1; + JITDUMP("Runtime reports comparison is known at jit time: %u\n", compareResult); + result = VNForIntCon(compareResult); } - // At this point we know enough to determine if the resulting compare will - // be true or false, but we'll let normal const folding take over. - // - return VNForFunc(type, func, handle0, handle1); + return result; } //------------------------------------------------------------------------ @@ -8014,8 +8035,14 @@ void Compiler::fgValueNumberTreeConst(GenTree* tree) case TYP_BOOL: if (tree->IsIconHandle()) { - tree->gtVNPair.SetBoth( - vnStore->VNForHandle(ssize_t(tree->AsIntConCommon()->IconValue()), tree->GetIconHandleFlag())); + const ssize_t embeddedHandle = tree->AsIntCon()->IconValue(); + const ssize_t compileTimeHandle = tree->AsIntCon()->gtCompileTimeHandle; + tree->gtVNPair.SetBoth(vnStore->VNForHandle(embeddedHandle, tree->GetIconHandleFlag())); + if (tree->GetIconHandleFlag() == GTF_ICON_CLASS_HDL) + { + assert(compileTimeHandle != 0); + vnStore->AddToEmbeddedHandleMap(embeddedHandle, compileTimeHandle); + } } else if ((typ == TYP_LONG) || (typ == TYP_ULONG)) { diff --git a/src/coreclr/jit/valuenum.h b/src/coreclr/jit/valuenum.h index 8fba842c78d96c..e188b3d76e227c 100644 --- a/src/coreclr/jit/valuenum.h +++ b/src/coreclr/jit/valuenum.h @@ -461,6 +461,11 @@ class ValueNumStore // that happens to be the same... ValueNum VNForHandle(ssize_t cnsVal, GenTreeFlags iconFlags); + void AddToEmbeddedHandleMap(ssize_t embeddedHandle, ssize_t compileTimeHandle) + { + m_embeddedToCompileTimeHandleMap.AddOrUpdate(embeddedHandle, compileTimeHandle); + } + // And the single constant for an object reference type. static ValueNum VNForNull() { @@ -1383,6 +1388,9 @@ class ValueNumStore return m_handleMap; } + typedef SmallHashTable EmbeddedToCompileTimeHandleMap; + EmbeddedToCompileTimeHandleMap m_embeddedToCompileTimeHandleMap; + struct LargePrimitiveKeyFuncsFloat : public JitLargePrimitiveKeyFuncs { static bool Equals(float x, float y) From 19d5fce869bc5577a89118bddfc27bcaf5e19882 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Mon, 18 Jul 2022 19:09:21 -0700 Subject: [PATCH 07/13] tolerate missing compile time handle --- src/coreclr/jit/valuenum.cpp | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 9c51010f11ec14..b37c98753bc6f0 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -3713,8 +3713,9 @@ ValueNum ValueNumStore::VNEvalFoldTypeCompare(var_types type, VNFunc func, Value // class handles and the VM agrees comparing these gives the same // result as comparing the runtime types. // - // Note the VN actually tracks the value of embedded handle; - // we need to pass the VM the associated the compile time handles. + // Note that VN actually tracks the value of embedded handle; + // we need to pass the VM the associated the compile time handles, + // in case they differ (say for prejitting or AOT). // ValueNum handle0 = arg0Func.m_args[0]; ValueNum handle1 = arg1Func.m_args[0]; @@ -3737,6 +3738,14 @@ ValueNum ValueNumStore::VNEvalFoldTypeCompare(var_types type, VNFunc func, Value const bool found1 = m_embeddedToCompileTimeHandleMap.TryGetValue(handleVal1, &compileTimeHandle1); assert(found0 && found1); + // We may see null compile time handles for some constructed class handle cases. + // We should fix the construction if possible. But just skip those cases for now. + // + if ((compileTimeHandle0 == 0) || (compileTimeHandle1 == 0)) + { + return NoVN; + } + JITDUMP("Asking runtime to compare %p (%s) and %p (%s) for equality\n", dspPtr(compileTimeHandle0), m_pComp->eeGetClassName(CORINFO_CLASS_HANDLE(compileTimeHandle0)), dspPtr(compileTimeHandle1), m_pComp->eeGetClassName(CORINFO_CLASS_HANDLE(compileTimeHandle1))); @@ -8035,12 +8044,11 @@ void Compiler::fgValueNumberTreeConst(GenTree* tree) case TYP_BOOL: if (tree->IsIconHandle()) { - const ssize_t embeddedHandle = tree->AsIntCon()->IconValue(); - const ssize_t compileTimeHandle = tree->AsIntCon()->gtCompileTimeHandle; + const ssize_t embeddedHandle = tree->AsIntCon()->IconValue(); tree->gtVNPair.SetBoth(vnStore->VNForHandle(embeddedHandle, tree->GetIconHandleFlag())); if (tree->GetIconHandleFlag() == GTF_ICON_CLASS_HDL) { - assert(compileTimeHandle != 0); + const ssize_t compileTimeHandle = tree->AsIntCon()->gtCompileTimeHandle; vnStore->AddToEmbeddedHandleMap(embeddedHandle, compileTimeHandle); } } From b99df8099bcc4e90da38554ef84ecd863346034f Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Mon, 18 Jul 2022 19:44:58 -0700 Subject: [PATCH 08/13] fix 32 bit builds --- src/coreclr/jit/smallhash.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/coreclr/jit/smallhash.h b/src/coreclr/jit/smallhash.h index ff2af50d137bd4..10502c77659ad1 100644 --- a/src/coreclr/jit/smallhash.h +++ b/src/coreclr/jit/smallhash.h @@ -75,6 +75,7 @@ struct HashTableInfo } }; +#ifdef HOST_32BIT //------------------------------------------------------------------------ // HashTableInfo: specialized version of HashTableInfo for ssize_t- // typed keys. @@ -92,6 +93,7 @@ struct HashTableInfo return (unsigned)key; } }; +#endif //------------------------------------------------------------------------ // HashTableBase: base type for HashTable and SmallHashTable. This class From 0156b3992c8bdb1e98cccd61e165b5a937100b73 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Mon, 18 Jul 2022 20:05:42 -0700 Subject: [PATCH 09/13] really fix 32 bit --- src/coreclr/jit/smallhash.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/smallhash.h b/src/coreclr/jit/smallhash.h index 10502c77659ad1..f16905c995fbd4 100644 --- a/src/coreclr/jit/smallhash.h +++ b/src/coreclr/jit/smallhash.h @@ -75,7 +75,7 @@ struct HashTableInfo } }; -#ifdef HOST_32BIT +#ifdef HOST_64BIT //------------------------------------------------------------------------ // HashTableInfo: specialized version of HashTableInfo for ssize_t- // typed keys. From d51c73ae4cbb0ce8a979f621315d1ef26a506b46 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Tue, 19 Jul 2022 07:58:19 -0700 Subject: [PATCH 10/13] add mono AOT excludes --- src/tests/issues.targets | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/tests/issues.targets b/src/tests/issues.targets index a284d2bff70841..07469ff0e09dca 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -2777,6 +2777,10 @@ https://github.com/dotnet/runtime/issues/67675 + + + https://github.com/dotnet/runtime/issues/72460 + @@ -3064,6 +3068,10 @@ https://github.com/dotnet/runtime/issues/67675 + + + https://github.com/dotnet/runtime/issues/72460 + From a0906298202af084b208faa804055c62d0bd233d Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Tue, 19 Jul 2022 16:47:21 -0700 Subject: [PATCH 11/13] minor TP refactor --- src/coreclr/jit/valuenum.cpp | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index b37c98753bc6f0..96aa2587539eaa 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -2139,10 +2139,14 @@ ValueNum ValueNumStore::VNForFunc(var_types typ, VNFunc func, ValueNum arg0VN, V // When both operands are runtime types we can sometimes also fold. // This is the VN analog of gtFoldTypeCompare // - resultVN = VNEvalFoldTypeCompare(typ, func, arg0VN, arg1VN); - if (resultVN != NoVN) + const genTreeOps oper = genTreeOps(func); + if (GenTree::StaticOperIs(oper, GT_EQ, GT_NE)) { - return resultVN; + resultVN = VNEvalFoldTypeCompare(typ, func, arg0VN, arg1VN); + if (resultVN != NoVN) + { + return resultVN; + } } // We canonicalize commutative operations. @@ -3682,16 +3686,8 @@ ValueNum ValueNumStore::EvalBitCastForConstantArgs(var_types dstType, ValueNum a // ValueNum ValueNumStore::VNEvalFoldTypeCompare(var_types type, VNFunc func, ValueNum arg0VN, ValueNum arg1VN) { - if (func >= VNF_Boundary) - { - return NoVN; - } - const genTreeOps oper = genTreeOps(func); - if (!GenTree::StaticOperIs(oper, GT_EQ, GT_NE)) - { - return NoVN; - } + assert(GenTree::StaticOperIs(oper, GT_EQ, GT_NE)); VNFuncApp arg0Func; const bool arg0IsFunc = GetVNFunc(arg0VN, &arg0Func); @@ -3718,12 +3714,17 @@ ValueNum ValueNumStore::VNEvalFoldTypeCompare(var_types type, VNFunc func, Value // in case they differ (say for prejitting or AOT). // ValueNum handle0 = arg0Func.m_args[0]; - ValueNum handle1 = arg1Func.m_args[0]; + if (!IsVNHandle(handle0)) + { + return NoVN; + } - if (!IsVNHandle(handle0) || !IsVNHandle(handle1)) + ValueNum handle1 = arg1Func.m_args[0]; + if (!IsVNHandle(handle1)) { return NoVN; } + assert(GetHandleFlags(handle0) == GTF_ICON_CLASS_HDL); assert(GetHandleFlags(handle1) == GTF_ICON_CLASS_HDL); From b926330e52b36b8a43eeec212de9d76d3b2881a3 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Tue, 19 Jul 2022 20:14:07 -0700 Subject: [PATCH 12/13] restrict to unequal VNs, remove mono test exclusions --- src/coreclr/jit/valuenum.cpp | 9 ++++++--- src/tests/issues.targets | 8 -------- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 96aa2587539eaa..d348659de4f441 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -2136,11 +2136,14 @@ ValueNum ValueNumStore::VNForFunc(var_types typ, VNFunc func, ValueNum arg0VN, V ValueNum resultVN = NoVN; - // When both operands are runtime types we can sometimes also fold. - // This is the VN analog of gtFoldTypeCompare + // Even if the argVNs differ, if both operands runtime types constructed from handles, + // we can sometimes also fold. + // + // The case where the arg VNs are equal is handled by EvalUsingMathIdentity below. + // This is the VN analog of gtFoldTypeCompare. // const genTreeOps oper = genTreeOps(func); - if (GenTree::StaticOperIs(oper, GT_EQ, GT_NE)) + if ((arg0VN != arg1VN) && GenTree::StaticOperIs(oper, GT_EQ, GT_NE)) { resultVN = VNEvalFoldTypeCompare(typ, func, arg0VN, arg1VN); if (resultVN != NoVN) diff --git a/src/tests/issues.targets b/src/tests/issues.targets index 07469ff0e09dca..a284d2bff70841 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -2777,10 +2777,6 @@ https://github.com/dotnet/runtime/issues/67675 - - - https://github.com/dotnet/runtime/issues/72460 - @@ -3068,10 +3064,6 @@ https://github.com/dotnet/runtime/issues/67675 - - - https://github.com/dotnet/runtime/issues/72460 - From 0c8cb31942d711cfba53b1867800110ad69dbb0b Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Thu, 21 Jul 2022 10:59:35 -0700 Subject: [PATCH 13/13] re-add mono exclusions --- src/tests/issues.targets | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/tests/issues.targets b/src/tests/issues.targets index a284d2bff70841..07469ff0e09dca 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -2777,6 +2777,10 @@ https://github.com/dotnet/runtime/issues/67675 + + + https://github.com/dotnet/runtime/issues/72460 + @@ -3064,6 +3068,10 @@ https://github.com/dotnet/runtime/issues/67675 + + + https://github.com/dotnet/runtime/issues/72460 +