diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 13ddd674096062..fb317faea42c13 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -2664,7 +2664,104 @@ AssertionIndex Compiler::optAssertionIsSubtype(GenTree* tree, GenTree* methodTab } //------------------------------------------------------------------------------ -// optVNConstantPropOnTree: Substitutes tree with an evaluated constant while +// optVNBasedFoldExpr_Call: Folds given call using VN to a simpler tree. +// +// Arguments: +// block - The block containing the tree. +// parent - The parent node of the tree. +// call - The call to fold +// +// Return Value: +// Returns a new tree or nullptr if nothing is changed. +// +GenTree* Compiler::optVNBasedFoldExpr_Call(BasicBlock* block, GenTree* parent, GenTreeCall* call) +{ + switch (call->GetHelperNum()) + { + // + // Fold "CAST(IsInstanceOf(obj, cls), cls)" to "IsInstanceOf(obj, cls)" + // where CAST is either ISINST or CASTCLASS. + // + case CORINFO_HELP_CHKCASTARRAY: + case CORINFO_HELP_CHKCASTANY: + case CORINFO_HELP_CHKCASTINTERFACE: + case CORINFO_HELP_CHKCASTCLASS: + case CORINFO_HELP_ISINSTANCEOFARRAY: + case CORINFO_HELP_ISINSTANCEOFCLASS: + case CORINFO_HELP_ISINSTANCEOFANY: + case CORINFO_HELP_ISINSTANCEOFINTERFACE: + { + GenTree* castClsArg = call->gtArgs.GetUserArgByIndex(0)->GetNode(); + GenTree* castObjArg = call->gtArgs.GetUserArgByIndex(1)->GetNode(); + ValueNum castClsArgVN = castClsArg->gtVNPair.GetConservative(); + ValueNum castObjArgVN = castObjArg->gtVNPair.GetConservative(); + + if ((castObjArg->gtFlags & GTF_ALL_EFFECT) != 0) + { + // It won't be trivial to properly extract side-effects from the call node. + // Ideally, we only need side effects from the castClsArg argument as the call itself + // won't throw any exceptions. But we should not forget about the EarlyNode (setup args) + return nullptr; + } + + VNFuncApp funcApp; + if (vnStore->GetVNFunc(castObjArgVN, &funcApp) && (funcApp.m_func == VNF_IsInstanceOf)) + { + ValueNum innerCastClsVN = funcApp.m_args[0]; + if (innerCastClsVN == castClsArgVN) + { + // The outer cast is redundant, remove it and preserve its side effects + // We do ignoreRoot here because the actual cast node never throws any exceptions. + GenTree* result = gtWrapWithSideEffects(castObjArg, call, GTF_ALL_EFFECT, true); + fgSetTreeSeq(result); + return result; + } + } + } + break; + + default: + break; + } + + return nullptr; +} + +//------------------------------------------------------------------------------ +// optVNBasedFoldExpr: Folds given tree using VN to a constant or a simpler tree. +// +// Arguments: +// block - The block containing the tree. +// parent - The parent node of the tree. +// tree - The tree to fold. +// +// Return Value: +// Returns a new tree or nullptr if nothing is changed. +// +GenTree* Compiler::optVNBasedFoldExpr(BasicBlock* block, GenTree* parent, GenTree* tree) +{ + // First, attempt to fold it to a constant if possible. + GenTree* foldedToCns = optVNBasedFoldConstExpr(block, parent, tree); + if (foldedToCns != nullptr) + { + return foldedToCns; + } + + switch (tree->OperGet()) + { + case GT_CALL: + return optVNBasedFoldExpr_Call(block, parent, tree->AsCall()); + + // We can add more VN-based foldings here. + + default: + break; + } + return nullptr; +} + +//------------------------------------------------------------------------------ +// optVNBasedFoldConstExpr: Substitutes tree with an evaluated constant while // managing side-effects. // // Arguments: @@ -2691,7 +2788,7 @@ AssertionIndex Compiler::optAssertionIsSubtype(GenTree* tree, GenTree* methodTab // the relop will evaluate to "true" or "false" statically, then the side-effects // will be put into new statements, presuming the JTrue will be folded away. // -GenTree* Compiler::optVNConstantPropOnTree(BasicBlock* block, GenTree* parent, GenTree* tree) +GenTree* Compiler::optVNBasedFoldConstExpr(BasicBlock* block, GenTree* parent, GenTree* tree) { if (tree->OperGet() == GT_JTRUE) { @@ -5109,21 +5206,10 @@ GenTree* Compiler::optAssertionProp_Call(ASSERT_VALARG_TP assertions, GenTreeCal unsigned index = optAssertionIsSubtype(arg1, arg2, assertions); if (index != NO_ASSERTION_INDEX) { -#ifdef DEBUG - if (verbose) - { - printf("\nDid VN based subtype prop for index #%02u in " FMT_BB ":\n", index, compCurBB->bbNum); - gtDispTree(call, nullptr, nullptr, true); - } -#endif - GenTree* list = nullptr; - gtExtractSideEffList(call, &list, GTF_SIDE_EFFECT, true); - if (list != nullptr) - { - arg1 = gtNewOperNode(GT_COMMA, call->TypeGet(), list, arg1); - fgSetTreeSeq(arg1); - } + JITDUMP("\nDid VN based subtype prop for index #%02u in " FMT_BB ":\n", index, compCurBB->bbNum); + DISPTREE(call); + arg1 = gtWrapWithSideEffects(arg1, call, GTF_SIDE_EFFECT, true); return optAssertionProp_Update(arg1, call, stmt); } @@ -6301,8 +6387,8 @@ GenTree* Compiler::optVNConstantPropOnJTrue(BasicBlock* block, GenTree* test) } //------------------------------------------------------------------------------ -// optVNConstantPropCurStmt -// Performs constant prop on the current statement's tree nodes. +// optVNBasedFoldCurStmt: Performs VN-based folding +// on the current statement's tree nodes using VN. // // Assumption: // This function is called as part of a post-order tree walk. @@ -6316,17 +6402,12 @@ GenTree* Compiler::optVNConstantPropOnJTrue(BasicBlock* block, GenTree* test) // Return Value: // Returns the standard visitor walk result. // -// Description: -// Checks if a node is an R-value and evaluates to a constant. If the node -// evaluates to constant, then the tree is replaced by its side effects and -// the constant node. -// -Compiler::fgWalkResult Compiler::optVNConstantPropCurStmt(BasicBlock* block, - Statement* stmt, - GenTree* parent, - GenTree* tree) +Compiler::fgWalkResult Compiler::optVNBasedFoldCurStmt(BasicBlock* block, + Statement* stmt, + GenTree* parent, + GenTree* tree) { - // Don't perform const prop on expressions marked with GTF_DONT_CSE + // Don't try and fold expressions marked with GTF_DONT_CSE // TODO-ASG: delete. if (!tree->CanCSE()) { @@ -6414,8 +6495,8 @@ Compiler::fgWalkResult Compiler::optVNConstantPropCurStmt(BasicBlock* block, return WALK_CONTINUE; } - // Perform the constant propagation - GenTree* newTree = optVNConstantPropOnTree(block, parent, tree); + // Perform the VN-based folding: + GenTree* newTree = optVNBasedFoldExpr(block, parent, tree); if (newTree == nullptr) { @@ -6428,7 +6509,7 @@ Compiler::fgWalkResult Compiler::optVNConstantPropCurStmt(BasicBlock* block, optAssertionProp_Update(newTree, tree, stmt); - JITDUMP("After constant propagation on [%06u]:\n", tree->gtTreeID); + JITDUMP("After VN-based fold of [%06u]:\n", tree->gtTreeID); DBEXEC(VERBOSE, gtDispStmt(stmt)); return WALK_CONTINUE; @@ -6496,7 +6577,7 @@ Compiler::fgWalkResult Compiler::optVNAssertionPropCurStmtVisitor(GenTree** ppTr pThis->optVnNonNullPropCurStmt(pData->block, pData->stmt, *ppTree); - return pThis->optVNConstantPropCurStmt(pData->block, pData->stmt, data->parent, *ppTree); + return pThis->optVNBasedFoldCurStmt(pData->block, pData->stmt, data->parent, *ppTree); } /***************************************************************************** diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 1d4dbcdb82aa8a..a4efc3e0c61b62 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -3515,6 +3515,11 @@ class Compiler GenTreeFlags GenTreeFlags = GTF_SIDE_EFFECT, bool ignoreRoot = false); + GenTree* gtWrapWithSideEffects(GenTree* tree, + GenTree* sideEffectsSource, + GenTreeFlags sideEffectsFlags = GTF_SIDE_EFFECT, + bool ignoreRoot = false); + bool gtSplitTree( BasicBlock* block, Statement* stmt, GenTree* splitPoint, Statement** firstNewStmt, GenTree*** splitPointUse); @@ -7710,9 +7715,11 @@ class Compiler public: void optVnNonNullPropCurStmt(BasicBlock* block, Statement* stmt, GenTree* tree); - fgWalkResult optVNConstantPropCurStmt(BasicBlock* block, Statement* stmt, GenTree* parent, GenTree* tree); + fgWalkResult optVNBasedFoldCurStmt(BasicBlock* block, Statement* stmt, GenTree* parent, GenTree* tree); GenTree* optVNConstantPropOnJTrue(BasicBlock* block, GenTree* test); - GenTree* optVNConstantPropOnTree(BasicBlock* block, GenTree* parent, GenTree* tree); + GenTree* optVNBasedFoldConstExpr(BasicBlock* block, GenTree* parent, GenTree* tree); + GenTree* optVNBasedFoldExpr(BasicBlock* block, GenTree* parent, GenTree* tree); + GenTree* optVNBasedFoldExpr_Call(BasicBlock* block, GenTree* parent, GenTreeCall* call); GenTree* optExtractSideEffListFromConst(GenTree* tree); AssertionIndex GetAssertionCount() diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 58c28f322f3759..22336da17625de 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -17063,6 +17063,41 @@ bool Compiler::gtSplitTree( return splitter.MadeChanges; } +//------------------------------------------------------------------------ +// gtWrapWithSideEffects: Extracts side effects from sideEffectSource (if any) +// and wraps the input tree with a COMMA node with them. +// +// Arguments: +// tree - the expression tree to wrap with side effects (if any) +// it has to be either a side effect free subnode of sideEffectsSource +// or any tree outside sideEffectsSource's hierarchy +// sideEffectsSource - the expression tree to extract side effects from +// sideEffectsFlags - side effect flags to be considered +// ignoreRoot - ignore side effects on the expression root node +// +// Return Value: +// The original tree wrapped with a COMMA node that contains the side effects +// or just the tree itself if sideEffectSource has no side effects. +// +GenTree* Compiler::gtWrapWithSideEffects(GenTree* tree, + GenTree* sideEffectsSource, + GenTreeFlags sideEffectsFlags, + bool ignoreRoot) +{ + GenTree* sideEffects = nullptr; + gtExtractSideEffList(sideEffectsSource, &sideEffects, sideEffectsFlags, ignoreRoot); + if (sideEffects != nullptr) + { + // TODO: assert if tree is a subnode of sideEffectsSource and the tree has its own side effects + // otherwise the resulting COMMA might have some side effects to be duplicated + // It should be possible to be smarter here and allow such cases by extracting the side effects + // properly for this particular case. For now, caller is responsible for avoiding such cases. + + tree = gtNewOperNode(GT_COMMA, tree->TypeGet(), sideEffects, tree); + } + return tree; +} + //------------------------------------------------------------------------ // gtExtractSideEffList: Extracts side effects from the given expression. // diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 2e79a604390203..b3d035b26d96bc 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -1677,8 +1677,7 @@ GenTree* Compiler::impRuntimeLookupToTree(CORINFO_RESOLVED_TOKEN* pResolvedToken return slotPtrTree; } - slotPtrTree = gtNewIndir(TYP_I_IMPL, slotPtrTree, GTF_IND_NONFAULTING); - slotPtrTree->gtFlags &= ~GTF_GLOB_REF; // TODO-Bug?: this is a quirk. Can we mark this indirection invariant? + slotPtrTree = gtNewIndir(TYP_I_IMPL, slotPtrTree, GTF_IND_NONFAULTING | GTF_IND_INVARIANT); return slotPtrTree; } diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 312fc8300966a6..ebf5244dedc43f 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -9653,22 +9653,8 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA } else { - GenTree* op1SideEffects = nullptr; - gtExtractSideEffList(op1, &op1SideEffects, GTF_ALL_EFFECT); - if (op1SideEffects != nullptr) - { - DEBUG_DESTROY_NODE(tree); - // Keep side-effects of op1 - tree = gtNewOperNode(GT_COMMA, TYP_INT, op1SideEffects, gtNewIconNode(0)); - JITDUMP("false with side effects:\n") - DISPTREE(tree); - } - else - { - JITDUMP("false\n"); - DEBUG_DESTROY_NODE(tree, op1); - tree = gtNewIconNode(0); - } + JITDUMP("false\n"); + tree = gtWrapWithSideEffects(gtNewIconNode(0), op1, GTF_ALL_EFFECT); } INDEBUG(tree->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); return tree; diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 8bb7c308207aff..977e984865aac7 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -13098,6 +13098,13 @@ bool Compiler::fgValueNumberHelperCall(GenTreeCall* call) vnStore->VNPairForFunc(TYP_REF, VNF_OverflowExc, vnStore->VNPForVoid())); break; + case CORINFO_HELP_CHKCASTINTERFACE: + case CORINFO_HELP_CHKCASTARRAY: + case CORINFO_HELP_CHKCASTCLASS: + case CORINFO_HELP_CHKCASTANY: + // InvalidCastExc for these is set in VNForCast + break; + default: // Setup vnpExc with the information that multiple different exceptions // could be generated by this helper