diff --git a/src/coreclr/format.patch b/src/coreclr/format.patch new file mode 100644 index 00000000000000..3759a982886803 --- /dev/null +++ b/src/coreclr/format.patch @@ -0,0 +1,47 @@ +diff --git a/src/coreclr/src/jit/importer.cpp b/src/coreclr/src/jit/importer.cpp +index a71c325ff48..c0569355c89 100644 +--- a/src/coreclr/src/jit/importer.cpp ++++ b/src/coreclr/src/jit/importer.cpp +@@ -15184,41 +15184,41 @@ void Compiler::impImportBlockCode(BasicBlock* block) + info.compCompHnd->compareTypesForEquality(resolvedToken.hClass, clsHnd); + + if (compare == TypeCompareState::Must) + { + JITDUMP("\nOptimizing %s (%s) -- type test will succeed\n", + opcode == CEE_UNBOX ? "UNBOX" : "UNBOX.ANY", eeGetClassName(clsHnd)); + + // For UNBOX, null check (if necessary), and then leave the box payload byref on the stack. + if (opcode == CEE_UNBOX) + { + GenTree* cloneOperand; + op1 = impCloneExpr(op1, &cloneOperand, NO_CLASS_HANDLE, (unsigned)CHECK_SPILL_ALL, + nullptr DEBUGARG("optimized unbox clone")); + + GenTree* boxPayloadOffset = gtNewIconNode(TARGET_POINTER_SIZE, TYP_I_IMPL); + GenTree* boxPayloadAddress = + gtNewOperNode(GT_ADD, TYP_BYREF, cloneOperand, boxPayloadOffset); + GenTree* nullcheck = gtNewOperNode(GT_NULLCHECK, TYP_I_IMPL, op1); + block->bbFlags |= BBF_HAS_NULLCHECK; + optMethodFlags |= OMF_HAS_NULLCHECK; +- GenTree* result = gtNewOperNode(GT_COMMA, TYP_BYREF, nullcheck, boxPayloadAddress); ++ GenTree* result = gtNewOperNode(GT_COMMA, TYP_BYREF, nullcheck, boxPayloadAddress); + impPushOnStack(result, tiRetVal); + break; + } + + // For UNBOX.ANY load the struct from the box payload byref (the load will nullcheck) + assert(opcode == CEE_UNBOX_ANY); + GenTree* boxPayloadOffset = gtNewIconNode(TARGET_POINTER_SIZE, TYP_I_IMPL); + GenTree* boxPayloadAddress = gtNewOperNode(GT_ADD, TYP_BYREF, op1, boxPayloadOffset); + impPushOnStack(boxPayloadAddress, tiRetVal); + oper = GT_OBJ; + goto OBJ; + } + else + { + JITDUMP("\nUnable to optimize %s -- can't resolve type comparison\n", + opcode == CEE_UNBOX ? "UNBOX" : "UNBOX.ANY"); + } + } + else + { diff --git a/src/coreclr/src/jit/block.cpp b/src/coreclr/src/jit/block.cpp index 62242d8d89934a..c44b26219e15c9 100644 --- a/src/coreclr/src/jit/block.cpp +++ b/src/coreclr/src/jit/block.cpp @@ -333,6 +333,10 @@ void BasicBlock::dspFlags() { printf("newobj "); } + if (bbFlags & BBF_HAS_NULLCHECK) + { + printf("nullcheck "); + } #if defined(FEATURE_EH_FUNCLETS) && defined(_TARGET_ARM_) if (bbFlags & BBF_FINALLY_TARGET) { diff --git a/src/coreclr/src/jit/block.h b/src/coreclr/src/jit/block.h index c4ef847b11ea4d..79e11ed39a81fb 100644 --- a/src/coreclr/src/jit/block.h +++ b/src/coreclr/src/jit/block.h @@ -465,7 +465,7 @@ struct BasicBlock : private LIR::Range #define BBF_COMPACT_UPD \ (BBF_CHANGED | BBF_GC_SAFE_POINT | BBF_HAS_JMP | BBF_NEEDS_GCPOLL | BBF_HAS_IDX_LEN | BBF_BACKWARD_JUMP | \ - BBF_HAS_NEWARRAY | BBF_HAS_NEWOBJ) + BBF_HAS_NEWARRAY | BBF_HAS_NEWOBJ | BBF_HAS_NULLCHECK) // Flags a block should not have had before it is split. @@ -481,14 +481,14 @@ struct BasicBlock : private LIR::Range // Flags gained by the bottom block when a block is split. // Note, this is a conservative guess. -// For example, the bottom block might or might not have BBF_HAS_NEWARRAY, -// but we assume it has BBF_HAS_NEWARRAY. +// For example, the bottom block might or might not have BBF_HAS_NEWARRAY or BBF_HAS_NULLCHECK, +// but we assume it has BBF_HAS_NEWARRAY and BBF_HAS_NULLCHECK. // TODO: Should BBF_RUN_RARELY be added to BBF_SPLIT_GAINED ? #define BBF_SPLIT_GAINED \ (BBF_DONT_REMOVE | BBF_HAS_LABEL | BBF_HAS_JMP | BBF_BACKWARD_JUMP | BBF_HAS_IDX_LEN | BBF_HAS_NEWARRAY | \ - BBF_PROF_WEIGHT | BBF_HAS_NEWOBJ | BBF_KEEP_BBJ_ALWAYS | BBF_CLONED_FINALLY_END) + BBF_PROF_WEIGHT | BBF_HAS_NEWOBJ | BBF_KEEP_BBJ_ALWAYS | BBF_CLONED_FINALLY_END | BBF_HAS_NULLCHECK) #ifndef __GNUC__ // GCC doesn't like C_ASSERT at global scope static_assert_no_msg((BBF_SPLIT_NONEXIST & BBF_SPLIT_LOST) == 0); diff --git a/src/coreclr/src/jit/compiler.h b/src/coreclr/src/jit/compiler.h index 23c692e1ffdd9f..938b717827691a 100644 --- a/src/coreclr/src/jit/compiler.h +++ b/src/coreclr/src/jit/compiler.h @@ -6396,17 +6396,27 @@ class Compiler OPK_NULLCHECK }; + typedef JitHashTable, GenTree*> LocalNumberToNullCheckTreeMap; + bool gtIsVtableRef(GenTree* tree); GenTree* getArrayLengthFromAllocation(GenTree* tree); GenTree* getObjectHandleNodeFromAllocation(GenTree* tree); GenTree* optPropGetValueRec(unsigned lclNum, unsigned ssaNum, optPropKind valueKind, int walkDepth); GenTree* optPropGetValue(unsigned lclNum, unsigned ssaNum, optPropKind valueKind); - GenTree* optEarlyPropRewriteTree(GenTree* tree); + GenTree* optEarlyPropRewriteTree(GenTree* tree, LocalNumberToNullCheckTreeMap* nullCheckMap); bool optDoEarlyPropForBlock(BasicBlock* block); bool optDoEarlyPropForFunc(); void optEarlyProp(); - void optFoldNullCheck(GenTree* tree); - bool optCanMoveNullCheckPastTree(GenTree* tree, bool isInsideTry); + void optFoldNullCheck(GenTree* tree, LocalNumberToNullCheckTreeMap* nullCheckMap); + GenTree* optFindNullCheckToFold(GenTree* tree, LocalNumberToNullCheckTreeMap* nullCheckMap); + bool optIsNullCheckFoldingLegal(GenTree* tree, + GenTree* nullCheckTree, + GenTree** nullCheckParent, + Statement** nullCheckStmt); + bool optCanMoveNullCheckPastTree(GenTree* tree, + unsigned nullCheckLclNum, + bool isInsideTry, + bool checkSideEffectSummary); #if ASSERTION_PROP /************************************************************************** diff --git a/src/coreclr/src/jit/compmemkind.h b/src/coreclr/src/jit/compmemkind.h index 986c4e2dce1806..f680b04987d455 100644 --- a/src/coreclr/src/jit/compmemkind.h +++ b/src/coreclr/src/jit/compmemkind.h @@ -57,6 +57,7 @@ CompMemKindMacro(ObjectAllocator) CompMemKindMacro(VariableLiveRanges) CompMemKindMacro(ClassLayout) CompMemKindMacro(TailMergeThrows) +CompMemKindMacro(EarlyProp) //clang-format on #undef CompMemKindMacro diff --git a/src/coreclr/src/jit/earlyprop.cpp b/src/coreclr/src/jit/earlyprop.cpp index 9d06857a1d86b4..64f6e98cb890af 100644 --- a/src/coreclr/src/jit/earlyprop.cpp +++ b/src/coreclr/src/jit/earlyprop.cpp @@ -183,6 +183,9 @@ void Compiler::optEarlyProp() compCurBB = block; + CompAllocator allocator(getAllocator(CMK_EarlyProp)); + LocalNumberToNullCheckTreeMap nullCheckMap(allocator); + for (Statement* stmt = block->firstStmt(); stmt != nullptr;) { // Preserve the next link before the propagation and morph. @@ -195,7 +198,7 @@ void Compiler::optEarlyProp() bool isRewritten = false; for (GenTree* tree = stmt->GetTreeList(); tree != nullptr; tree = tree->gtNext) { - GenTree* rewrittenTree = optEarlyPropRewriteTree(tree); + GenTree* rewrittenTree = optEarlyPropRewriteTree(tree, &nullCheckMap); if (rewrittenTree != nullptr) { gtUpdateSideEffects(stmt, rewrittenTree); @@ -229,26 +232,35 @@ void Compiler::optEarlyProp() // // Arguments: // tree - The input tree node to be rewritten. +// nullCheckMap - Map of the local numbers to the latest NULLCHECKs on those locals in the current basic block. // // Return Value: // Return a new tree if the original tree was successfully rewritten. // The containing tree links are updated. // -GenTree* Compiler::optEarlyPropRewriteTree(GenTree* tree) +GenTree* Compiler::optEarlyPropRewriteTree(GenTree* tree, LocalNumberToNullCheckTreeMap* nullCheckMap) { GenTree* objectRefPtr = nullptr; optPropKind propKind = optPropKind::OPK_INVALID; + if (tree->OperIsIndirOrArrLength()) + { + // optFoldNullCheck takes care of updating statement info if a null check is removed. + optFoldNullCheck(tree, nullCheckMap); + } + else + { + return nullptr; + } + if (tree->OperGet() == GT_ARR_LENGTH) { objectRefPtr = tree->AsOp()->gtOp1; propKind = optPropKind::OPK_ARRAYLEN; } - else if (tree->OperIsIndir()) + else { - // optFoldNullCheck takes care of updating statement info if a null check is removed. - optFoldNullCheck(tree); - + assert(tree->OperIsIndir()); if (gtIsVtableRef(tree)) { // Don't propagate type handles that are used as null checks, which are usually in @@ -269,10 +281,6 @@ GenTree* Compiler::optEarlyPropRewriteTree(GenTree* tree) return nullptr; } } - else - { - return nullptr; - } if (!objectRefPtr->OperIsScalarLocal() || !lvaInSsa(objectRefPtr->AsLclVarCommon()->GetLclNum())) @@ -477,208 +485,363 @@ GenTree* Compiler::optPropGetValueRec(unsigned lclNum, unsigned ssaNum, optPropK } //---------------------------------------------------------------- -// optFoldNullChecks: Try to find a GT_NULLCHECK node that can be folded into the GT_INDIR node. +// optFoldNullChecks: Try to find a GT_NULLCHECK node that can be folded into the indirection node mark it for removal +// if possible. // // Arguments: -// tree - The input GT_INDIR tree. +// tree - The input indirection tree. +// nullCheckMap - Map of the local numbers to the latest NULLCHECKs on those locals in the current basic block // +// Notes: +// If a GT_NULLCHECK node is post-dominated by an indirection node on the same local and the trees between +// the GT_NULLCHECK and the indirection don't have unsafe side effects, the GT_NULLCHECK can be removed. +// The indir will cause a NullReferenceException if and only if GT_NULLCHECK will cause the same +// NullReferenceException. -void Compiler::optFoldNullCheck(GenTree* tree) +void Compiler::optFoldNullCheck(GenTree* tree, LocalNumberToNullCheckTreeMap* nullCheckMap) { - // - // Check for a pattern like this: - // - // = - // / \. - // x comma - // / \. - // nullcheck + - // | / \. - // y y const - // - // - // some trees in the same - // basic block with - // no unsafe side effects - // - // indir - // | - // x - // - // where the const is suitably small - // and transform it into - // - // = - // / \. - // x + - // / \. - // y const - // - // - // some trees with no unsafe side effects here - // - // indir - // | - // x - if ((compCurBB->bbFlags & BBF_HAS_NULLCHECK) == 0) { return; } - assert(tree->OperIsIndir()); + GenTree* nullCheckTree = optFindNullCheckToFold(tree, nullCheckMap); + GenTree* nullCheckParent = nullptr; + Statement* nullCheckStmt = nullptr; + if ((nullCheckTree != nullptr) && optIsNullCheckFoldingLegal(tree, nullCheckTree, &nullCheckParent, &nullCheckStmt)) + { +#ifdef DEBUG + if (verbose) + { + printf("optEarlyProp Marking a null check for removal\n"); + gtDispTree(nullCheckTree); + printf("\n"); + } +#endif + // Remove the null check + nullCheckTree->gtFlags &= ~(GTF_EXCEPT | GTF_DONT_CSE); + + // Set this flag to prevent reordering + nullCheckTree->gtFlags |= GTF_ORDER_SIDEEFF; + nullCheckTree->gtFlags |= GTF_IND_NONFAULTING; + + if (nullCheckParent != nullptr) + { + nullCheckParent->gtFlags &= ~GTF_DONT_CSE; + } + + nullCheckMap->Remove(nullCheckTree->gtGetOp1()->AsLclVarCommon()->GetLclNum()); + + // Re-morph the statement. + Statement* curStmt = compCurStmt; + fgMorphBlockStmt(compCurBB, nullCheckStmt DEBUGARG("optFoldNullCheck")); + compCurStmt = curStmt; + } + + if ((tree->OperGet() == GT_NULLCHECK) && (tree->gtGetOp1()->OperGet() == GT_LCL_VAR)) + { + nullCheckMap->Set(tree->gtGetOp1()->AsLclVarCommon()->GetLclNum(), tree, + LocalNumberToNullCheckTreeMap::SetKind::Overwrite); + } +} + +//---------------------------------------------------------------- +// optFindNullCheckToFold: Try to find a GT_NULLCHECK node that can be folded into the indirection node. +// +// Arguments: +// tree - The input indirection tree. +// nullCheckMap - Map of the local numbers to the latest NULLCHECKs on those locals in the current basic block +// +// Notes: +// Check for cases where +// 1. One of the following trees +// +// nullcheck(x) +// or +// x = comma(nullcheck(y), add(y, const1)) +// +// is post-dominated in the same basic block by one of the following trees +// +// indir(x) +// or +// indir(add(x, const2)) +// +// (indir is any node for which OperIsIndirOrArrLength() is true.) +// +// 2. const1 + const2 if sufficiently small. + +GenTree* Compiler::optFindNullCheckToFold(GenTree* tree, LocalNumberToNullCheckTreeMap* nullCheckMap) +{ + assert(tree->OperIsIndirOrArrLength()); + + GenTree* addr = (tree->OperGet() == GT_ARR_LENGTH) ? tree->AsArrLen()->ArrRef() : tree->AsIndir()->Addr(); + + ssize_t offsetValue = 0; + + if ((addr->OperGet() == GT_ADD) && addr->gtGetOp2()->IsCnsIntOrI()) + { + offsetValue += addr->gtGetOp2()->AsIntConCommon()->IconValue(); + addr = addr->gtGetOp1(); + } + + if (addr->OperGet() != GT_LCL_VAR) + { + return nullptr; + } + + GenTreeLclVarCommon* const lclVarNode = addr->AsLclVarCommon(); + const unsigned ssaNum = lclVarNode->GetSsaNum(); + + if (ssaNum == SsaConfig::RESERVED_SSA_NUM) + { + return nullptr; + } + + const unsigned lclNum = lclVarNode->GetLclNum(); + GenTree* nullCheckTree = nullptr; + unsigned nullCheckLclNum = BAD_VAR_NUM; + + // Check if we saw a nullcheck on this local in this basic block + // This corresponds to nullcheck(x) tree in the header comment. + if (nullCheckMap->Lookup(lclNum, &nullCheckTree)) + { + GenTree* nullCheckAddr = nullCheckTree->AsIndir()->Addr(); + if ((nullCheckAddr->OperGet() != GT_LCL_VAR) || (nullCheckAddr->AsLclVarCommon()->GetSsaNum() != ssaNum)) + { + nullCheckTree = nullptr; + } + else + { + nullCheckLclNum = nullCheckAddr->AsLclVarCommon()->GetLclNum(); + } + } - GenTree* const addr = tree->AsIndir()->Addr(); - if (addr->OperGet() == GT_LCL_VAR) + if (nullCheckTree == nullptr) { - // Check if we have the pattern above and find the nullcheck node if we do. + // Check if we have x = comma(nullcheck(y), add(y, const1)) pattern. - // Find the definition of the indirected local (x in the picture) - GenTreeLclVarCommon* const lclVarNode = addr->AsLclVarCommon(); + // Find the definition of the indirected local ('x' in the pattern above). + LclSsaVarDsc* defLoc = lvaTable[lclNum].GetPerSsaData(ssaNum); + + if (compCurBB != defLoc->GetBlock()) + { + return nullptr; + } - const unsigned lclNum = lclVarNode->GetLclNum(); - const unsigned ssaNum = lclVarNode->GetSsaNum(); + GenTree* defRHS = defLoc->GetAssignment()->gtGetOp2(); - if (ssaNum != SsaConfig::RESERVED_SSA_NUM) + if (defRHS->OperGet() != GT_COMMA) { - LclSsaVarDsc* defLoc = lvaTable[lclNum].GetPerSsaData(ssaNum); - BasicBlock* defBlock = defLoc->GetBlock(); + return nullptr; + } + + const bool commaOnly = true; + GenTree* commaOp1EffectiveValue = defRHS->gtGetOp1()->gtEffectiveVal(commaOnly); - if (compCurBB == defBlock) + if (commaOp1EffectiveValue->OperGet() != GT_NULLCHECK) + { + return nullptr; + } + + GenTree* nullCheckAddress = commaOp1EffectiveValue->gtGetOp1(); + + if ((nullCheckAddress->OperGet() != GT_LCL_VAR) || (defRHS->gtGetOp2()->OperGet() != GT_ADD)) + { + return nullptr; + } + + // We found a candidate for 'y' in the pattern above. + + GenTree* additionNode = defRHS->gtGetOp2(); + GenTree* additionOp1 = additionNode->gtGetOp1(); + GenTree* additionOp2 = additionNode->gtGetOp2(); + if ((additionOp1->OperGet() == GT_LCL_VAR) && + (additionOp1->AsLclVarCommon()->GetLclNum() == nullCheckAddress->AsLclVarCommon()->GetLclNum()) && + (additionOp2->IsCnsIntOrI())) + { + offsetValue += additionOp2->AsIntConCommon()->IconValue(); + nullCheckTree = commaOp1EffectiveValue; + } + } + + if (fgIsBigOffset(offsetValue)) + { + return nullptr; + } + else + { + return nullCheckTree; + } +} + +//---------------------------------------------------------------- +// optIsNullCheckFoldingLegal: Check the nodes between the GT_NULLCHECK node and the indirection to determine +// if null check folding is legal. +// +// Arguments: +// tree - The input indirection tree. +// nullCheckTree - The GT_NULLCHECK tree that is a candidate for removal. +// nullCheckParent - The parent of the GT_NULLCHECK tree that is a candidate for removal (out-parameter). +// nullCheckStatement - The statement of the GT_NULLCHECK tree that is a candidate for removal (out-parameter). + +bool Compiler::optIsNullCheckFoldingLegal(GenTree* tree, + GenTree* nullCheckTree, + GenTree** nullCheckParent, + Statement** nullCheckStmt) +{ + // Check all nodes between the GT_NULLCHECK and the indirection to see + // if any nodes have unsafe side effects. + unsigned nullCheckLclNum = nullCheckTree->gtGetOp1()->AsLclVarCommon()->GetLclNum(); + bool isInsideTry = compCurBB->hasTryIndex(); + bool canRemoveNullCheck = true; + const unsigned maxNodesWalked = 50; + unsigned nodesWalked = 0; + + // First walk the nodes in the statement containing the GT_NULLCHECK in forward execution order + // until we get to the indirection or process the statement root. + GenTree* previousTree = nullCheckTree; + GenTree* currentTree = nullCheckTree->gtNext; + assert(fgStmtListThreaded); + while (canRemoveNullCheck && (currentTree != tree) && (currentTree != nullptr)) + { + if ((*nullCheckParent == nullptr) && (nullCheckTree->gtGetChildPointer(currentTree) != nullptr)) + { + *nullCheckParent = currentTree; + } + const bool checkExceptionSummary = false; + if ((nodesWalked++ > maxNodesWalked) || + !optCanMoveNullCheckPastTree(currentTree, nullCheckLclNum, isInsideTry, checkExceptionSummary)) + { + canRemoveNullCheck = false; + } + else + { + previousTree = currentTree; + currentTree = currentTree->gtNext; + } + } + + if (currentTree == tree) + { + // The GT_NULLCHECK and the indirection are in the same statements. + *nullCheckStmt = compCurStmt; + } + else + { + // The GT_NULLCHECK and the indirection are in different statements. + // Walk the nodes in the statement containing the indirection + // in reverse execution order starting with the indirection's + // predecessor. + GenTree* nullCheckStatementRoot = previousTree; + currentTree = tree->gtPrev; + while (canRemoveNullCheck && (currentTree != nullptr)) + { + const bool checkExceptionSummary = false; + if ((nodesWalked++ > maxNodesWalked) || + !optCanMoveNullCheckPastTree(currentTree, nullCheckLclNum, isInsideTry, checkExceptionSummary)) + { + canRemoveNullCheck = false; + } + else { - GenTree* defParent = defLoc->GetAssignment(); - assert(defParent->OperIs(GT_ASG)); + currentTree = currentTree->gtPrev; + } + } - if (defParent->gtNext == nullptr) - { - GenTree* defRHS = defParent->gtGetOp2(); - if (defRHS->OperGet() == GT_COMMA) - { - if (defRHS->gtGetOp1()->OperGet() == GT_NULLCHECK) - { - GenTree* nullCheckTree = defRHS->gtGetOp1(); - if (nullCheckTree->gtGetOp1()->OperGet() == GT_LCL_VAR) - { - // We found a candidate for 'y' in the picture - unsigned nullCheckLclNum = nullCheckTree->gtGetOp1()->AsLclVarCommon()->GetLclNum(); - - if (defRHS->gtGetOp2()->OperGet() == GT_ADD) - { - GenTree* additionNode = defRHS->gtGetOp2(); - if ((additionNode->gtGetOp1()->OperGet() == GT_LCL_VAR) && - (additionNode->gtGetOp1()->AsLclVarCommon()->GetLclNum() == nullCheckLclNum)) - { - GenTree* offset = additionNode->gtGetOp2(); - if (offset->IsCnsIntOrI()) - { - if (!fgIsBigOffset(offset->AsIntConCommon()->IconValue())) - { - // Walk from the use to the def in reverse execution order to see - // if any nodes have unsafe side effects. - GenTree* currentTree = lclVarNode->gtPrev; - bool isInsideTry = compCurBB->hasTryIndex(); - bool canRemoveNullCheck = true; - const unsigned maxNodesWalked = 25; - unsigned nodesWalked = 0; - - // First walk the nodes in the statement containing the indirection - // in reverse execution order starting with the indirection's - // predecessor. - while (canRemoveNullCheck && (currentTree != nullptr)) - { - if ((nodesWalked++ > maxNodesWalked) || - !optCanMoveNullCheckPastTree(currentTree, isInsideTry)) - { - canRemoveNullCheck = false; - } - else - { - currentTree = currentTree->gtPrev; - } - } - - // Then walk the statement list in reverse execution order - // until we get to the statement containing the null check. - // We only need to check the side effects at the root of each statement. - Statement* curStmt = compCurStmt->GetPrevStmt(); - currentTree = curStmt->GetRootNode(); - while (canRemoveNullCheck && (currentTree != defParent)) - { - if ((nodesWalked++ > maxNodesWalked) || - !optCanMoveNullCheckPastTree(currentTree, isInsideTry)) - { - canRemoveNullCheck = false; - } - else - { - curStmt = curStmt->GetPrevStmt(); - assert(curStmt != nullptr); - currentTree = curStmt->GetRootNode(); - } - } - - if (canRemoveNullCheck) - { - // Remove the null check - nullCheckTree->gtFlags &= ~(GTF_EXCEPT | GTF_DONT_CSE); - - // Set this flag to prevent reordering - nullCheckTree->gtFlags |= GTF_ORDER_SIDEEFF; - nullCheckTree->gtFlags |= GTF_IND_NONFAULTING; - - defRHS->gtFlags &= ~(GTF_EXCEPT | GTF_DONT_CSE); - defRHS->gtFlags |= - additionNode->gtFlags & (GTF_EXCEPT | GTF_DONT_CSE); - - // Re-morph the statement. - fgMorphBlockStmt(compCurBB, curStmt DEBUGARG("optFoldNullCheck")); - } - } - } - } - } - } - } - } - } + // Finally, walk the statement list in reverse execution order + // until we get to the statement containing the null check. + // We only check the side effects at the root of each statement. + Statement* curStmt = compCurStmt->GetPrevStmt(); + currentTree = curStmt->GetRootNode(); + while (canRemoveNullCheck && (currentTree != nullCheckStatementRoot)) + { + const bool checkExceptionSummary = true; + if ((nodesWalked++ > maxNodesWalked) || + !optCanMoveNullCheckPastTree(currentTree, nullCheckLclNum, isInsideTry, checkExceptionSummary)) + { + canRemoveNullCheck = false; + } + else + { + curStmt = curStmt->GetPrevStmt(); + currentTree = curStmt->GetRootNode(); } } + *nullCheckStmt = curStmt; } + + if (canRemoveNullCheck && (*nullCheckParent == nullptr)) + { + *nullCheckParent = nullCheckTree->gtGetParent(nullptr); + } + + return canRemoveNullCheck; } //---------------------------------------------------------------- -// optCanMoveNullCheckPastTree: Check if GT_NULLCHECK can be folded into a node that -// is after tree is execution order. +// optCanMoveNullCheckPastTree: Check if a nullcheck node that is before `tree` +// in execution order may be folded into an indirection node that +// is after `tree` is execution order. // // Arguments: -// tree - The input GT_INDIR tree. -// isInsideTry - True if tree is inside try, false otherwise +// tree - The tree to check. +// nullCheckLclNum - The local variable that GT_NULLCHECK checks. +// isInsideTry - True if tree is inside try, false otherwise. +// checkSideEffectSummary -If true, check side effect summary flags only, +// otherwise check the side effects of the operation itself. // // Return Value: -// True if GT_NULLCHECK can be folded into a node that is after tree is execution order, +// True if nullcheck may be folded into a node that is after tree in execution order, // false otherwise. -bool Compiler::optCanMoveNullCheckPastTree(GenTree* tree, bool isInsideTry) +bool Compiler::optCanMoveNullCheckPastTree(GenTree* tree, + unsigned nullCheckLclNum, + bool isInsideTry, + bool checkSideEffectSummary) { bool result = true; - if (isInsideTry) + + if ((tree->gtFlags & GTF_CALL) != 0) { - // We disallow calls, exception sources, and all assignments. - // Assignments to locals are disallowed inside try because - // they may be live in the handler. - if ((tree->gtFlags & GTF_SIDE_EFFECT) != 0) - { - result = false; - } + result = !checkSideEffectSummary && !tree->OperRequiresCallFlag(this); } - else + + if (result && (tree->gtFlags & GTF_EXCEPT) != 0) + { + result = !checkSideEffectSummary && !tree->OperMayThrow(this); + } + + if (result && ((tree->gtFlags & GTF_ASG) != 0)) { - // We disallow calls, exception sources, and assignments to - // global memory. - if (GTF_GLOBALLY_VISIBLE_SIDE_EFFECTS(tree->gtFlags)) + if (tree->OperGet() == GT_ASG) { - result = false; + GenTree* lhs = tree->gtGetOp1(); + GenTree* rhs = tree->gtGetOp2(); + if (checkSideEffectSummary && ((rhs->gtFlags & GTF_ASG) != 0)) + { + result = false; + } + else if (isInsideTry) + { + // Inside try we allow only assignments to locals not live in handlers. + // lvVolatileHint is set to true on variables that are line in handlers. + result = (lhs->OperGet() == GT_LCL_VAR) && !lvaTable[lhs->AsLclVarCommon()->GetLclNum()].lvVolatileHint; + } + else + { + // We disallow only assignments to global memory. + result = ((lhs->gtFlags & GTF_GLOB_REF) == 0); + } + } + else if (checkSideEffectSummary) + { + result = !isInsideTry && ((tree->gtFlags & GTF_GLOB_REF) == 0); + } + else + { + result = !isInsideTry && (!tree->OperRequiresAsgFlag() || ((tree->gtFlags & GTF_GLOB_REF) == 0)); } } + return result; } diff --git a/src/coreclr/src/jit/importer.cpp b/src/coreclr/src/jit/importer.cpp index 132cf2bf082e47..c526e7bfb591a1 100644 --- a/src/coreclr/src/jit/importer.cpp +++ b/src/coreclr/src/jit/importer.cpp @@ -5891,7 +5891,9 @@ int Compiler::impBoxPatternMatch(CORINFO_RESOLVED_TOKEN* pResolvedToken, const B if (treeToNullcheck != nullptr) { GenTree* nullcheck = gtNewOperNode(GT_NULLCHECK, TYP_I_IMPL, treeToNullcheck); - result = gtNewOperNode(GT_COMMA, TYP_INT, nullcheck, result); + compCurBB->bbFlags |= BBF_HAS_NULLCHECK; + optMethodFlags |= OMF_HAS_NULLCHECK; + result = gtNewOperNode(GT_COMMA, TYP_INT, nullcheck, result); } impPushOnStack(result, typeInfo(TI_INT)); @@ -13017,6 +13019,8 @@ void Compiler::impImportBlockCode(BasicBlock* block) if (op1->OperIs(GT_FIELD, GT_IND, GT_OBJ)) { op1->ChangeOper(GT_NULLCHECK); + block->bbFlags |= BBF_HAS_NULLCHECK; + optMethodFlags |= OMF_HAS_NULLCHECK; op1->gtType = TYP_BYTE; } else @@ -15229,7 +15233,9 @@ void Compiler::impImportBlockCode(BasicBlock* block) GenTree* boxPayloadAddress = gtNewOperNode(GT_ADD, TYP_BYREF, cloneOperand, boxPayloadOffset); GenTree* nullcheck = gtNewOperNode(GT_NULLCHECK, TYP_I_IMPL, op1); - GenTree* result = gtNewOperNode(GT_COMMA, TYP_BYREF, nullcheck, boxPayloadAddress); + block->bbFlags |= BBF_HAS_NULLCHECK; + optMethodFlags |= OMF_HAS_NULLCHECK; + GenTree* result = gtNewOperNode(GT_COMMA, TYP_BYREF, nullcheck, boxPayloadAddress); impPushOnStack(result, tiRetVal); break; } diff --git a/src/coreclr/src/jit/morph.cpp b/src/coreclr/src/jit/morph.cpp index 075efa426f9023..c271c40450c0ca 100644 --- a/src/coreclr/src/jit/morph.cpp +++ b/src/coreclr/src/jit/morph.cpp @@ -12560,38 +12560,47 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) if (op2->IsCnsIntOrI() && varTypeIsIntegralOrI(typ)) { - /* Fold "((x+icon1)+icon2) to (x+(icon1+icon2))" */ + // Fold "((x+icon1)+icon2) to (x+(icon1+icon2))" + // Fold "((comma(y, x+icon1)+icon2) to comma(y, x+(icon1+icon2))" CLANG_FORMAT_COMMENT_ANCHOR; - if (op1->gtOper == GT_ADD && // - !gtIsActiveCSE_Candidate(op1) && // - !op1->gtOverflow() && // - op1->AsOp()->gtOp2->IsCnsIntOrI() && // - (op1->AsOp()->gtOp2->OperGet() == op2->OperGet()) && // - (op1->AsOp()->gtOp2->TypeGet() != TYP_REF) && // Don't fold REFs - (op2->TypeGet() != TYP_REF)) // Don't fold REFs + const bool commasOnly = true; + GenTree* op1EffectiveValue = op1->gtEffectiveVal(commasOnly); + + if (op1EffectiveValue->gtOper == GT_ADD && !gtIsActiveCSE_Candidate(op1EffectiveValue) && + !op1EffectiveValue->gtOverflow() && op1EffectiveValue->AsOp()->gtOp2->IsCnsIntOrI() && + (op1EffectiveValue->AsOp()->gtOp2->OperGet() == op2->OperGet()) && + (op1EffectiveValue->AsOp()->gtOp2->TypeGet() != TYP_REF) && (op2->TypeGet() != TYP_REF)) { - cns1 = op1->AsOp()->gtOp2; - op2->AsIntConCommon()->SetIconValue(cns1->AsIntConCommon()->IconValue() + - op2->AsIntConCommon()->IconValue()); + cns1 = op1EffectiveValue->AsOp()->gtOp2; + + cns1->AsIntConCommon()->SetIconValue(cns1->AsIntConCommon()->IconValue() + + op2->AsIntConCommon()->IconValue()); #ifdef _TARGET_64BIT_ - if (op2->TypeGet() == TYP_INT) + if (cns1->TypeGet() == TYP_INT) { // we need to properly re-sign-extend or truncate after adding two int constants above - op2->AsIntCon()->TruncateOrSignExtend32(); + cns1->AsIntCon()->TruncateOrSignExtend32(); } #endif //_TARGET_64BIT_ if (cns1->OperGet() == GT_CNS_INT) { - op2->AsIntCon()->gtFieldSeq = + cns1->AsIntCon()->gtFieldSeq = GetFieldSeqStore()->Append(cns1->AsIntCon()->gtFieldSeq, op2->AsIntCon()->gtFieldSeq); } - DEBUG_DESTROY_NODE(cns1); + DEBUG_DESTROY_NODE(op2); - tree->AsOp()->gtOp1 = op1->AsOp()->gtOp1; - DEBUG_DESTROY_NODE(op1); - op1 = tree->AsOp()->gtOp1; + GenTree* oldTree = tree; + tree = tree->AsOp()->gtOp1; + op1 = tree->AsOp()->gtOp1; + op2 = tree->AsOp()->gtOp2; + DEBUG_DESTROY_NODE(oldTree); + + if (tree->OperGet() != GT_ADD) + { + return tree; + } } // Fold (x + 0).