diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 9a4be3c6e4e9e3..980bfcfb5dd97a 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -1307,6 +1307,17 @@ Compiler::AssertionDsc* Compiler::optGetAssertion(AssertionIndex assertIndex) return assertion; } +ValueNum Compiler::optConservativeNormalVN(GenTree* tree) +{ + if (optLocalAssertionProp) + { + return ValueNumStore::NoVN; + } + + assert(vnStore != nullptr); + return vnStore->VNConservativeNormalValue(tree->gtVNPair); +} + //------------------------------------------------------------------------ // optCastConstantSmall: Cast a constant to a small type. // diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 0c660d71b52eb2..0d09e2652bc381 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -4260,13 +4260,6 @@ void CodeGen::genCheckUseBlockInit() continue; } - // Initialization of OSR locals must be handled specially - if (compiler->lvaIsOSRLocal(varNum)) - { - varDsc->lvMustInit = 0; - continue; - } - if (compiler->fgVarIsNeverZeroInitializedInProlog(varNum)) { varDsc->lvMustInit = 0; diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index d921aa458f6e10..906299515317a9 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -4746,29 +4746,31 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl // At this point we know if we are fully interruptible or not if (opts.OptimizationEnabled()) { - bool doSsa = true; - bool doEarlyProp = true; - bool doValueNum = true; - bool doLoopHoisting = true; - bool doCopyProp = true; - bool doBranchOpt = true; - bool doCse = true; - bool doAssertionProp = true; - bool doRangeAnalysis = true; - bool doIfConversion = true; - int iterations = 1; + bool doSsa = true; + bool doEarlyProp = true; + bool doValueNum = true; + bool doLoopHoisting = true; + bool doCopyProp = true; + bool doBranchOpt = true; + bool doCse = true; + bool doAssertionProp = true; + bool doRangeAnalysis = true; + bool doIfConversion = true; + bool doVNBasedDeadStoreRemoval = true; + int iterations = 1; #if defined(OPT_CONFIG) - doSsa = (JitConfig.JitDoSsa() != 0); - doEarlyProp = doSsa && (JitConfig.JitDoEarlyProp() != 0); - doValueNum = doSsa && (JitConfig.JitDoValueNumber() != 0); - doLoopHoisting = doValueNum && (JitConfig.JitDoLoopHoisting() != 0); - doCopyProp = doValueNum && (JitConfig.JitDoCopyProp() != 0); - doBranchOpt = doValueNum && (JitConfig.JitDoRedundantBranchOpts() != 0); - doCse = doValueNum; - doAssertionProp = doValueNum && (JitConfig.JitDoAssertionProp() != 0); - doRangeAnalysis = doAssertionProp && (JitConfig.JitDoRangeAnalysis() != 0); - doIfConversion = doIfConversion && (JitConfig.JitDoIfConversion() != 0); + doSsa = (JitConfig.JitDoSsa() != 0); + doEarlyProp = doSsa && (JitConfig.JitDoEarlyProp() != 0); + doValueNum = doSsa && (JitConfig.JitDoValueNumber() != 0); + doLoopHoisting = doValueNum && (JitConfig.JitDoLoopHoisting() != 0); + doCopyProp = doValueNum && (JitConfig.JitDoCopyProp() != 0); + doBranchOpt = doValueNum && (JitConfig.JitDoRedundantBranchOpts() != 0); + doCse = doValueNum; + doAssertionProp = doValueNum && (JitConfig.JitDoAssertionProp() != 0); + doRangeAnalysis = doAssertionProp && (JitConfig.JitDoRangeAnalysis() != 0); + doIfConversion = doIfConversion && (JitConfig.JitDoIfConversion() != 0); + doVNBasedDeadStoreRemoval = doValueNum && (JitConfig.JitDoVNBasedDeadStoreRemoval() != 0); if (opts.optRepeat) { @@ -4864,6 +4866,13 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl DoPhase(this, PHASE_OPTIMIZE_INDEX_CHECKS, &Compiler::rangeCheckPhase); } + if (doVNBasedDeadStoreRemoval) + { + // Note: this invalidates SSA and value numbers on tree nodes. + // + DoPhase(this, PHASE_VN_BASED_DEAD_STORE_REMOVAL, &Compiler::optVNBasedDeadStoreRemoval); + } + if (fgModified) { // update the flowgraph if we modified it during the optimization phase @@ -9294,7 +9303,6 @@ void cTreeFlags(Compiler* comp, GenTree* tree) genTreeOps op = tree->OperGet(); switch (op) { - case GT_LCL_VAR: case GT_LCL_VAR_ADDR: case GT_LCL_FLD: diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 193399040296a3..301b58cff05604 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -6054,10 +6054,6 @@ class Compiler // Mark a loop as removed. void optMarkLoopRemoved(unsigned loopNum); - // During global assertion prop, returns the conservative normal VN for a tree; - // otherwise returns NoVN - ValueNum optConservativeNormalVN(GenTree* tree); - private: // Requires "lnum" to be the index of an outermost loop in the loop table. Traverses the body of that loop, // including all nested loops, and records the set of "side effects" of the loop: fields (object instance and @@ -6832,6 +6828,8 @@ class Compiler } }; + PhaseStatus optVNBasedDeadStoreRemoval(); + // clang-format off #define OMF_HAS_NEWARRAY 0x00000001 // Method contains 'new' of an SD array @@ -7333,6 +7331,10 @@ class Compiler AssertionIndex optFindComplementary(AssertionIndex assertionIndex); void optMapComplementary(AssertionIndex assertionIndex, AssertionIndex index); + ValueNum optConservativeNormalVN(GenTree* tree); + + ssize_t optCastConstantSmall(ssize_t iconVal, var_types smallType); + // Assertion creation functions. AssertionIndex optCreateAssertion(GenTree* op1, GenTree* op2, @@ -7340,7 +7342,6 @@ class Compiler bool helperCallArgs = false); AssertionIndex optFinalizeCreatingAssertion(AssertionDsc* assertion); - ssize_t optCastConstantSmall(ssize_t iconVal, var_types smallType); bool optTryExtractSubrangeAssertion(GenTree* source, IntegralRange* pRange); diff --git a/src/coreclr/jit/compphases.h b/src/coreclr/jit/compphases.h index fd040f3acd03e7..2fb14b058a3121 100644 --- a/src/coreclr/jit/compphases.h +++ b/src/coreclr/jit/compphases.h @@ -88,6 +88,7 @@ CompPhaseNameMacro(PHASE_VN_COPY_PROP, "VN based copy prop", CompPhaseNameMacro(PHASE_OPTIMIZE_BRANCHES, "Redundant branch opts", false, -1, false) CompPhaseNameMacro(PHASE_ASSERTION_PROP_MAIN, "Assertion prop", false, -1, false) CompPhaseNameMacro(PHASE_IF_CONVERSION, "If conversion", false, -1, false) +CompPhaseNameMacro(PHASE_VN_BASED_DEAD_STORE_REMOVAL,"VN-based dead store removal", false, -1, false) CompPhaseNameMacro(PHASE_OPT_UPDATE_FLOW_GRAPH, "Update flow graph opt pass", false, -1, false) CompPhaseNameMacro(PHASE_COMPUTE_EDGE_WEIGHTS2, "Compute edge weights (2, false)",false, -1, false) CompPhaseNameMacro(PHASE_INSERT_GC_POLLS, "Insert GC Polls", false, -1, true) diff --git a/src/coreclr/jit/jitconfigvalues.h b/src/coreclr/jit/jitconfigvalues.h index 2f337078a71de7..1ade1a3009d952 100644 --- a/src/coreclr/jit/jitconfigvalues.h +++ b/src/coreclr/jit/jitconfigvalues.h @@ -419,9 +419,12 @@ CONFIG_INTEGER(JitDoEarlyProp, W("JitDoEarlyProp"), 1) // Perform Early Value Pr CONFIG_INTEGER(JitDoLoopHoisting, W("JitDoLoopHoisting"), 1) // Perform loop hoisting on loop invariant values CONFIG_INTEGER(JitDoLoopInversion, W("JitDoLoopInversion"), 1) // Perform loop inversion on "for/while" loops CONFIG_INTEGER(JitDoRangeAnalysis, W("JitDoRangeAnalysis"), 1) // Perform range check analysis +CONFIG_INTEGER(JitDoVNBasedDeadStoreRemoval, W("JitDoVNBasedDeadStoreRemoval"), 1) // Perform VN-based dead store + // removal CONFIG_INTEGER(JitDoRedundantBranchOpts, W("JitDoRedundantBranchOpts"), 1) // Perform redundant branch optimizations CONFIG_STRING(JitEnableRboRange, W("JitEnableRboRange")) CONFIG_STRING(JitEnableTailMergeRange, W("JitEnableTailMergeRange")) +CONFIG_STRING(JitEnableVNBasedDeadStoreRemovalRange, W("JitEnableVNBasedDeadStoreRemovalRange")) CONFIG_INTEGER(JitDoSsa, W("JitDoSsa"), 1) // Perform Static Single Assignment (SSA) numbering on the variables CONFIG_INTEGER(JitDoValueNumber, W("JitDoValueNumber"), 1) // Perform value numbering on method expressions diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index bb4c3377069273..c8fd45aed2bc66 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -10689,6 +10689,104 @@ void Compiler::optRemoveRedundantZeroInits() } } +//------------------------------------------------------------------------ +// optVNBasedDeadStoreRemoval: VN(value)-based dead store removal. +// +// The phase iterates over partial stores referenced by the SSA +// descriptors and deletes those which do not change the local's value. +// +// Return Value: +// A suitable phase status. +// +PhaseStatus Compiler::optVNBasedDeadStoreRemoval() +{ +#ifdef DEBUG + static ConfigMethodRange JitEnableVNBasedDeadStoreRemovalRange; + JitEnableVNBasedDeadStoreRemovalRange.EnsureInit(JitConfig.JitEnableVNBasedDeadStoreRemovalRange()); + + if (!JitEnableVNBasedDeadStoreRemovalRange.Contains(info.compMethodHash())) + { + JITDUMP("VN-based dead store removal disabled by JitEnableVNBasedDeadStoreRemovalRange\n"); + return PhaseStatus::MODIFIED_NOTHING; + } +#endif + + bool madeChanges = false; + + for (unsigned lclNum = 0; lclNum < lvaCount; lclNum++) + { + if (!lvaInSsa(lclNum)) + { + continue; + } + + LclVarDsc* varDsc = lvaGetDesc(lclNum); + unsigned defCount = varDsc->lvPerSsaData.GetCount(); + if (defCount <= 2) + { + continue; + } + + for (unsigned defIndex = 1; defIndex < defCount; defIndex++) + { + LclSsaVarDsc* defDsc = varDsc->lvPerSsaData.GetSsaDefByIndex(defIndex); + GenTreeOp* store = defDsc->GetAssignment(); + + if (store != nullptr) + { + assert(store->OperIs(GT_ASG) && defDsc->m_vnPair.BothDefined()); + + JITDUMP("Considering [%06u] for removal...\n", dspTreeID(store)); + + GenTree* lhs = store->gtGetOp1(); + if (!lhs->OperIs(GT_LCL_FLD) || ((lhs->gtFlags & GTF_VAR_USEASG) == 0) || + (lhs->AsLclFld()->GetLclNum() != lclNum)) + { + continue; + } + + ValueNum oldLclValue = varDsc->GetPerSsaData(defDsc->GetUseDefSsaNum())->m_vnPair.GetConservative(); + ValueNum oldStoreValue = + vnStore->VNForLoad(VNK_Conservative, oldLclValue, lvaLclExactSize(lclNum), lhs->TypeGet(), + lhs->AsLclFld()->GetLclOffs(), lhs->AsLclFld()->GetSize()); + + GenTree* rhs = store->gtGetOp2(); + ValueNum storeValue; + if (lhs->TypeIs(TYP_STRUCT) && rhs->IsIntegralConst(0)) + { + storeValue = vnStore->VNForZeroObj(lhs->AsLclFld()->GetLayout()); + } + else + { + storeValue = rhs->GetVN(VNK_Conservative); + } + + if (oldStoreValue == storeValue) + { + JITDUMP("Removed dead store:\n"); + DISPTREE(store); + + lhs->gtFlags &= ~(GTF_VAR_DEF | GTF_VAR_USEASG); + + store->ChangeOper(GT_COMMA); + if (store->IsReverseOp()) + { + std::swap(store->gtOp1, store->gtOp2); + store->ClearReverseOp(); + } + store->gtType = store->gtGetOp2()->TypeGet(); + store->SetAllEffectsFlags(store->gtOp1, store->gtOp2); + gtUpdateTreeAncestorsSideEffects(store); + + madeChanges = true; + } + } + } + } + + return madeChanges ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING; +} + #ifdef DEBUG //------------------------------------------------------------------------ @@ -10930,14 +11028,3 @@ void Compiler::optMarkLoopRemoved(unsigned loopNum) // `fgDebugCheckLoopTable()` is called. #endif // DEBUG } - -ValueNum Compiler::optConservativeNormalVN(GenTree* tree) -{ - if (optLocalAssertionProp) - { - return ValueNumStore::NoVN; - } - - assert(vnStore != nullptr); - return vnStore->VNConservativeNormalValue(tree->gtVNPair); -} diff --git a/src/coreclr/jit/rationalize.cpp b/src/coreclr/jit/rationalize.cpp index dad87564355ac0..0f2a32d95eb428 100644 --- a/src/coreclr/jit/rationalize.cpp +++ b/src/coreclr/jit/rationalize.cpp @@ -574,8 +574,12 @@ Compiler::fgWalkResult Rationalizer::RewriteNode(GenTree** useEdge, Compiler::Ge // args as these have now been sequenced. for (CallArg& arg : node->AsCall()->gtArgs.EarlyArgs()) { - if (!arg.GetEarlyNode()->IsValue()) + if (arg.GetLateNode() != nullptr) { + if (arg.GetEarlyNode()->IsValue()) + { + arg.GetEarlyNode()->SetUnusedValue(); + } arg.SetEarlyNode(nullptr); } } diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 283fb5f92f9dff..433d8753976a66 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -7581,18 +7581,9 @@ PhaseStatus Compiler::fgValueNumber() // The last clause covers the use-before-def variables (the ones that are live-in to the first block), // these are variables that are read before being initialized (at least on some control flow paths) // if they are not must-init, then they get VNF_InitVal(i), as with the param case.) - - bool isZeroed = (info.compInitMem || varDsc->lvMustInit); - - // For OSR, locals or promoted fields of locals may be missing the initial def - // because of partial importation. We can't assume they are zero. - if (lvaIsOSRLocal(lclNum)) - { - isZeroed = false; - } - - ValueNum initVal = ValueNumStore::NoVN; // We must assign a new value to initVal - var_types typ = varDsc->TypeGet(); + bool isZeroed = !fgVarNeedsExplicitZeroInit(lclNum, /* bbInALoop */ false, /* bbIsReturn */ false); + ValueNum initVal = ValueNumStore::NoVN; // We must assign a new value to initVal + var_types typ = varDsc->TypeGet(); switch (typ) {