diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index dd92e05f464af..bea5ca8507652 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -6785,6 +6785,8 @@ class Compiler PhaseStatus fgMarkAddressExposedLocals(); void fgSequenceLocals(Statement* stmt); + bool fgExposeUnpropagatedLocals(bool propagatedAny, class LocalEqualsLocalAddrAssertions* assertions); + void fgExposeLocalsInBitVec(BitVec_ValArg_T bitVec); PhaseStatus PhysicalPromotion(); diff --git a/src/coreclr/jit/jitconfigvalues.h b/src/coreclr/jit/jitconfigvalues.h index e354e3421acbe..dbd56791d88d8 100644 --- a/src/coreclr/jit/jitconfigvalues.h +++ b/src/coreclr/jit/jitconfigvalues.h @@ -567,6 +567,7 @@ OPT_CONFIG_STRING(JitOnlyOptimizeRange, OPT_CONFIG_STRING(JitEnablePhysicalPromotionRange, W("JitEnablePhysicalPromotionRange")) OPT_CONFIG_STRING(JitEnableCrossBlockLocalAssertionPropRange, W("JitEnableCrossBlockLocalAssertionPropRange")) OPT_CONFIG_STRING(JitEnableInductionVariableOptsRange, W("JitEnableInductionVariableOptsRange")) +OPT_CONFIG_STRING(JitEnableLocalAddrPropagationRange, W("JitEnableLocalAddrPropagationRange")) OPT_CONFIG_INTEGER(JitDoSsa, W("JitDoSsa"), 1) // Perform Static Single Assignment (SSA) numbering on the variables OPT_CONFIG_INTEGER(JitDoValueNumber, W("JitDoValueNumber"), 1) // Perform value numbering on method expressions diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp index 6a4534fc3f63a..b8ecc8b1f736f 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/src/coreclr/jit/lclmorph.cpp @@ -175,6 +175,277 @@ void Compiler::fgSequenceLocals(Statement* stmt) seq.Sequence(stmt); } +struct LocalEqualsLocalAddrAssertion +{ + // Local num on the LHS + unsigned DestLclNum; + // Local num on the RHS (having its adress taken) + unsigned AddressLclNum; + // Offset into RHS local + unsigned AddressOffset; + + LocalEqualsLocalAddrAssertion(unsigned destLclNum, unsigned addressLclNum, unsigned addressOffset) + : DestLclNum(destLclNum) + , AddressLclNum(addressLclNum) + , AddressOffset(addressOffset) + { + } + +#ifdef DEBUG + void Print() const + { + printf("V%02u = &V%02u[+%03u]", DestLclNum, AddressLclNum, AddressOffset); + } +#endif +}; + +struct AssertionKeyFuncs +{ + static bool Equals(const LocalEqualsLocalAddrAssertion& lhs, const LocalEqualsLocalAddrAssertion rhs) + { + return (lhs.DestLclNum == rhs.DestLclNum) && (lhs.AddressLclNum == rhs.AddressLclNum) && + (lhs.AddressOffset == rhs.AddressOffset); + } + + static unsigned GetHashCode(const LocalEqualsLocalAddrAssertion& val) + { + unsigned hash = val.DestLclNum; + hash ^= val.AddressLclNum + 0x9e3779b9 + (hash << 19) + (hash >> 13); + hash ^= val.AddressOffset + 0x9e3779b9 + (hash << 19) + (hash >> 13); + return hash; + } +}; + +typedef JitHashTable AssertionToIndexMap; + +class LocalEqualsLocalAddrAssertions +{ + Compiler* m_comp; + ArrayStack m_assertions; + AssertionToIndexMap m_map; + uint64_t* m_lclAssertions; + uint64_t* m_outgoingAssertions; + uint64_t m_currentAssertions = 0; + BitVec m_localsToExpose; + +public: + LocalEqualsLocalAddrAssertions(Compiler* comp) + : m_comp(comp) + , m_assertions(comp->getAllocator(CMK_LocalAddressVisitor)) + , m_map(comp->getAllocator(CMK_LocalAddressVisitor)) + { + m_lclAssertions = + comp->lvaCount == 0 ? nullptr : new (comp, CMK_LocalAddressVisitor) uint64_t[comp->lvaCount]{}; + m_outgoingAssertions = new (comp, CMK_LocalAddressVisitor) uint64_t[comp->m_dfsTree->GetPostOrderCount()]{}; + + BitVecTraits localsTraits(comp->lvaCount, comp); + m_localsToExpose = BitVecOps::MakeEmpty(&localsTraits); + } + + //------------------------------------------------------------------------ + // GetLocalsToExpose: Get the bit vector of locals that were marked to be + // exposed. + // + BitVec_ValRet_T GetLocalsToExpose() + { + return m_localsToExpose; + } + + //------------------------------------------------------------------------ + // IsMarkedForExposure: Check if a specific local is marked to be exposed. + // + // Return Value: + // True if so. + // + bool IsMarkedForExposure(unsigned lclNum) + { + BitVecTraits traits(m_comp->lvaCount, m_comp); + if (BitVecOps::IsMember(&traits, m_localsToExpose, lclNum)) + { + return true; + } + + LclVarDsc* dsc = m_comp->lvaGetDesc(lclNum); + if (dsc->lvIsStructField && BitVecOps::IsMember(&traits, m_localsToExpose, dsc->lvParentLcl)) + { + return true; + } + + return false; + } + + //------------------------------------------------------------------- + // StartBlock: Start a new block by computing incoming assertions for the + // block. + // + // Arguments: + // block - The block + // + void StartBlock(BasicBlock* block) + { + if ((m_assertions.Height() == 0) || (block->bbPreds == nullptr) || m_comp->bbIsHandlerBeg(block)) + { + m_currentAssertions = 0; + return; + } + + m_currentAssertions = UINT64_MAX; + for (BasicBlock* pred : block->PredBlocks()) + { + assert(m_comp->m_dfsTree->Contains(pred)); + if (pred->bbPostorderNum <= block->bbPostorderNum) + { + m_currentAssertions = 0; + break; + } + + m_currentAssertions &= m_outgoingAssertions[pred->bbPostorderNum]; + } + +#ifdef DEBUG + if (m_currentAssertions != 0) + { + JITDUMP(FMT_BB " incoming assertions:\n", block->bbNum); + uint64_t assertions = m_currentAssertions; + do + { + uint32_t index = BitOperations::BitScanForward(assertions); + JITDUMP(" A%02u: ", index); + DBEXEC(VERBOSE, m_assertions.BottomRef((int)index).Print()); + JITDUMP("\n"); + + assertions ^= uint64_t(1) << index; + } while (assertions != 0); + } +#endif + } + + //------------------------------------------------------------------- + // EndBlock: End a block by recording its final outgoing assertions. + // + // Arguments: + // block - The block + // + void EndBlock(BasicBlock* block) + { + m_outgoingAssertions[block->bbPostorderNum] = m_currentAssertions; + } + + //------------------------------------------------------------------- + // OnExposed: Mark that a local is having its address taken. + // + // Arguments: + // lclNum - The local + // + void OnExposed(unsigned lclNum) + { + BitVecTraits localsTraits(m_comp->lvaCount, m_comp); + BitVecOps::AddElemD(&localsTraits, m_localsToExpose, lclNum); + } + + //------------------------------------------------------------------- + // Record: Record an assertion about the specified local. + // + // Arguments: + // dstLclNum - Destination local + // srcLclNum - Local having its address taken + // srcOffs - Offset into the source local of the address being taken + // + void Record(unsigned dstLclNum, unsigned srcLclNum, unsigned srcOffs) + { + LocalEqualsLocalAddrAssertion assertion(dstLclNum, srcLclNum, srcOffs); + + unsigned index; + if (m_assertions.Height() >= 64) + { + if (!m_map.Lookup(assertion, &index)) + { + JITDUMP("Out of assertion space; dropping assertion "); + DBEXEC(VERBOSE, assertion.Print()); + JITDUMP("\n"); + return; + } + } + else + { + unsigned* pIndex = m_map.LookupPointerOrAdd(assertion, UINT_MAX); + if (*pIndex == UINT_MAX) + { + index = (unsigned)m_assertions.Height(); + *pIndex = index; + m_assertions.Push(assertion); + m_lclAssertions[dstLclNum] |= uint64_t(1) << index; + + JITDUMP("Adding new assertion A%02u ", index); + DBEXEC(VERBOSE, assertion.Print()); + JITDUMP("\n"); + } + else + { + index = *pIndex; + JITDUMP("Adding existing assertion A%02u ", index); + DBEXEC(VERBOSE, assertion.Print()); + JITDUMP("\n"); + } + } + + m_currentAssertions |= uint64_t(1) << index; + } + + //------------------------------------------------------------------- + // Clear: Clear active assertions about the specified local. + // + // Arguments: + // dstLclNum - Destination local + // + void Clear(unsigned dstLclNum) + { + m_currentAssertions &= ~m_lclAssertions[dstLclNum]; + } + + //----------------------------------------------------------------------------------- + // GetCurrentAssertion: + // Get the current assertion about a local's value. + // + // Arguments: + // lclNum - The local + // + // Return Value: + // Assertion, or nullptr if there is no current assertion. + // + const LocalEqualsLocalAddrAssertion* GetCurrentAssertion(unsigned lclNum) + { + uint64_t curAssertion = m_currentAssertions & m_lclAssertions[lclNum]; + assert(genMaxOneBit(curAssertion)); + if (curAssertion == 0) + { + return nullptr; + } + + return &m_assertions.BottomRef(BitOperations::BitScanForward(curAssertion)); + } + + //----------------------------------------------------------------------------------- + // GetLocalsWithAssertions: + // Get a bit vector of all locals that have assertions about their value. + // + // Return Value: + // Bit vector of locals. + // + BitVec_ValRet_T GetLocalsWithAssertions() + { + BitVecTraits localsTraits(m_comp->lvaCount, m_comp); + BitVec result(BitVecOps::MakeEmpty(&localsTraits)); + + for (int i = 0; i < m_assertions.Height(); i++) + { + BitVecOps::AddElemD(&localsTraits, result, m_assertions.BottomRef(i).DestLclNum); + } + + return result; + } +}; + class LocalAddressVisitor final : public GenTreeVisitor { // During tree traversal every GenTree node produces a "value" that represents: @@ -259,6 +530,13 @@ class LocalAddressVisitor final : public GenTreeVisitor m_offset = lclAddr->GetLclOffs(); } + void Address(unsigned lclNum, unsigned lclOffs) + { + assert(IsUnknown()); + m_lclNum = lclNum; + m_offset = lclOffs; + } + //------------------------------------------------------------------------ // AddOffset: Produce an address value from an address value. // @@ -326,10 +604,12 @@ class LocalAddressVisitor final : public GenTreeVisitor LclFld }; - ArrayStack m_valueStack; - bool m_stmtModified; - bool m_madeChanges; - LocalSequencer* m_sequencer; + ArrayStack m_valueStack; + bool m_stmtModified = false; + bool m_madeChanges = false; + bool m_propagatedAddrs = false; + LocalSequencer* m_sequencer; + LocalEqualsLocalAddrAssertions* m_lclAddrAssertions; public: enum @@ -339,12 +619,11 @@ class LocalAddressVisitor final : public GenTreeVisitor UseExecutionOrder = true, }; - LocalAddressVisitor(Compiler* comp, LocalSequencer* sequencer) + LocalAddressVisitor(Compiler* comp, LocalSequencer* sequencer, LocalEqualsLocalAddrAssertions* assertions) : GenTreeVisitor(comp) , m_valueStack(comp->getAllocator(CMK_LocalAddressVisitor)) - , m_stmtModified(false) - , m_madeChanges(false) , m_sequencer(sequencer) + , m_lclAddrAssertions(assertions) { } @@ -353,6 +632,11 @@ class LocalAddressVisitor final : public GenTreeVisitor return m_madeChanges; } + bool PropagatedAnyAddresses() const + { + return m_propagatedAddrs; + } + void VisitStmt(Statement* stmt) { #ifdef DEBUG @@ -407,6 +691,46 @@ class LocalAddressVisitor final : public GenTreeVisitor #endif // DEBUG } + void VisitBlock(BasicBlock* block) + { + // Make the current basic block address available globally + m_compiler->compCurBB = block; + + if (m_lclAddrAssertions != nullptr) + { + m_lclAddrAssertions->StartBlock(block); + } + + for (Statement* const stmt : block->Statements()) + { +#ifdef FEATURE_SIMD + if (m_compiler->opts.OptimizationEnabled() && stmt->GetRootNode()->TypeIs(TYP_FLOAT) && + stmt->GetRootNode()->OperIsStore()) + { + m_madeChanges |= m_compiler->fgMorphCombineSIMDFieldStores(block, stmt); + } +#endif + + VisitStmt(stmt); + } + + // We could check for GT_JMP inside the visitor, but this node is very + // rare so keeping it here avoids pessimizing the hot code. + if (block->endsWithJmpMethod(m_compiler)) + { + // GT_JMP has implicit uses of all arguments. + for (unsigned lclNum = 0; lclNum < m_compiler->info.compArgsCount; lclNum++) + { + UpdateEarlyRefCount(lclNum, nullptr, nullptr); + } + } + + if (m_lclAddrAssertions != nullptr) + { + m_lclAddrAssertions->EndBlock(block); + } + } + // Morph promoted struct fields and count local occurrences. // // Also create and push the value produced by the visited node. This is done here @@ -499,12 +823,29 @@ class LocalAddressVisitor final : public GenTreeVisitor FALLTHROUGH; case GT_STORE_LCL_VAR: + { assert(TopValue(0).Node() == node->AsLclVarCommon()->Data()); + if (m_lclAddrAssertions != nullptr) + { + HandleLocalStoreAssertions(node->AsLclVarCommon(), TopValue(0)); + } + EscapeValue(TopValue(0), node); PopValue(); - FALLTHROUGH; + + SequenceLocal(node->AsLclVarCommon()); + break; + } case GT_LCL_VAR: + if (m_lclAddrAssertions != nullptr) + { + HandleLocalAssertions(node->AsLclVarCommon(), TopValue(0)); + } + + SequenceLocal(node->AsLclVarCommon()); + break; + case GT_LCL_FLD: SequenceLocal(node->AsLclVarCommon()); break; @@ -589,10 +930,34 @@ class LocalAddressVisitor final : public GenTreeVisitor case GT_STOREIND: case GT_STORE_BLK: + { + assert(TopValue(2).Node() == node); + assert(TopValue(1).Node() == node->AsIndir()->Addr()); assert(TopValue(0).Node() == node->AsIndir()->Data()); + + // Data value always escapes. EscapeValue(TopValue(0), node); + + if (node->AsIndir()->IsVolatile() || !TopValue(1).IsAddress()) + { + // Volatile indirections must not be removed so the address, if any, must be escaped. + EscapeValue(TopValue(1), node); + } + else + { + // This consumes the address. + ProcessIndirection(use, TopValue(1), user); + + if ((m_lclAddrAssertions != nullptr) && (*use)->OperIsLocalStore()) + { + HandleLocalStoreAssertions((*use)->AsLclVarCommon(), TopValue(0)); + } + } + PopValue(); - FALLTHROUGH; + PopValue(); + break; + } case GT_BLK: case GT_IND: @@ -607,6 +972,11 @@ class LocalAddressVisitor final : public GenTreeVisitor else { ProcessIndirection(use, TopValue(0), user); + + if ((m_lclAddrAssertions != nullptr) && (*use)->OperIs(GT_LCL_VAR)) + { + HandleLocalAssertions((*use)->AsLclVarCommon(), TopValue(1)); + } } PopValue(); @@ -754,8 +1124,16 @@ class LocalAddressVisitor final : public GenTreeVisitor if (!hasHiddenStructArg) { - m_compiler->lvaSetVarAddrExposed( - varDsc->lvIsStructField ? varDsc->lvParentLcl : lclNum DEBUGARG(AddressExposedReason::ESCAPE_ADDRESS)); + unsigned exposedLclNum = varDsc->lvIsStructField ? varDsc->lvParentLcl : lclNum; + + if (m_lclAddrAssertions != nullptr) + { + m_lclAddrAssertions->OnExposed(exposedLclNum); + } + else + { + m_compiler->lvaSetVarAddrExposed(exposedLclNum DEBUGARG(AddressExposedReason::ESCAPE_ADDRESS)); + } } #ifdef TARGET_64BIT @@ -831,8 +1209,15 @@ class LocalAddressVisitor final : public GenTreeVisitor if (isWide) { - m_compiler->lvaSetVarAddrExposed( - varDsc->lvIsStructField ? varDsc->lvParentLcl : lclNum DEBUGARG(AddressExposedReason::WIDE_INDIR)); + unsigned exposedLclNum = varDsc->lvIsStructField ? varDsc->lvParentLcl : lclNum; + if (m_lclAddrAssertions != nullptr) + { + m_lclAddrAssertions->OnExposed(exposedLclNum); + } + else + { + m_compiler->lvaSetVarAddrExposed(exposedLclNum DEBUGARG(AddressExposedReason::WIDE_INDIR)); + } MorphLocalAddress(node->AsIndir()->Addr(), lclNum, offset); node->gtFlags |= GTF_GLOB_REF; // GLOB_REF may not be set already in the "large offset" case. @@ -857,7 +1242,9 @@ class LocalAddressVisitor final : public GenTreeVisitor void MorphLocalAddress(GenTree* addr, unsigned lclNum, unsigned offset) { assert(addr->TypeIs(TYP_BYREF, TYP_I_IMPL)); - assert(m_compiler->lvaVarAddrExposed(lclNum) || m_compiler->lvaGetDesc(lclNum)->IsHiddenBufferStructArg()); + assert(m_compiler->lvaVarAddrExposed(lclNum) || + ((m_lclAddrAssertions != nullptr) && m_lclAddrAssertions->IsMarkedForExposure(lclNum)) || + m_compiler->lvaGetDesc(lclNum)->IsHiddenBufferStructArg()); if (m_compiler->IsValidLclAddr(lclNum, offset)) { @@ -1408,6 +1795,59 @@ class LocalAddressVisitor final : public GenTreeVisitor } } + //----------------------------------------------------------------------------------- + // HandleLocalStoreAssertions: + // Handle clearing and generating assertions for a local store with the + // specified data value. + // + // Argument: + // store - The local store + // data - Value representing data + // + void HandleLocalStoreAssertions(GenTreeLclVarCommon* store, Value& data) + { + m_lclAddrAssertions->Clear(store->GetLclNum()); + + if (data.IsAddress() && store->OperIs(GT_STORE_LCL_VAR)) + { + LclVarDsc* dsc = m_compiler->lvaGetDesc(store); + // TODO-CQ: We currently don't handle promoted fields, but that has + // no impact since practically all promoted structs end up with + // lvHasLdAddrOp set. + if (!dsc->lvPromoted && !dsc->lvIsStructField && !dsc->lvHasLdAddrOp) + { + m_lclAddrAssertions->Record(store->GetLclNum(), data.LclNum(), data.Offset()); + } + } + } + + //----------------------------------------------------------------------------------- + // HandleLocalAssertions: + // Try to refine the specified "addr" value based on assertions about a specified local. + // + // Argument: + // lcl - The local node + // value - Value of local; will be modified to be an address if an assertion could be found + // + void HandleLocalAssertions(GenTreeLclVarCommon* lcl, Value& value) + { + assert(lcl->OperIs(GT_LCL_VAR)); + if (!lcl->TypeIs(TYP_I_IMPL, TYP_BYREF)) + { + return; + } + + const LocalEqualsLocalAddrAssertion* assertion = m_lclAddrAssertions->GetCurrentAssertion(lcl->GetLclNum()); + if (assertion != nullptr) + { + JITDUMP("Using assertion "); + DBEXEC(VERBOSE, assertion->Print()); + JITDUMP("\n"); + value.Address(assertion->AddressLclNum, assertion->AddressOffset); + m_propagatedAddrs = true; + } + } + public: //------------------------------------------------------------------------ // UpdateEarlyRefCount: updates the ref count for locals @@ -1499,41 +1939,43 @@ class LocalAddressVisitor final : public GenTreeVisitor // PhaseStatus Compiler::fgMarkAddressExposedLocals() { - bool madeChanges = false; - LocalSequencer sequencer(this); - LocalAddressVisitor visitor(this, opts.OptimizationEnabled() ? &sequencer : nullptr); + bool madeChanges = false; - for (BasicBlock* const block : Blocks()) + if (opts.OptimizationDisabled()) { - // Make the current basic block address available globally - compCurBB = block; - - for (Statement* const stmt : block->Statements()) + LocalAddressVisitor visitor(this, nullptr, nullptr); + for (BasicBlock* const block : Blocks()) { -#ifdef FEATURE_SIMD - if (opts.OptimizationEnabled() && stmt->GetRootNode()->TypeIs(TYP_FLOAT) && - stmt->GetRootNode()->OperIsStore()) - { - madeChanges |= fgMorphCombineSIMDFieldStores(block, stmt); - } -#endif + visitor.VisitBlock(block); + } + + madeChanges = visitor.MadeChanges(); + } + else + { + LocalEqualsLocalAddrAssertions assertions(this); + LocalEqualsLocalAddrAssertions* pAssertions = &assertions; - visitor.VisitStmt(stmt); +#ifdef DEBUG + static ConfigMethodRange s_range; + s_range.EnsureInit(JitConfig.JitEnableLocalAddrPropagationRange()); + if (!s_range.Contains(info.compMethodHash())) + { + pAssertions = nullptr; } +#endif - // We could check for GT_JMP inside the visitor, but this node is very - // rare so keeping it here avoids pessimizing the hot code. - if (block->endsWithJmpMethod(this)) + LocalSequencer sequencer(this); + LocalAddressVisitor visitor(this, &sequencer, pAssertions); + for (unsigned i = m_dfsTree->GetPostOrderCount(); i != 0; i--) { - // GT_JMP has implicit uses of all arguments. - for (unsigned lclNum = 0; lclNum < info.compArgsCount; lclNum++) - { - visitor.UpdateEarlyRefCount(lclNum, nullptr, nullptr); - } + visitor.VisitBlock(m_dfsTree->GetPostOrder(i - 1)); } - } - madeChanges |= visitor.MadeChanges(); + madeChanges = visitor.MadeChanges(); + + madeChanges |= fgExposeUnpropagatedLocals(visitor.PropagatedAnyAddresses(), &assertions); + } return madeChanges ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING; } @@ -1645,3 +2087,167 @@ bool Compiler::fgMorphCombineSIMDFieldStores(BasicBlock* block, Statement* stmt) return true; } #endif // FEATURE_SIMD + +//----------------------------------------------------------------------------------- +// fgExposeUnpropagatedLocals: +// Expose the final set of locals that were computed to have their address +// taken. Deletes LCL_ADDR nodes used as the data source of a store if the +// destination can be proven to not be read, and avoid exposing these if +// possible. +// +// Argument: +// propagatedAny - Whether any LCL_ADDR values were propagated +// assertions - Data structure tracking LCL_ADDR assertions +// +// Return Value: +// True if any changes were made to the IR; otherwise false. +// +bool Compiler::fgExposeUnpropagatedLocals(bool propagatedAny, LocalEqualsLocalAddrAssertions* assertions) +{ + if (!propagatedAny) + { + fgExposeLocalsInBitVec(assertions->GetLocalsToExpose()); + return false; + } + + BitVecTraits localsTraits(lvaCount, this); + BitVec unreadLocals = assertions->GetLocalsWithAssertions(); + + struct Store + { + Statement* Statement; + GenTreeLclVarCommon* Tree; + }; + + ArrayStack stores(getAllocator(CMK_LocalAddressVisitor)); + + for (unsigned i = m_dfsTree->GetPostOrderCount(); i != 0; i--) + { + BasicBlock* block = m_dfsTree->GetPostOrder(i - 1); + + for (Statement* stmt : block->Statements()) + { + for (GenTreeLclVarCommon* lcl : stmt->LocalsTreeList()) + { + if (!BitVecOps::IsMember(&localsTraits, unreadLocals, lcl->GetLclNum())) + { + continue; + } + + if (lcl->OperIs(GT_STORE_LCL_VAR, GT_STORE_LCL_FLD)) + { + if (lcl->TypeIs(TYP_I_IMPL, TYP_BYREF) && ((lcl->Data()->gtFlags & GTF_SIDE_EFFECT) == 0)) + { + stores.Push({stmt, lcl}); + } + } + else + { + BitVecOps::RemoveElemD(&localsTraits, unreadLocals, lcl->GetLclNum()); + } + } + } + } + + if (BitVecOps::IsEmpty(&localsTraits, unreadLocals)) + { + JITDUMP("No destinations of propagated LCL_ADDR nodes are unread\n"); + fgExposeLocalsInBitVec(assertions->GetLocalsToExpose()); + return false; + } + + bool changed = false; + for (int i = 0; i < stores.Height(); i++) + { + const Store& store = stores.BottomRef(i); + assert(store.Tree->TypeIs(TYP_I_IMPL, TYP_BYREF)); + + if (BitVecOps::IsMember(&localsTraits, unreadLocals, store.Tree->GetLclNum())) + { + JITDUMP("V%02u is unread; removing store data of [%06u]\n", store.Tree->GetLclNum(), dspTreeID(store.Tree)); + DISPTREE(store.Tree); + + store.Tree->Data()->BashToConst(0, store.Tree->Data()->TypeGet()); + fgSequenceLocals(store.Statement); + + JITDUMP("\nResult:\n"); + DISPTREE(store.Tree); + JITDUMP("\n"); + changed = true; + } + } + + if (changed) + { + // Finally compute new set of exposed locals. + BitVec exposedLocals(BitVecOps::MakeEmpty(&localsTraits)); + + for (unsigned i = m_dfsTree->GetPostOrderCount(); i != 0; i--) + { + BasicBlock* block = m_dfsTree->GetPostOrder(i - 1); + + for (Statement* stmt : block->Statements()) + { + for (GenTreeLclVarCommon* lcl : stmt->LocalsTreeList()) + { + if (!lcl->OperIs(GT_LCL_ADDR)) + { + continue; + } + + LclVarDsc* lclDsc = lvaGetDesc(lcl); + unsigned exposedLclNum = lclDsc->lvIsStructField ? lclDsc->lvParentLcl : lcl->GetLclNum(); + BitVecOps::AddElemD(&localsTraits, exposedLocals, exposedLclNum); + } + } + } + + auto dumpVars = [=, &localsTraits](BitVec_ValArg_T vec, BitVec_ValArg_T other) { + const char* sep = ""; + for (unsigned lclNum = 0; lclNum < lvaCount; lclNum++) + { + if (BitVecOps::IsMember(&localsTraits, vec, lclNum)) + { + JITDUMP("%sV%02u", sep, lclNum); + sep = " "; + } + else if (BitVecOps::IsMember(&localsTraits, other, lclNum)) + { + JITDUMP("%s ", sep); + sep = " "; + } + } + }; + + // TODO-CQ: Instead of intersecting here, we should teach the above + // logic about retbuf LCL_ADDRs not leading to exposure. This should + // allow us to assert that exposedLocals is a subset of + // assertions->GetLocalsToExpose(). + BitVecOps::IntersectionD(&localsTraits, exposedLocals, assertions->GetLocalsToExpose()); + + JITDUMP("Old exposed set: "); + dumpVars(assertions->GetLocalsToExpose(), exposedLocals); + JITDUMP("\nNew exposed set: "); + dumpVars(exposedLocals, assertions->GetLocalsToExpose()); + JITDUMP("\n"); + + fgExposeLocalsInBitVec(exposedLocals); + } + + return changed; +} + +//----------------------------------------------------------------------------------- +// fgExposeLocalsInBitVec: +// Mark all locals in the bit vector as address exposed. +// +void Compiler::fgExposeLocalsInBitVec(BitVec_ValArg_T bitVec) +{ + BitVecTraits localsTraits(lvaCount, this); + BitVecOps::Iter iter(&localsTraits, bitVec); + unsigned lclNum; + while (iter.NextElem(&lclNum)) + { + lvaSetVarAddrExposed(lclNum DEBUGARG(AddressExposedReason::ESCAPE_ADDRESS)); + } +} diff --git a/src/coreclr/jit/ssabuilder.cpp b/src/coreclr/jit/ssabuilder.cpp index 0a5229e32b8b7..a0b018b3078b4 100644 --- a/src/coreclr/jit/ssabuilder.cpp +++ b/src/coreclr/jit/ssabuilder.cpp @@ -1198,6 +1198,7 @@ void SsaBuilder::RenameVariables() assert(varDsc->lvTracked); if (varDsc->lvIsParam || m_pCompiler->info.compInitMem || varDsc->lvMustInit || + (varTypeIsGC(varDsc) && !varDsc->lvHasExplicitInit) || VarSetOps::IsMember(m_pCompiler, m_pCompiler->fgFirstBB->bbLiveIn, varDsc->lvVarIndex)) { unsigned ssaNum = varDsc->lvPerSsaData.AllocSsaNum(m_allocator); diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 909e5ab768af4..58a5285b2e350 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -10007,7 +10007,7 @@ PhaseStatus Compiler::fgValueNumber() ssaDef->m_vnPair.SetBoth(initVal); ssaDef->SetBlock(fgFirstBB); } - else if (info.compInitMem || varDsc->lvMustInit || + else if (info.compInitMem || varDsc->lvMustInit || (varTypeIsGC(varDsc) && !varDsc->lvHasExplicitInit) || VarSetOps::IsMember(this, fgFirstBB->bbLiveIn, varDsc->lvVarIndex)) { // The last clause covers the use-before-def variables (the ones that are live-in to the first block), diff --git a/src/tests/JIT/opt/AssertionPropagation/LocalMorphBackedge.cs b/src/tests/JIT/opt/AssertionPropagation/LocalMorphBackedge.cs new file mode 100644 index 0000000000000..f5b3e7898a1a3 --- /dev/null +++ b/src/tests/JIT/opt/AssertionPropagation/LocalMorphBackedge.cs @@ -0,0 +1,36 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.CompilerServices; +using Xunit; + +public unsafe class LocalMorphBackedge +{ + [Fact] + public static int TestEntryPoint() + { + int x = 1234; + int y = 5678; + int* px; + int** ppx = null; + + for (int i = 100; i < GetUpper(); i++) + { + px = &x; + + if (ppx != null) + { + *ppx = &y; + } + + *px = i; + ppx = &px; + } + + return x; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static int GetUpper() => 102; +} \ No newline at end of file diff --git a/src/tests/JIT/opt/AssertionPropagation/LocalMorphBackedge.csproj b/src/tests/JIT/opt/AssertionPropagation/LocalMorphBackedge.csproj new file mode 100644 index 0000000000000..501217e4d8689 --- /dev/null +++ b/src/tests/JIT/opt/AssertionPropagation/LocalMorphBackedge.csproj @@ -0,0 +1,9 @@ + + + None + True + + + + +