From f31dd26bbe09b841809ec14ed2b5446a56bd98b1 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 12 Apr 2023 00:11:08 +0200 Subject: [PATCH 01/19] Optimize GDV check --- src/coreclr/jit/gentree.cpp | 46 +++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index a7bc183192e7e5..a48c823c42527c 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -13419,6 +13419,52 @@ GenTree* Compiler::gtFoldTypeCompare(GenTree* tree) return compare; } + // Assist GDV and remove the guard if we can get the actual type of object, e.g. + // + // * NE int + // +--* IND long + // | \--* LCL_VAR ref V02 tmp1 + // \--* CNS_INT(h) long 0x7ffe684d6df8 class + // + // Here if we can get the exact class of LCL_VAR we can fold the whole check + // + if (op1->OperIs(GT_IND) && op2->IsIconHandle(GTF_ICON_CLASS_HDL)) + { + GenTreeIndir* indir = op1->AsIndir(); + if (indir->HasBase() && !indir->HasIndex() && (indir->Offset() == 0) && indir->Base()->TypeIs(TYP_REF)) + { + bool isExact = false; + bool isNonNull = false; + CORINFO_CLASS_HANDLE handle = gtGetClassHandle(indir->Base(), &isExact, &isNonNull); + if ((handle != NO_CLASS_HANDLE) && isExact) + { + GenTree* result; + if (reinterpret_cast(handle) == op2->AsIntCon()->IconValue()) + { + result = gtNewIconNode(oper == GT_EQ ? 1 : 0); + } + else + { + result = gtNewIconNode(oper == GT_EQ ? 0 : 1); + } + + GenTree* op1SideEffects = nullptr; + gtExtractSideEffList(op1, &op1SideEffects, GTF_ALL_EFFECT); + if (op1SideEffects != nullptr) + { + // Keep side-effects of op1 + DEBUG_DESTROY_NODE(tree); + return gtNewOperNode(GT_COMMA, TYP_INT, op1SideEffects, result); + } + else + { + DEBUG_DESTROY_NODE(tree, op1); + return result; + } + } + } + } + // If one operand creates a type from a handle and the other operand is fetching the type from an object, // we can sometimes optimize the type compare into a simpler // method table comparison. From 7a754d67c509dd02169b9d3a9445844da80bff5e Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 12 Apr 2023 00:23:45 +0200 Subject: [PATCH 02/19] Improve --- src/coreclr/jit/gentree.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index a48c823c42527c..5be2628abf6697 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -13428,7 +13428,8 @@ GenTree* Compiler::gtFoldTypeCompare(GenTree* tree) // // Here if we can get the exact class of LCL_VAR we can fold the whole check // - if (op1->OperIs(GT_IND) && op2->IsIconHandle(GTF_ICON_CLASS_HDL)) + if (op1->OperIs(GT_IND) && op2->IsIconHandle(GTF_ICON_CLASS_HDL) && op1->TypeIs(TYP_I_IMPL) && + op2->TypeIs(TYP_I_IMPL)) { GenTreeIndir* indir = op1->AsIndir(); if (indir->HasBase() && !indir->HasIndex() && (indir->Offset() == 0) && indir->Base()->TypeIs(TYP_REF)) @@ -13447,6 +13448,7 @@ GenTree* Compiler::gtFoldTypeCompare(GenTree* tree) { result = gtNewIconNode(oper == GT_EQ ? 0 : 1); } + assert((oper == GT_EQ) || (oper == GT_NE)); GenTree* op1SideEffects = nullptr; gtExtractSideEffList(op1, &op1SideEffects, GTF_ALL_EFFECT); From 0246f11b251c4adab3aabb242d8f5286123a44cb Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 12 Apr 2023 01:25:05 +0200 Subject: [PATCH 03/19] move to VN --- src/coreclr/jit/gentree.cpp | 48 ------------------------------------ src/coreclr/jit/valuenum.cpp | 41 +++++++++++++++++++++--------- 2 files changed, 29 insertions(+), 60 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 5be2628abf6697..a7bc183192e7e5 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -13419,54 +13419,6 @@ GenTree* Compiler::gtFoldTypeCompare(GenTree* tree) return compare; } - // Assist GDV and remove the guard if we can get the actual type of object, e.g. - // - // * NE int - // +--* IND long - // | \--* LCL_VAR ref V02 tmp1 - // \--* CNS_INT(h) long 0x7ffe684d6df8 class - // - // Here if we can get the exact class of LCL_VAR we can fold the whole check - // - if (op1->OperIs(GT_IND) && op2->IsIconHandle(GTF_ICON_CLASS_HDL) && op1->TypeIs(TYP_I_IMPL) && - op2->TypeIs(TYP_I_IMPL)) - { - GenTreeIndir* indir = op1->AsIndir(); - if (indir->HasBase() && !indir->HasIndex() && (indir->Offset() == 0) && indir->Base()->TypeIs(TYP_REF)) - { - bool isExact = false; - bool isNonNull = false; - CORINFO_CLASS_HANDLE handle = gtGetClassHandle(indir->Base(), &isExact, &isNonNull); - if ((handle != NO_CLASS_HANDLE) && isExact) - { - GenTree* result; - if (reinterpret_cast(handle) == op2->AsIntCon()->IconValue()) - { - result = gtNewIconNode(oper == GT_EQ ? 1 : 0); - } - else - { - result = gtNewIconNode(oper == GT_EQ ? 0 : 1); - } - assert((oper == GT_EQ) || (oper == GT_NE)); - - GenTree* op1SideEffects = nullptr; - gtExtractSideEffList(op1, &op1SideEffects, GTF_ALL_EFFECT); - if (op1SideEffects != nullptr) - { - // Keep side-effects of op1 - DEBUG_DESTROY_NODE(tree); - return gtNewOperNode(GT_COMMA, TYP_INT, op1SideEffects, result); - } - else - { - DEBUG_DESTROY_NODE(tree, op1); - return result; - } - } - } - } - // If one operand creates a type from a handle and the other operand is fetching the type from an object, // we can sometimes optimize the type compare into a simpler // method table comparison. diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 9e9ce8caad5269..0f0e3de27fcdbb 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -10606,23 +10606,40 @@ void Compiler::fgValueNumberTree(GenTree* tree) { assert(!isVolatile); // We don't expect both volatile and invariant - // Are we dereferencing the method table slot of some newly allocated object? - // - bool wasNewobj = false; - if ((oper == GT_IND) && (addr->TypeGet() == TYP_REF) && (tree->TypeGet() == TYP_I_IMPL)) + bool returnsTypeHandle = false; + if ((oper == GT_IND) && addr->TypeIs(TYP_REF) && tree->TypeIs(TYP_I_IMPL)) { - VNFuncApp funcApp; - const bool addrIsVNFunc = vnStore->GetVNFunc(addrNvnp.GetLiberal(), &funcApp); - - if (addrIsVNFunc && (funcApp.m_func == VNF_JitNew) && addrNvnp.BothEqual()) + GenTreeIndir* indir = tree->AsIndir(); + if (!indir->HasIndex() && (indir->Offset() == 0)) { - tree->gtVNPair = - vnStore->VNPWithExc(ValueNumPair(funcApp.m_args[0], funcApp.m_args[0]), addrXvnp); - wasNewobj = true; + // We try to access GC object's type, let's see if we know the exact type already + // First, we're trying to do that via gtGetClassHandle. + // + bool isExact = false; + bool isNonNull = false; + CORINFO_CLASS_HANDLE handle = gtGetClassHandle(addr, &isExact, &isNonNull); + if (isExact && (handle != NO_CLASS_HANDLE)) + { + ValueNum handleVn = vnStore->VNForIntPtrCon((ssize_t)handle); + tree->gtVNPair = vnStore->VNPWithExc(ValueNumPair(handleVn, handleVn), addrXvnp); + returnsTypeHandle = true; + } + else + { + // Then, let's see if we can find JitNew at least + VNFuncApp funcApp; + const bool addrIsVNFunc = vnStore->GetVNFunc(addrNvnp.GetLiberal(), &funcApp); + if (addrIsVNFunc && (funcApp.m_func == VNF_JitNew) && addrNvnp.BothEqual()) + { + tree->gtVNPair = + vnStore->VNPWithExc(ValueNumPair(funcApp.m_args[0], funcApp.m_args[0]), addrXvnp); + returnsTypeHandle = true; + } + } } } - if (!wasNewobj) + if (!returnsTypeHandle) { // Indirections off of addresses for boxed statics represent bases for // the address of the static itself. Here we will use "nullptr" for the From 3e5d5d6b638cbc3f560af5fbb6a1443d53a6e8c0 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 12 Apr 2023 01:40:43 +0200 Subject: [PATCH 04/19] use handle --- src/coreclr/jit/valuenum.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 0f0e3de27fcdbb..8f2d855f08c0a7 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -10620,8 +10620,8 @@ void Compiler::fgValueNumberTree(GenTree* tree) CORINFO_CLASS_HANDLE handle = gtGetClassHandle(addr, &isExact, &isNonNull); if (isExact && (handle != NO_CLASS_HANDLE)) { - ValueNum handleVn = vnStore->VNForIntPtrCon((ssize_t)handle); - tree->gtVNPair = vnStore->VNPWithExc(ValueNumPair(handleVn, handleVn), addrXvnp); + ValueNum handleVN = vnStore->VNForHandle((ssize_t)handle, GTF_ICON_CLASS_HDL); + tree->gtVNPair = vnStore->VNPWithExc(ValueNumPair(handleVN, handleVN), addrXvnp); returnsTypeHandle = true; } else From 493461158ac2827ec38c5b33a07b374eadc99d1a Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 12 Apr 2023 11:52:04 +0200 Subject: [PATCH 05/19] Add in Morph too --- src/coreclr/jit/morph.cpp | 20 +++++++++++++++++ src/coreclr/jit/valuenum.cpp | 42 ++++++++++++++++-------------------- 2 files changed, 39 insertions(+), 23 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index cc524e5421485b..4d992303eede09 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -9843,6 +9843,26 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA } } + if (tree->TypeIs(TYP_I_IMPL) && op1->TypeIs(TYP_REF) && !gtIsActiveCSE_Candidate(tree) && + !gtIsActiveCSE_Candidate(op1)) + { + bool isExact = false; + bool isNonNull = false; + CORINFO_CLASS_HANDLE handle = gtGetClassHandle(op1, &isExact, &isNonNull); + if (isExact && (handle != NO_CLASS_HANDLE)) + { + GenTree* result = gtNewIconNode((ssize_t)handle, TYP_I_IMPL); + GenTree* sideEffects = nullptr; + gtExtractSideEffList(tree, &sideEffects, GTF_ALL_EFFECT); + if (sideEffects != nullptr) + { + result = gtNewOperNode(GT_COMMA, TYP_I_IMPL, sideEffects, result); + } + INDEBUG(result->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); + return result; + } + } + #ifdef TARGET_ARM GenTree* effOp1 = op1->gtEffectiveVal(true); // Check for a misalignment floating point indirection. diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 8f2d855f08c0a7..a8bc24bc616e07 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -10609,33 +10609,29 @@ void Compiler::fgValueNumberTree(GenTree* tree) bool returnsTypeHandle = false; if ((oper == GT_IND) && addr->TypeIs(TYP_REF) && tree->TypeIs(TYP_I_IMPL)) { - GenTreeIndir* indir = tree->AsIndir(); - if (!indir->HasIndex() && (indir->Offset() == 0)) + // We try to access GC object's type, let's see if we know the exact type already + // First, we're trying to do that via gtGetClassHandle. + // + bool isExact = false; + bool isNonNull = false; + CORINFO_CLASS_HANDLE handle = gtGetClassHandle(addr, &isExact, &isNonNull); + if (isExact && (handle != NO_CLASS_HANDLE)) { - // We try to access GC object's type, let's see if we know the exact type already - // First, we're trying to do that via gtGetClassHandle. - // - bool isExact = false; - bool isNonNull = false; - CORINFO_CLASS_HANDLE handle = gtGetClassHandle(addr, &isExact, &isNonNull); - if (isExact && (handle != NO_CLASS_HANDLE)) + ValueNum handleVN = vnStore->VNForHandle((ssize_t)handle, GTF_ICON_CLASS_HDL); + tree->gtVNPair = vnStore->VNPWithExc(ValueNumPair(handleVN, handleVN), addrXvnp); + returnsTypeHandle = true; + } + else + { + // Then, let's see if we can find JitNew at least + VNFuncApp funcApp; + const bool addrIsVNFunc = vnStore->GetVNFunc(addrNvnp.GetLiberal(), &funcApp); + if (addrIsVNFunc && (funcApp.m_func == VNF_JitNew) && addrNvnp.BothEqual()) { - ValueNum handleVN = vnStore->VNForHandle((ssize_t)handle, GTF_ICON_CLASS_HDL); - tree->gtVNPair = vnStore->VNPWithExc(ValueNumPair(handleVN, handleVN), addrXvnp); + tree->gtVNPair = + vnStore->VNPWithExc(ValueNumPair(funcApp.m_args[0], funcApp.m_args[0]), addrXvnp); returnsTypeHandle = true; } - else - { - // Then, let's see if we can find JitNew at least - VNFuncApp funcApp; - const bool addrIsVNFunc = vnStore->GetVNFunc(addrNvnp.GetLiberal(), &funcApp); - if (addrIsVNFunc && (funcApp.m_func == VNF_JitNew) && addrNvnp.BothEqual()) - { - tree->gtVNPair = - vnStore->VNPWithExc(ValueNumPair(funcApp.m_args[0], funcApp.m_args[0]), addrXvnp); - returnsTypeHandle = true; - } - } } } From 4446e6d375cab43351eaa4db945d42fd602ef557 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 12 Apr 2023 11:58:42 +0200 Subject: [PATCH 06/19] add comments --- src/coreclr/jit/morph.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 4d992303eede09..fe8c566c72878f 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -9843,6 +9843,8 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA } } + // Fold obj->pMT to a constant type handle if we know the exact type of the IND target. + // These are typically produced by GDV. if (tree->TypeIs(TYP_I_IMPL) && op1->TypeIs(TYP_REF) && !gtIsActiveCSE_Candidate(tree) && !gtIsActiveCSE_Candidate(op1)) { From 846e7d79bdfbeed6df69e23f422339ae3a5515ae Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Wed, 12 Apr 2023 13:15:27 +0200 Subject: [PATCH 07/19] Update morph.cpp --- src/coreclr/jit/morph.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index fe8c566c72878f..88891cd55a6a7d 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -9853,7 +9853,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA CORINFO_CLASS_HANDLE handle = gtGetClassHandle(op1, &isExact, &isNonNull); if (isExact && (handle != NO_CLASS_HANDLE)) { - GenTree* result = gtNewIconNode((ssize_t)handle, TYP_I_IMPL); + GenTree* result = gtNewIconHandleNode((size_t)handle, GTF_ICON_CLASS_HDL); GenTree* sideEffects = nullptr; gtExtractSideEffList(tree, &sideEffects, GTF_ALL_EFFECT); if (sideEffects != nullptr) From 349e77ac54fb16e8c962998ee6a2a370b7d74c9e Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 12 Apr 2023 13:44:55 +0200 Subject: [PATCH 08/19] fix build --- src/coreclr/jit/morph.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 88891cd55a6a7d..637f61e7782c81 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -9853,7 +9853,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA CORINFO_CLASS_HANDLE handle = gtGetClassHandle(op1, &isExact, &isNonNull); if (isExact && (handle != NO_CLASS_HANDLE)) { - GenTree* result = gtNewIconHandleNode((size_t)handle, GTF_ICON_CLASS_HDL); + GenTree* result = gtNewIconEmbClsHndNode(handle); GenTree* sideEffects = nullptr; gtExtractSideEffList(tree, &sideEffects, GTF_ALL_EFFECT); if (sideEffects != nullptr) From 036f431f20f22cdad7e97ebf3cd884430bf1a9e1 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 12 Apr 2023 14:18:26 +0200 Subject: [PATCH 09/19] remove morph part --- src/coreclr/jit/morph.cpp | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 637f61e7782c81..cc524e5421485b 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -9843,28 +9843,6 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA } } - // Fold obj->pMT to a constant type handle if we know the exact type of the IND target. - // These are typically produced by GDV. - if (tree->TypeIs(TYP_I_IMPL) && op1->TypeIs(TYP_REF) && !gtIsActiveCSE_Candidate(tree) && - !gtIsActiveCSE_Candidate(op1)) - { - bool isExact = false; - bool isNonNull = false; - CORINFO_CLASS_HANDLE handle = gtGetClassHandle(op1, &isExact, &isNonNull); - if (isExact && (handle != NO_CLASS_HANDLE)) - { - GenTree* result = gtNewIconEmbClsHndNode(handle); - GenTree* sideEffects = nullptr; - gtExtractSideEffList(tree, &sideEffects, GTF_ALL_EFFECT); - if (sideEffects != nullptr) - { - result = gtNewOperNode(GT_COMMA, TYP_I_IMPL, sideEffects, result); - } - INDEBUG(result->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); - return result; - } - } - #ifdef TARGET_ARM GenTree* effOp1 = op1->gtEffectiveVal(true); // Check for a misalignment floating point indirection. From 30855afbdc4c1285ed426a6cb0ff4b701e850240 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 13 Apr 2023 02:32:07 +0200 Subject: [PATCH 10/19] fix naot --- src/coreclr/jit/valuenum.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index a8bc24bc616e07..775e51023647e9 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -10617,9 +10617,14 @@ void Compiler::fgValueNumberTree(GenTree* tree) CORINFO_CLASS_HANDLE handle = gtGetClassHandle(addr, &isExact, &isNonNull); if (isExact && (handle != NO_CLASS_HANDLE)) { - ValueNum handleVN = vnStore->VNForHandle((ssize_t)handle, GTF_ICON_CLASS_HDL); - tree->gtVNPair = vnStore->VNPWithExc(ValueNumPair(handleVN, handleVN), addrXvnp); - returnsTypeHandle = true; + // Make sure the "exact" handle is not shared + if (info.compCompHnd->compareTypesForEquality(handle, handle) == TypeCompareState::Must) + { + JITDUMP("IND(obj) is actually a class handle for %s\n", eeGetClassName(handle)); + ValueNum handleVN = vnStore->VNForHandle((ssize_t)handle, GTF_ICON_CLASS_HDL); + tree->gtVNPair = vnStore->VNPWithExc(ValueNumPair(handleVN, handleVN), addrXvnp); + returnsTypeHandle = true; + } } else { From be87146c1f05c1ad12d7a5b6751895b0533d6cbc Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 13 Apr 2023 12:36:16 +0200 Subject: [PATCH 11/19] test --- src/coreclr/jit/valuenum.cpp | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 775e51023647e9..37ee8a850caf47 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -10617,13 +10617,16 @@ void Compiler::fgValueNumberTree(GenTree* tree) CORINFO_CLASS_HANDLE handle = gtGetClassHandle(addr, &isExact, &isNonNull); if (isExact && (handle != NO_CLASS_HANDLE)) { - // Make sure the "exact" handle is not shared - if (info.compCompHnd->compareTypesForEquality(handle, handle) == TypeCompareState::Must) + if (info.compCompHnd->canInlineTypeCheck(handle, CORINFO_INLINE_TYPECHECK_SOURCE_VTABLE) == + CORINFO_INLINE_TYPECHECK_PASS) { - JITDUMP("IND(obj) is actually a class handle for %s\n", eeGetClassName(handle)); - ValueNum handleVN = vnStore->VNForHandle((ssize_t)handle, GTF_ICON_CLASS_HDL); - tree->gtVNPair = vnStore->VNPWithExc(ValueNumPair(handleVN, handleVN), addrXvnp); - returnsTypeHandle = true; + if (info.compCompHnd->compareTypesForEquality(handle, handle) == TypeCompareState::Must) + { + JITDUMP("IND(obj) is actually a class handle for %s\n", eeGetClassName(handle)); + ValueNum handleVN = vnStore->VNForHandle((ssize_t)handle, GTF_ICON_CLASS_HDL); + tree->gtVNPair = vnStore->VNPWithExc(ValueNumPair(handleVN, handleVN), addrXvnp); + returnsTypeHandle = true; + } } } else From 38eea8af36e90e95b23e7be0ef7bd241fee07691 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 13 Apr 2023 16:27:54 +0200 Subject: [PATCH 12/19] more coservative fix --- src/coreclr/jit/valuenum.cpp | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 37ee8a850caf47..a094eb7969b5a5 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -10618,15 +10618,13 @@ void Compiler::fgValueNumberTree(GenTree* tree) if (isExact && (handle != NO_CLASS_HANDLE)) { if (info.compCompHnd->canInlineTypeCheck(handle, CORINFO_INLINE_TYPECHECK_SOURCE_VTABLE) == - CORINFO_INLINE_TYPECHECK_PASS) + CORINFO_INLINE_TYPECHECK_PASS && + impIsClassExact(handle)) { - if (info.compCompHnd->compareTypesForEquality(handle, handle) == TypeCompareState::Must) - { - JITDUMP("IND(obj) is actually a class handle for %s\n", eeGetClassName(handle)); - ValueNum handleVN = vnStore->VNForHandle((ssize_t)handle, GTF_ICON_CLASS_HDL); - tree->gtVNPair = vnStore->VNPWithExc(ValueNumPair(handleVN, handleVN), addrXvnp); - returnsTypeHandle = true; - } + JITDUMP("IND(obj) is actually a class handle for %s\n", eeGetClassName(handle)); + ValueNum handleVN = vnStore->VNForHandle((ssize_t)handle, GTF_ICON_CLASS_HDL); + tree->gtVNPair = vnStore->VNPWithExc(ValueNumPair(handleVN, handleVN), addrXvnp); + returnsTypeHandle = true; } } else From f41fe8359a23551f40cf50e2d7315606f88121d9 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 14 Apr 2023 01:06:12 +0200 Subject: [PATCH 13/19] test2 --- src/coreclr/jit/valuenum.cpp | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index a094eb7969b5a5..162b3ea26c553b 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -10617,15 +10617,13 @@ void Compiler::fgValueNumberTree(GenTree* tree) CORINFO_CLASS_HANDLE handle = gtGetClassHandle(addr, &isExact, &isNonNull); if (isExact && (handle != NO_CLASS_HANDLE)) { - if (info.compCompHnd->canInlineTypeCheck(handle, CORINFO_INLINE_TYPECHECK_SOURCE_VTABLE) == - CORINFO_INLINE_TYPECHECK_PASS && - impIsClassExact(handle)) - { - JITDUMP("IND(obj) is actually a class handle for %s\n", eeGetClassName(handle)); - ValueNum handleVN = vnStore->VNForHandle((ssize_t)handle, GTF_ICON_CLASS_HDL); - tree->gtVNPair = vnStore->VNPWithExc(ValueNumPair(handleVN, handleVN), addrXvnp); - returnsTypeHandle = true; - } + JITDUMP("IND(obj) is actually a class handle for %s\n", eeGetClassName(handle)); + void* pEmbedClsHnd; + void* embedClsHnd = (void*)info.compCompHnd->embedClassHandle(handle, &pEmbedClsHnd); + ssize_t cnsHandle = (ssize_t)(pEmbedClsHnd != nullptr ? pEmbedClsHnd : embedClsHnd); + ValueNum handleVN = vnStore->VNForHandle(cnsHandle, GTF_ICON_CLASS_HDL); + tree->gtVNPair = vnStore->VNPWithExc(ValueNumPair(handleVN, handleVN), addrXvnp); + returnsTypeHandle = true; } else { From 2477124570c7a491942f1c043411b52ad9f2f5c8 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 14 Apr 2023 14:36:25 +0200 Subject: [PATCH 14/19] fix cg? --- src/coreclr/jit/valuenum.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 162b3ea26c553b..e2b89abb0af0e5 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -10617,6 +10617,7 @@ void Compiler::fgValueNumberTree(GenTree* tree) CORINFO_CLASS_HANDLE handle = gtGetClassHandle(addr, &isExact, &isNonNull); if (isExact && (handle != NO_CLASS_HANDLE)) { + info.compCompHnd->classMustBeLoadedBeforeCodeIsRun(handle); JITDUMP("IND(obj) is actually a class handle for %s\n", eeGetClassName(handle)); void* pEmbedClsHnd; void* embedClsHnd = (void*)info.compCompHnd->embedClassHandle(handle, &pEmbedClsHnd); From 70e5e38d4eab8088630d2d89295ebd1be5c3911e Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 14 Apr 2023 17:40:39 +0200 Subject: [PATCH 15/19] skip indirect handles --- src/coreclr/jit/valuenum.cpp | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index e2b89abb0af0e5..a17def87a3e740 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -10619,12 +10619,17 @@ void Compiler::fgValueNumberTree(GenTree* tree) { info.compCompHnd->classMustBeLoadedBeforeCodeIsRun(handle); JITDUMP("IND(obj) is actually a class handle for %s\n", eeGetClassName(handle)); - void* pEmbedClsHnd; - void* embedClsHnd = (void*)info.compCompHnd->embedClassHandle(handle, &pEmbedClsHnd); - ssize_t cnsHandle = (ssize_t)(pEmbedClsHnd != nullptr ? pEmbedClsHnd : embedClsHnd); - ValueNum handleVN = vnStore->VNForHandle(cnsHandle, GTF_ICON_CLASS_HDL); - tree->gtVNPair = vnStore->VNPWithExc(ValueNumPair(handleVN, handleVN), addrXvnp); - returnsTypeHandle = true; + void* pEmbedClsHnd; + void* embedClsHnd = (void*)info.compCompHnd->embedClassHandle(handle, &pEmbedClsHnd); + if (pEmbedClsHnd == nullptr) + { + // Skip indirect handles for now since this path is mostly for PGO scenarios + assert(embedClsHnd != nullptr); + ssize_t cnsHandle = (ssize_t)(pEmbedClsHnd != nullptr ? pEmbedClsHnd : embedClsHnd); + ValueNum handleVN = vnStore->VNForHandle(cnsHandle, GTF_ICON_CLASS_HDL); + tree->gtVNPair = vnStore->VNPWithExc(ValueNumPair(handleVN, handleVN), addrXvnp); + returnsTypeHandle = true; + } } else { From 3b188099aadc8f6e69216f854b9eab2273fa51bb Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 14 Apr 2023 23:50:22 +0200 Subject: [PATCH 16/19] Address feedback --- src/coreclr/jit/valuenum.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index a17def87a3e740..b13a693ff3aede 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -10625,8 +10625,7 @@ void Compiler::fgValueNumberTree(GenTree* tree) { // Skip indirect handles for now since this path is mostly for PGO scenarios assert(embedClsHnd != nullptr); - ssize_t cnsHandle = (ssize_t)(pEmbedClsHnd != nullptr ? pEmbedClsHnd : embedClsHnd); - ValueNum handleVN = vnStore->VNForHandle(cnsHandle, GTF_ICON_CLASS_HDL); + ValueNum handleVN = vnStore->VNForHandle((ssize_t)embedClsHnd, GTF_ICON_CLASS_HDL); tree->gtVNPair = vnStore->VNPWithExc(ValueNumPair(handleVN, handleVN), addrXvnp); returnsTypeHandle = true; } From 602a2ba700360cb0174ef03daa71f05d04e94982 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Sat, 15 Apr 2023 00:07:55 +0200 Subject: [PATCH 17/19] Update src/coreclr/jit/valuenum.cpp Co-authored-by: Jan Kotas --- src/coreclr/jit/valuenum.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index b13a693ff3aede..280e58dece0d0d 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -10617,7 +10617,6 @@ void Compiler::fgValueNumberTree(GenTree* tree) CORINFO_CLASS_HANDLE handle = gtGetClassHandle(addr, &isExact, &isNonNull); if (isExact && (handle != NO_CLASS_HANDLE)) { - info.compCompHnd->classMustBeLoadedBeforeCodeIsRun(handle); JITDUMP("IND(obj) is actually a class handle for %s\n", eeGetClassName(handle)); void* pEmbedClsHnd; void* embedClsHnd = (void*)info.compCompHnd->embedClassHandle(handle, &pEmbedClsHnd); From beb3ab9a0b8f82651678bd0788d79eeb837cb07f Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 21 Apr 2023 18:27:37 +0200 Subject: [PATCH 18/19] fix naot assert --- src/coreclr/jit/valuenum.cpp | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 47eecca26517be..10651259991a03 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -10650,15 +10650,19 @@ void Compiler::fgValueNumberTree(GenTree* tree) if (isExact && (handle != NO_CLASS_HANDLE)) { JITDUMP("IND(obj) is actually a class handle for %s\n", eeGetClassName(handle)); - void* pEmbedClsHnd; - void* embedClsHnd = (void*)info.compCompHnd->embedClassHandle(handle, &pEmbedClsHnd); - if (pEmbedClsHnd == nullptr) + // Filter out all shared generic instantiations + if (info.compCompHnd->compareTypesForEquality(handle, handle) != TypeCompareState::May) { - // Skip indirect handles for now since this path is mostly for PGO scenarios - assert(embedClsHnd != nullptr); - ValueNum handleVN = vnStore->VNForHandle((ssize_t)embedClsHnd, GTF_ICON_CLASS_HDL); - tree->gtVNPair = vnStore->VNPWithExc(ValueNumPair(handleVN, handleVN), addrXvnp); - returnsTypeHandle = true; + void* pEmbedClsHnd; + void* embedClsHnd = (void*)info.compCompHnd->embedClassHandle(handle, &pEmbedClsHnd); + if (pEmbedClsHnd == nullptr) + { + // Skip indirect handles for now since this path is mostly for PGO scenarios + assert(embedClsHnd != nullptr); + ValueNum handleVN = vnStore->VNForHandle((ssize_t)embedClsHnd, GTF_ICON_CLASS_HDL); + tree->gtVNPair = vnStore->VNPWithExc(ValueNumPair(handleVN, handleVN), addrXvnp); + returnsTypeHandle = true; + } } } else From 166e900a7c8e7835f6c23246f39df5de40f177e5 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 21 Apr 2023 22:45:31 +0200 Subject: [PATCH 19/19] Apply Jan's suggestion --- src/coreclr/jit/valuenum.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 10651259991a03..9b580f69c672a4 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -10651,7 +10651,7 @@ void Compiler::fgValueNumberTree(GenTree* tree) { JITDUMP("IND(obj) is actually a class handle for %s\n", eeGetClassName(handle)); // Filter out all shared generic instantiations - if (info.compCompHnd->compareTypesForEquality(handle, handle) != TypeCompareState::May) + if ((info.compCompHnd->getClassAttribs(handle) & CORINFO_FLG_SHAREDINST) == 0) { void* pEmbedClsHnd; void* embedClsHnd = (void*)info.compCompHnd->embedClassHandle(handle, &pEmbedClsHnd);