From c2c81bb298d69dc590df049e8bfbb5105b893064 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 7 Jul 2022 20:41:28 +0200 Subject: [PATCH 1/8] Propagate typeof() during inlining --- src/coreclr/jit/importer.cpp | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 8cff503d5eabeb..6fd95c212adacb 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -20913,6 +20913,16 @@ GenTree* Compiler::impInlineFetchArg(unsigned lclNum, InlArgInfo* inlArgInfo, In argInfo.argHasTmp = true; argInfo.argTmpNum = tmpNum; + bool substitute = false; + + if (argNode->IsCall() && + (argNode->AsCall()->gtCallMethHnd == eeFindHelper(CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPE)) && + (gtGetHelperArgClassHandle(argNode->AsCall()->gtArgs.GetArgByIndex(0)->GetEarlyNode()) != + NO_CLASS_HANDLE)) + { + substitute = true; + } + // If we require strict exception order, then arguments must // be evaluated in sequence before the body of the inlined method. // So we need to evaluate them to a temp. @@ -20922,8 +20932,8 @@ GenTree* Compiler::impInlineFetchArg(unsigned lclNum, InlArgInfo* inlArgInfo, In // TODO-1stClassStructs: We currently do not reuse an existing lclVar // if it is a struct, because it requires some additional handling. - if ((!varTypeIsStruct(lclTyp) && !argInfo.argHasSideEff && !argInfo.argHasGlobRef && - !argInfo.argHasCallerLocalRef)) + if (substitute || (!varTypeIsStruct(lclTyp) && !argInfo.argHasSideEff && !argInfo.argHasGlobRef && + !argInfo.argHasCallerLocalRef)) { /* Get a *LARGE* LCL_VAR node */ op1 = gtNewLclLNode(tmpNum, genActualType(lclTyp) DEBUGARG(lclNum)); From ac821f522a12e7ce673e7d1a26a9877675fac38d Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 8 Jul 2022 22:21:40 +0200 Subject: [PATCH 2/8] test --- src/coreclr/jit/fgbasic.cpp | 2 +- src/coreclr/jit/fginline.cpp | 3 ++- src/coreclr/jit/importer.cpp | 31 ++++++++++++++++++------------- src/coreclr/jit/inline.h | 1 + 4 files changed, 22 insertions(+), 15 deletions(-) diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index f818fd6018e6da..3d20a0b4573cc6 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -833,7 +833,7 @@ class FgStack const unsigned argNum = value - SLOT_ARGUMENT; if (argNum < info->argCnt) { - return info->inlArgInfo[argNum].argIsInvariant; + return info->inlArgInfo[argNum].argIsInvariant || info->inlArgInfo[argNum].argIsInvariantComplex; } return false; } diff --git a/src/coreclr/jit/fginline.cpp b/src/coreclr/jit/fginline.cpp index d59301991aaa61..137f8ba83b5acc 100644 --- a/src/coreclr/jit/fginline.cpp +++ b/src/coreclr/jit/fginline.cpp @@ -1606,7 +1606,8 @@ Statement* Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo) { /* The argument is either not used or a const or lcl var */ - noway_assert(!argInfo.argIsUsed || argInfo.argIsInvariant || argInfo.argIsLclVar); + noway_assert(!argInfo.argIsUsed || argInfo.argIsInvariant || argInfo.argIsLclVar || + argInfo.argIsInvariantComplex); /* Make sure we didnt change argNode's along the way, or else subsequent uses of the arg would have worked with the bashed value */ diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 6fd95c212adacb..32da4ebba46ce6 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -20206,6 +20206,17 @@ void Compiler::impInlineRecordArgInfo(InlineInfo* pInlineInfo, return; } } + else + { + if (curArgVal->IsCall() && + (curArgVal->AsCall()->gtCallMethHnd == eeFindHelper(CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPE)) && + (gtGetHelperArgClassHandle(curArgVal->AsCall()->gtArgs.GetArgByIndex(0)->GetEarlyNode()) != + NO_CLASS_HANDLE)) + { + inlCurArgInfo->argIsInvariantComplex = true; + inlCurArgInfo->argHasSideEff = false; + } + } bool isExact = false; bool isNonNull = false; @@ -20244,6 +20255,10 @@ void Compiler::impInlineRecordArgInfo(InlineInfo* pInlineInfo, { printf(" is a constant"); } + if (inlCurArgInfo->argIsInvariantComplex) + { + printf(" is an invariant complex expr"); + } if (inlCurArgInfo->argHasGlobRef) { printf(" has global refs"); @@ -20759,7 +20774,7 @@ GenTree* Compiler::impInlineFetchArg(unsigned lclNum, InlArgInfo* inlArgInfo, In GenTree* argNode = argInfo.arg->GetNode(); assert(!argNode->OperIs(GT_RET_EXPR)); - if (argInfo.argIsInvariant && !argCanBeModified) + if ((argInfo.argIsInvariant || argInfo.argIsInvariantComplex) && !argCanBeModified) { // Directly substitute constants or addresses of locals // @@ -20913,16 +20928,6 @@ GenTree* Compiler::impInlineFetchArg(unsigned lclNum, InlArgInfo* inlArgInfo, In argInfo.argHasTmp = true; argInfo.argTmpNum = tmpNum; - bool substitute = false; - - if (argNode->IsCall() && - (argNode->AsCall()->gtCallMethHnd == eeFindHelper(CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPE)) && - (gtGetHelperArgClassHandle(argNode->AsCall()->gtArgs.GetArgByIndex(0)->GetEarlyNode()) != - NO_CLASS_HANDLE)) - { - substitute = true; - } - // If we require strict exception order, then arguments must // be evaluated in sequence before the body of the inlined method. // So we need to evaluate them to a temp. @@ -20932,8 +20937,8 @@ GenTree* Compiler::impInlineFetchArg(unsigned lclNum, InlArgInfo* inlArgInfo, In // TODO-1stClassStructs: We currently do not reuse an existing lclVar // if it is a struct, because it requires some additional handling. - if (substitute || (!varTypeIsStruct(lclTyp) && !argInfo.argHasSideEff && !argInfo.argHasGlobRef && - !argInfo.argHasCallerLocalRef)) + if ((!varTypeIsStruct(lclTyp) && !argInfo.argHasSideEff && !argInfo.argHasGlobRef && + !argInfo.argHasCallerLocalRef)) { /* Get a *LARGE* LCL_VAR node */ op1 = gtNewLclLNode(tmpNum, genActualType(lclTyp) DEBUGARG(lclNum)); diff --git a/src/coreclr/jit/inline.h b/src/coreclr/jit/inline.h index f21a77da4fa147..946bfa721b8de5 100644 --- a/src/coreclr/jit/inline.h +++ b/src/coreclr/jit/inline.h @@ -638,6 +638,7 @@ struct InlArgInfo unsigned argTmpNum; // the argument tmp number unsigned argIsUsed : 1; // is this arg used at all? unsigned argIsInvariant : 1; // the argument is a constant or a local variable address + unsigned argIsInvariantComplex : 1; // the argument is a complex invariant expression e.g. "typeof(int)" unsigned argIsLclVar : 1; // the argument is a local variable unsigned argIsThis : 1; // the argument is the 'this' pointer unsigned argHasSideEff : 1; // the argument has side effects From b8df7616b7dfdbdc1da7f0685d26ffd0f7ce243a Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 9 Jul 2022 01:22:47 +0200 Subject: [PATCH 3/8] Clean up --- src/coreclr/jit/fgbasic.cpp | 2 +- src/coreclr/jit/fginline.cpp | 16 +++------------- src/coreclr/jit/importer.cpp | 18 ++++++------------ src/coreclr/jit/inline.h | 1 - 4 files changed, 10 insertions(+), 27 deletions(-) diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index 3d20a0b4573cc6..f818fd6018e6da 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -833,7 +833,7 @@ class FgStack const unsigned argNum = value - SLOT_ARGUMENT; if (argNum < info->argCnt) { - return info->inlArgInfo[argNum].argIsInvariant || info->inlArgInfo[argNum].argIsInvariantComplex; + return info->inlArgInfo[argNum].argIsInvariant; } return false; } diff --git a/src/coreclr/jit/fginline.cpp b/src/coreclr/jit/fginline.cpp index 137f8ba83b5acc..9232c9a0ee5dd5 100644 --- a/src/coreclr/jit/fginline.cpp +++ b/src/coreclr/jit/fginline.cpp @@ -1604,22 +1604,12 @@ Statement* Compiler::fgInlinePrependStatements(InlineInfo* inlineInfo) } else { - /* The argument is either not used or a const or lcl var */ - - noway_assert(!argInfo.argIsUsed || argInfo.argIsInvariant || argInfo.argIsLclVar || - argInfo.argIsInvariantComplex); - - /* Make sure we didnt change argNode's along the way, or else - subsequent uses of the arg would have worked with the bashed value */ - if (argInfo.argIsInvariant) - { - assert(argNode->OperIsConst() || argNode->gtOper == GT_ADDR); - } + // The argument is either not used or a const or lcl var + noway_assert(!argInfo.argIsUsed || argInfo.argIsInvariant || argInfo.argIsLclVar); noway_assert((argInfo.argIsLclVar == 0) == (argNode->gtOper != GT_LCL_VAR || (argNode->gtFlags & GTF_GLOB_REF))); - /* If the argument has side effects, append it */ - + // If the argument has side effects, append it if (argInfo.argHasSideEff) { noway_assert(argInfo.argIsUsed == false); diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 32da4ebba46ce6..c1755b54250bed 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -20213,8 +20213,8 @@ void Compiler::impInlineRecordArgInfo(InlineInfo* pInlineInfo, (gtGetHelperArgClassHandle(curArgVal->AsCall()->gtArgs.GetArgByIndex(0)->GetEarlyNode()) != NO_CLASS_HANDLE)) { - inlCurArgInfo->argIsInvariantComplex = true; - inlCurArgInfo->argHasSideEff = false; + inlCurArgInfo->argIsInvariant = true; + inlCurArgInfo->argHasSideEff = false; } } @@ -20253,11 +20253,7 @@ void Compiler::impInlineRecordArgInfo(InlineInfo* pInlineInfo, } if (inlCurArgInfo->argIsInvariant) { - printf(" is a constant"); - } - if (inlCurArgInfo->argIsInvariantComplex) - { - printf(" is an invariant complex expr"); + printf(" is a constant or invariant"); } if (inlCurArgInfo->argHasGlobRef) { @@ -20523,10 +20519,9 @@ void Compiler::impInlineInitVars(InlineInfo* pInlineInfo) inlArgInfo[i].argIsLclVar = false; // Try to fold the node in case we have constant arguments. - if (inlArgInfo[i].argIsInvariant) + if (inlArgInfo[i].argIsInvariant && inlArgNode->OperIsConst()) { inlArgNode = gtFoldExprConst(inlArgNode); - assert(inlArgNode->OperIsConst()); } *pInlArgNode = inlArgNode; } @@ -20540,10 +20535,9 @@ void Compiler::impInlineInitVars(InlineInfo* pInlineInfo) /* Try to fold the node in case we have constant arguments */ - if (inlArgInfo[i].argIsInvariant) + if (inlArgInfo[i].argIsInvariant && inlArgNode->OperIsConst()) { inlArgNode = gtFoldExprConst(inlArgNode); - assert(inlArgNode->OperIsConst()); } *pInlArgNode = inlArgNode; } @@ -20774,7 +20768,7 @@ GenTree* Compiler::impInlineFetchArg(unsigned lclNum, InlArgInfo* inlArgInfo, In GenTree* argNode = argInfo.arg->GetNode(); assert(!argNode->OperIs(GT_RET_EXPR)); - if ((argInfo.argIsInvariant || argInfo.argIsInvariantComplex) && !argCanBeModified) + if (argInfo.argIsInvariant && !argCanBeModified) { // Directly substitute constants or addresses of locals // diff --git a/src/coreclr/jit/inline.h b/src/coreclr/jit/inline.h index 946bfa721b8de5..f21a77da4fa147 100644 --- a/src/coreclr/jit/inline.h +++ b/src/coreclr/jit/inline.h @@ -638,7 +638,6 @@ struct InlArgInfo unsigned argTmpNum; // the argument tmp number unsigned argIsUsed : 1; // is this arg used at all? unsigned argIsInvariant : 1; // the argument is a constant or a local variable address - unsigned argIsInvariantComplex : 1; // the argument is a complex invariant expression e.g. "typeof(int)" unsigned argIsLclVar : 1; // the argument is a local variable unsigned argIsThis : 1; // the argument is the 'this' pointer unsigned argHasSideEff : 1; // the argument has side effects From 906c097af7a0c3bac6ed8124f04cdc9241a60493 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 9 Jul 2022 11:08:14 +0200 Subject: [PATCH 4/8] test --- src/coreclr/jit/importer.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index c1755b54250bed..11d0f43bb0217c 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -20519,9 +20519,9 @@ void Compiler::impInlineInitVars(InlineInfo* pInlineInfo) inlArgInfo[i].argIsLclVar = false; // Try to fold the node in case we have constant arguments. - if (inlArgInfo[i].argIsInvariant && inlArgNode->OperIsConst()) + if (inlArgInfo[i].argIsInvariant) { - inlArgNode = gtFoldExprConst(inlArgNode); + inlArgNode = gtFoldExpr(inlArgNode); } *pInlArgNode = inlArgNode; } @@ -20533,11 +20533,10 @@ void Compiler::impInlineInitVars(InlineInfo* pInlineInfo) inlArgInfo[i].argIsLclVar = false; - /* Try to fold the node in case we have constant arguments */ - - if (inlArgInfo[i].argIsInvariant && inlArgNode->OperIsConst()) + // Try to fold the node in case we have constant arguments. + if (inlArgInfo[i].argIsInvariant) { - inlArgNode = gtFoldExprConst(inlArgNode); + inlArgNode = gtFoldExpr(inlArgNode); } *pInlArgNode = inlArgNode; } From f60d6f624eb0cdf6768c6ab5ad891313aa35bb99 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 9 Jul 2022 16:25:54 +0200 Subject: [PATCH 5/8] fold typeof() != null --- src/coreclr/jit/gentree.cpp | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index ae7583871a2bc7..7b99d0dd0fa4a2 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -12913,17 +12913,19 @@ GenTree* Compiler::gtFoldTypeCompare(GenTree* tree) // Screen for the right kinds of operands GenTree* const op1 = tree->AsOp()->gtOp1; - const TypeProducerKind op1Kind = gtGetTypeProducerKind(op1); - if (op1Kind == TPK_Unknown) - { - return tree; - } - GenTree* const op2 = tree->AsOp()->gtOp2; + const TypeProducerKind op1Kind = gtGetTypeProducerKind(op1); const TypeProducerKind op2Kind = gtGetTypeProducerKind(op2); - if (op2Kind == TPK_Unknown) + + // Fold "typeof(handle) cmp null" + if ((op2->IsIntegralConst(0) && (op1Kind == TPK_Handle)) || (op1->IsIntegralConst(0) && (op2Kind == TPK_Handle))) { - return tree; + GenTree* call = op1Kind == TPK_Handle ? op1 : op2; + GenTree* handle = call->AsCall()->gtArgs.GetArgByIndex(0)->GetNode(); + if (gtGetHelperArgClassHandle(handle) != NO_CLASS_HANDLE) + { + return oper == GT_EQ ? gtNewFalse() : gtNewTrue(); + } } // If both types are created via handles, we can simply compare From d4ecae9f360d706f43522ce1a8611c1985c7161e Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Sat, 9 Jul 2022 21:14:04 +0200 Subject: [PATCH 6/8] Update src/coreclr/jit/gentree.cpp Co-authored-by: Andy Ayers --- src/coreclr/jit/gentree.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 7b99d0dd0fa4a2..d177397615dd40 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -12918,7 +12918,7 @@ GenTree* Compiler::gtFoldTypeCompare(GenTree* tree) const TypeProducerKind op2Kind = gtGetTypeProducerKind(op2); // Fold "typeof(handle) cmp null" - if ((op2->IsIntegralConst(0) && (op1Kind == TPK_Handle)) || (op1->IsIntegralConst(0) && (op2Kind == TPK_Handle))) + if (((op2Kind == TPK_Null) && (op1Kind == TPK_Handle)) || ((op1Kind == TPK_ Null) && (op2Kind == TPK_Handle))) { GenTree* call = op1Kind == TPK_Handle ? op1 : op2; GenTree* handle = call->AsCall()->gtArgs.GetArgByIndex(0)->GetNode(); From a5f4e3e383ecee41d480ed2bf17f3df5520f3b50 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 9 Jul 2022 21:16:14 +0200 Subject: [PATCH 7/8] clean up --- src/coreclr/jit/gentree.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index d177397615dd40..065d122b82fac6 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -12896,6 +12896,7 @@ GenTree* Compiler::gtCreateHandleCompare(genTreeOps oper, // Checks for // typeof(...) == obj.GetType() // typeof(...) == typeof(...) +// typeof(...) == null // obj1.GetType() == obj2.GetType() // // And potentially optimizes away the need to obtain actual @@ -12918,7 +12919,7 @@ GenTree* Compiler::gtFoldTypeCompare(GenTree* tree) const TypeProducerKind op2Kind = gtGetTypeProducerKind(op2); // Fold "typeof(handle) cmp null" - if (((op2Kind == TPK_Null) && (op1Kind == TPK_Handle)) || ((op1Kind == TPK_ Null) && (op2Kind == TPK_Handle))) + if (((op2Kind == TPK_Null) && (op1Kind == TPK_Handle)) || ((op1Kind == TPK_Null) && (op2Kind == TPK_Handle))) { GenTree* call = op1Kind == TPK_Handle ? op1 : op2; GenTree* handle = call->AsCall()->gtArgs.GetArgByIndex(0)->GetNode(); From 262c9d3b96c4f092c7a3a482ec7d05fb710468d6 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 10 Jul 2022 13:30:20 +0200 Subject: [PATCH 8/8] clean up --- src/coreclr/jit/importer.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 11d0f43bb0217c..e9b647b9114b52 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -20208,8 +20208,7 @@ void Compiler::impInlineRecordArgInfo(InlineInfo* pInlineInfo, } else { - if (curArgVal->IsCall() && - (curArgVal->AsCall()->gtCallMethHnd == eeFindHelper(CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPE)) && + if (curArgVal->IsHelperCall() && gtIsTypeHandleToRuntimeTypeHelper(curArgVal->AsCall()) && (gtGetHelperArgClassHandle(curArgVal->AsCall()->gtArgs.GetArgByIndex(0)->GetEarlyNode()) != NO_CLASS_HANDLE)) {