From e1081df6da8ffbe667a7ea909a154c5b59e86351 Mon Sep 17 00:00:00 2001 From: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com> Date: Tue, 6 Dec 2022 20:26:30 +0300 Subject: [PATCH] Fold all local addresses in local morph (#79194) * Fix FIELD size calculation "getFieldClass" returns the field's owner class, not its own. * Precise value numbering for local addresses * ADD(LCL_ADDR, CONST) => LCL_FLD_ADDR * Fold to local address nodes in local morph Unconditionally. * Clean up lvQuirkToLong * Support ADD(ADDR(LCL), CONST) in local morph * Morph the local address for locations * Fold hidden buffer args too * Fix gcWriteBarrierFormFromTargetAddress * Simplify IsLocalAddrExpr equivalents No diffs except for one regression in a 400K bytes struct test. --- src/coreclr/jit/compiler.h | 2 - src/coreclr/jit/flowgraph.cpp | 23 ---- src/coreclr/jit/gcinfo.cpp | 14 +-- src/coreclr/jit/gentree.cpp | 122 ++----------------- src/coreclr/jit/lclmorph.cpp | 218 +++++++++++++++++++--------------- src/coreclr/jit/lclvars.cpp | 2 +- src/coreclr/jit/morph.cpp | 26 ++-- src/coreclr/jit/optimizer.cpp | 56 ++++----- src/coreclr/jit/valuenum.cpp | 11 +- 9 files changed, 182 insertions(+), 292 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 92d7ad8a59bd10..803ac05d6a0e34 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -2970,8 +2970,6 @@ class Compiler typedef ArrayStack GenTreeStack; - static bool gtHasCallOnStack(GenTreeStack* parentStack); - //========================================================================= // BasicBlock functions #ifdef DEBUG diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index e5bcee36e0ab5b..7ed3f33073ac0f 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -754,29 +754,6 @@ GenTreeLclVar* Compiler::fgIsIndirOfAddrOfLocal(GenTree* tree) { GenTree* addr = tree->AsIndir()->Addr(); - // Post rationalization, we can have Indir(Lea(..) trees. Therefore to recognize - // Indir of addr of a local, skip over Lea in Indir(Lea(base, index, scale, offset)) - // to get to base variable. - if (addr->OperGet() == GT_LEA) - { - // We use this method in backward dataflow after liveness computation - fgInterBlockLocalVarLiveness(). - // Therefore it is critical that we don't miss 'uses' of any local. It may seem this method overlooks - // if the index part of the LEA has indir( someAddrOperator ( lclVar ) ) to search for a use but it's - // covered by the fact we're traversing the expression in execution order and we also visit the index. - GenTreeAddrMode* lea = addr->AsAddrMode(); - GenTree* base = lea->Base(); - - if (base != nullptr) - { - if (base->OperGet() == GT_IND) - { - return fgIsIndirOfAddrOfLocal(base); - } - // else use base as addr - addr = base; - } - } - if (addr->OperGet() == GT_ADDR) { GenTree* lclvar = addr->AsOp()->gtOp1; diff --git a/src/coreclr/jit/gcinfo.cpp b/src/coreclr/jit/gcinfo.cpp index 6ee757a7238bc8..2947053dff1057 100644 --- a/src/coreclr/jit/gcinfo.cpp +++ b/src/coreclr/jit/gcinfo.cpp @@ -284,7 +284,13 @@ GCInfo::WriteBarrierForm GCInfo::gcIsWriteBarrierCandidate(GenTreeStoreInd* stor // GCInfo::WriteBarrierForm GCInfo::gcWriteBarrierFormFromTargetAddress(GenTree* tgtAddr) { - // We will assume there is no point in trying to deconstruct a TYP_I_IMPL address. + if (tgtAddr->IsLocalAddrExpr() != nullptr) + { + // No need for a GC barrier when writing to a local variable. + return GCInfo::WBF_NoBarrier; + } + + // No point in trying to further deconstruct a TYP_I_IMPL address. if (tgtAddr->TypeGet() == TYP_I_IMPL) { return GCInfo::WBF_BarrierUnknown; @@ -347,12 +353,6 @@ GCInfo::WriteBarrierForm GCInfo::gcWriteBarrierFormFromTargetAddress(GenTree* tg } } - if (tgtAddr->IsLocalAddrExpr() != nullptr) - { - // No need for a GC barrier when writing to a local variable. - return GCInfo::WBF_NoBarrier; - } - if (tgtAddr->TypeGet() == TYP_REF) { return GCInfo::WBF_BarrierUnchecked; diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 23767cf9ea8595..13c79a059ebe64 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -16407,33 +16407,6 @@ bool Compiler::gtHasCatchArg(GenTree* tree) return false; } -//------------------------------------------------------------------------ -// gtHasCallOnStack: -// -// Arguments: -// parentStack: a context (stack of parent nodes) -// -// Return Value: -// returns true if any of the parent nodes are a GT_CALL -// -// Assumptions: -// We have a stack of parent nodes. This generally requires that -// we are performing a recursive tree walk using struct fgWalkData -// -//------------------------------------------------------------------------ -/* static */ bool Compiler::gtHasCallOnStack(GenTreeStack* parentStack) -{ - for (int i = 0; i < parentStack->Height(); i++) - { - GenTree* node = parentStack->Top(i); - if (node->OperGet() == GT_CALL) - { - return true; - } - } - return false; -} - //------------------------------------------------------------------------ // gtGetTypeProducerKind: determine if a tree produces a runtime type, and // if so, how. @@ -16829,87 +16802,27 @@ bool GenTree::DefinesLocal( // Return Value: // Whether "this" is a LCL_VAR|FLD_ADDR-equivalent tree. // -// Notes: -// This function is contractually bound to recognize a superset of trees -// that "LocalAddressVisitor" recognizes, as it is used by "DefinesLocal" -// to detect stores to tracked locals. -// bool GenTree::DefinesLocalAddr(GenTreeLclVarCommon** pLclVarTree, ssize_t* pOffset) { - if (OperIs(GT_ADDR, GT_LCL_VAR_ADDR, GT_LCL_FLD_ADDR)) + if (OperIs(GT_ADDR) || OperIsLocalAddr()) { - GenTree* addrArg = this; + GenTree* lclNode = this; if (OperGet() == GT_ADDR) { - addrArg = AsOp()->gtOp1; + lclNode = AsOp()->gtOp1; } - if (addrArg->IsLocal() || addrArg->OperIsLocalAddr()) + if (lclNode->IsLocal() || lclNode->OperIsLocalAddr()) { - GenTreeLclVarCommon* addrArgLcl = addrArg->AsLclVarCommon(); - *pLclVarTree = addrArgLcl; + *pLclVarTree = lclNode->AsLclVarCommon(); if (pOffset != nullptr) { - *pOffset += addrArgLcl->GetLclOffs(); + *pOffset += lclNode->AsLclVarCommon()->GetLclOffs(); } return true; } - else if (addrArg->OperGet() == GT_IND) - { - // A GT_ADDR of a GT_IND can both be optimized away, recurse using the child of the GT_IND - return addrArg->AsIndir()->Addr()->DefinesLocalAddr(pLclVarTree, pOffset); - } - } - else if (OperGet() == GT_ADD) - { - if (AsOp()->gtOp1->IsCnsIntOrI()) - { - if (pOffset != nullptr) - { - *pOffset += AsOp()->gtOp1->AsIntCon()->IconValue(); - } - - return AsOp()->gtOp2->DefinesLocalAddr(pLclVarTree, pOffset); - } - else if (AsOp()->gtOp2->IsCnsIntOrI()) - { - if (pOffset != nullptr) - { - *pOffset += AsOp()->gtOp2->AsIntCon()->IconValue(); - } - - return AsOp()->gtOp1->DefinesLocalAddr(pLclVarTree, pOffset); - } - } - // Post rationalization we could have GT_IND(GT_LEA(..)) trees. - else if (OperGet() == GT_LEA) - { - // This method gets invoked during liveness computation and therefore it is critical - // that we don't miss 'use' of any local. The below logic is making the assumption - // that in case of LEA(base, index, offset) - only base can be a GT_LCL_VAR_ADDR - // and index is not. - CLANG_FORMAT_COMMENT_ANCHOR; - -#ifdef DEBUG - GenTree* index = AsOp()->gtOp2; - if (index != nullptr) - { - assert(!index->DefinesLocalAddr(pLclVarTree, pOffset)); - } -#endif // DEBUG - - GenTree* base = AsAddrMode()->Base(); - if (base != nullptr) - { - if (pOffset != nullptr) - { - *pOffset += AsAddrMode()->Offset(); - } - - return base->DefinesLocalAddr(pLclVarTree, pOffset); - } } // Otherwise... @@ -16929,17 +16842,7 @@ const GenTreeLclVarCommon* GenTree::IsLocalAddrExpr() const { return this->AsLclVarCommon(); } - else if (OperGet() == GT_ADD) - { - if (AsOp()->gtOp1->OperGet() == GT_CNS_INT) - { - return AsOp()->gtOp2->IsLocalAddrExpr(); - } - else if (AsOp()->gtOp2->OperGet() == GT_CNS_INT) - { - return AsOp()->gtOp1->IsLocalAddrExpr(); - } - } + // Otherwise... return nullptr; } @@ -18395,12 +18298,6 @@ bool Compiler::gtIsTypeof(GenTree* tree, CORINFO_CLASS_HANDLE* handle) // Returns: // A tree representing the address of a local. // -// Remarks: -// This function should not be used until after morph when local address -// nodes have been normalized. However, before that IsOptimizingRetBufAsLocal -// can be used to at least check if the call has a retbuf that we are -// optimizing. -// GenTree* Compiler::gtCallGetDefinedRetBufLclAddr(GenTreeCall* call) { if (!call->IsOptimizingRetBufAsLocal()) @@ -18427,10 +18324,7 @@ GenTree* Compiler::gtCallGetDefinedRetBufLclAddr(GenTreeCall* call) // This may be called very late to check validity of LIR. node = node->gtSkipReloadOrCopy(); -#ifdef DEBUG - GenTreeLclVarCommon* lcl; - assert(node->DefinesLocalAddr(&lcl) && lvaGetDesc(lcl)->lvHiddenBufferStructArg); -#endif + assert(node->OperIsLocalAddr() && lvaGetDesc(node->AsLclVarCommon())->lvHiddenBufferStructArg); return node; } diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp index f2f5c44a46e6e7..810f22d7fe7b68 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/src/coreclr/jit/lclmorph.cpp @@ -248,7 +248,6 @@ class LocalAddressVisitor final : public GenTreeVisitor // Arguments: // val - the input value // addOffset - the offset to add - // field - the FIELD node that uses the input address value // // Return Value: // `true` if the value was consumed. `false` if the input value @@ -470,6 +469,29 @@ class LocalAddressVisitor final : public GenTreeVisitor PopValue(); break; + case GT_ADD: + assert(TopValue(2).Node() == node); + assert(TopValue(1).Node() == node->gtGetOp1()); + assert(TopValue(0).Node() == node->gtGetOp2()); + + if (node->gtGetOp2()->IsCnsIntOrI()) + { + ssize_t offset = node->gtGetOp2()->AsIntCon()->IconValue(); + if (FitsIn(offset) && TopValue(2).AddOffset(TopValue(1), static_cast(offset))) + { + INDEBUG(TopValue(0).Consume()); + PopValue(); + PopValue(); + break; + } + } + + EscapeValue(TopValue(0), node); + PopValue(); + EscapeValue(TopValue(0), node); + PopValue(); + break; + case GT_FIELD_ADDR: if (node->AsField()->IsInstance()) { @@ -636,8 +658,10 @@ class LocalAddressVisitor final : public GenTreeVisitor unsigned lclNum = val.LclNum(); LclVarDsc* varDsc = m_compiler->lvaGetDesc(lclNum); - bool hasHiddenStructArg = false; - if (m_compiler->opts.compJitOptimizeStructHiddenBuffer) + GenTreeCall* callUser = user->IsCall() ? user->AsCall() : nullptr; + bool hasHiddenStructArg = false; + if (m_compiler->opts.compJitOptimizeStructHiddenBuffer && (callUser != nullptr) && + IsValidLclAddr(lclNum, val.Offset())) { // We will only attempt this optimization for locals that are: // a) Not susceptible to liveness bugs (see "lvaSetHiddenBufferStructArg"). @@ -652,14 +676,12 @@ class LocalAddressVisitor final : public GenTreeVisitor } #endif // TARGET_X86 - GenTreeCall* callTree = user->IsCall() ? user->AsCall() : nullptr; - - if (isSuitableLocal && (callTree != nullptr) && callTree->gtArgs.HasRetBuffer() && - (val.Node() == callTree->gtArgs.GetRetBufferArg()->GetNode())) + if (isSuitableLocal && callUser->gtArgs.HasRetBuffer() && + (val.Node() == callUser->gtArgs.GetRetBufferArg()->GetNode())) { m_compiler->lvaSetHiddenBufferStructArg(lclNum); hasHiddenStructArg = true; - callTree->gtCallMoreFlags |= GTF_CALL_M_RETBUFFARG_LCLOPT; + callUser->gtCallMoreFlags |= GTF_CALL_M_RETBUFFARG_LCLOPT; } } @@ -668,32 +690,22 @@ class LocalAddressVisitor final : public GenTreeVisitor m_compiler->lvaSetVarAddrExposed( varDsc->lvIsStructField ? varDsc->lvParentLcl : lclNum DEBUGARG(AddressExposedReason::ESCAPE_ADDRESS)); } + #ifdef TARGET_64BIT // If the address of a variable is passed in a call and the allocation size of the variable // is 32 bits we will quirk the size to 64 bits. Some PInvoke signatures incorrectly specify // a ByRef to an INT32 when they actually write a SIZE_T or INT64. There are cases where // overwriting these extra 4 bytes corrupts some data (such as a saved register) that leads // to A/V. Whereas previously the JIT64 codegen did not lead to an A/V. - if (!varDsc->lvIsParam && !varDsc->lvIsStructField && (genActualType(varDsc->TypeGet()) == TYP_INT)) + if ((callUser != nullptr) && !varDsc->lvIsParam && !varDsc->lvIsStructField && genActualTypeIsInt(varDsc)) { - // TODO-Cleanup: This should simply check if the user is a call node, not if a call ancestor exists. - if (Compiler::gtHasCallOnStack(&m_ancestors)) - { - varDsc->lvQuirkToLong = true; - JITDUMP("Adding a quirk for the storage size of V%02u of type %s\n", val.LclNum(), - varTypeName(varDsc->TypeGet())); - } + varDsc->lvQuirkToLong = true; + JITDUMP("Adding a quirk for the storage size of V%02u of type %s\n", val.LclNum(), + varTypeName(varDsc->TypeGet())); } #endif // TARGET_64BIT - // TODO-ADDR: For now use LCL_VAR_ADDR and LCL_FLD_ADDR only as call arguments and assignment sources. - // Other usages require more changes. For example, a tree like OBJ(ADD(ADDR(LCL_VAR), 4)) - // could be changed to OBJ(LCL_FLD_ADDR) but historically DefinesLocalAddr did not recognize - // LCL_FLD_ADDR (and there may be other things now as well). - if (user->OperIs(GT_CALL, GT_ASG) && !hasHiddenStructArg) - { - MorphLocalAddress(val); - } + MorphLocalAddress(val.Node(), lclNum, val.Offset()); INDEBUG(val.Consume();) } @@ -738,7 +750,7 @@ class LocalAddressVisitor final : public GenTreeVisitor // say, 2 consecutive Int32 struct fields as Int64 has more practical value. LclVarDsc* varDsc = m_compiler->lvaGetDesc(val.LclNum()); - unsigned indirSize = GetIndirSize(node, user); + unsigned indirSize = GetIndirSize(node); bool isWide; if ((indirSize == 0) || ((val.Offset() + indirSize) > UINT16_MAX)) @@ -782,6 +794,7 @@ class LocalAddressVisitor final : public GenTreeVisitor m_compiler->lvaSetVarAddrExposed(varDsc->lvIsStructField ? varDsc->lvParentLcl : val.LclNum() DEBUGARG(AddressExposedReason::WIDE_INDIR)); + MorphWideLocalIndir(val); } else { @@ -800,57 +813,25 @@ class LocalAddressVisitor final : public GenTreeVisitor // user - the node that uses the indirection // // Notes: - // This returns 0 for indirection of unknown size. GT_IND nodes that have type - // TYP_STRUCT are expected to only appears on the RHS of an assignment, in which - // case the LHS size will be used instead. Otherwise 0 is returned as well. + // This returns 0 for indirection of unknown size, i. e. IND + // nodes that are used as sources of STORE_DYN_BLKs. // - unsigned GetIndirSize(GenTree* indir, GenTree* user) + unsigned GetIndirSize(GenTree* indir) { assert(indir->OperIs(GT_IND, GT_OBJ, GT_BLK, GT_FIELD)); if (indir->TypeGet() != TYP_STRUCT) { - return genTypeSize(indir->TypeGet()); + return genTypeSize(indir); } - // A struct indir that is the RHS of an assignment needs special casing: - // - It can be a GT_IND of type TYP_STRUCT, in which case the size is given by the LHS. - // - It can be a GT_OBJ that has a correct size, but different than the size of the LHS. - // The LHS size takes precedence. - // Just take the LHS size in all cases. - if (user != nullptr && user->OperIs(GT_ASG) && (indir == user->gtGetOp2())) + if (indir->OperIs(GT_IND)) { - indir = user->gtGetOp1(); - - if (indir->TypeGet() != TYP_STRUCT) - { - return genTypeSize(indir->TypeGet()); - } - - // The LHS may be a LCL_VAR/LCL_FLD, these are not indirections so we need to handle them here. - switch (indir->OperGet()) - { - case GT_LCL_VAR: - return m_compiler->lvaGetDesc(indir->AsLclVar())->lvExactSize; - case GT_LCL_FLD: - return indir->AsLclFld()->GetSize(); - default: - break; - } + // TODO-1stClassStructs: remove once "IND" nodes are no more. + return 0; } - switch (indir->OperGet()) - { - case GT_FIELD: - return m_compiler->info.compCompHnd->getClassSize( - m_compiler->info.compCompHnd->getFieldClass(indir->AsField()->gtFldHnd)); - case GT_BLK: - case GT_OBJ: - return indir->AsBlk()->GetLayout()->GetSize(); - default: - assert(indir->OperIs(GT_IND)); - return 0; - } + return indir->GetLayout(m_compiler)->GetSize(); } //------------------------------------------------------------------------ @@ -858,44 +839,33 @@ class LocalAddressVisitor final : public GenTreeVisitor // to a single LCL_VAR_ADDR or LCL_FLD_ADDR node. // // Arguments: - // val - a value that represents the local address + // addr - The address tree + // lclNum - Local number of the variable in question + // offset - Offset for the address // - void MorphLocalAddress(const Value& val) + void MorphLocalAddress(GenTree* addr, unsigned lclNum, unsigned offset) { - assert(val.IsAddress()); - assert(val.Node()->TypeIs(TYP_BYREF, TYP_I_IMPL)); - assert(m_compiler->lvaVarAddrExposed(val.LclNum())); - - LclVarDsc* varDsc = m_compiler->lvaGetDesc(val.LclNum()); - - if (varDsc->lvPromoted || varDsc->lvIsStructField) - { - // TODO-ADDR: For now we ignore promoted variables, they require - // additional changes in subsequent phases. - return; - } - - GenTree* addr = val.Node(); + assert(addr->TypeIs(TYP_BYREF, TYP_I_IMPL)); + assert(m_compiler->lvaVarAddrExposed(lclNum) || m_compiler->lvaGetDesc(lclNum)->IsHiddenBufferStructArg()); - if (val.Offset() > UINT16_MAX) + if (offset == 0) { - // The offset is too large to store in a LCL_FLD_ADDR node, - // use ADD(LCL_VAR_ADDR, offset) instead. - addr->ChangeOper(GT_ADD); - addr->AsOp()->gtOp1 = m_compiler->gtNewLclVarAddrNode(val.LclNum()); - addr->AsOp()->gtOp2 = m_compiler->gtNewIconNode(val.Offset(), TYP_I_IMPL); + addr->ChangeOper(GT_LCL_VAR_ADDR); + addr->AsLclVar()->SetLclNum(lclNum); } - else if (val.Offset() != 0) + else if (IsValidLclAddr(lclNum, offset)) { addr->ChangeOper(GT_LCL_FLD_ADDR); - addr->AsLclFld()->SetLclNum(val.LclNum()); - addr->AsLclFld()->SetLclOffs(val.Offset()); + addr->AsLclFld()->SetLclNum(lclNum); + addr->AsLclFld()->SetLclOffs(offset); addr->AsLclFld()->SetLayout(nullptr); } else { - addr->ChangeOper(GT_LCL_VAR_ADDR); - addr->AsLclVar()->SetLclNum(val.LclNum()); + // The offset was too large to store in a LCL_FLD_ADDR node, use ADD(LCL_VAR_ADDR, offset) instead. + addr->ChangeOper(GT_ADD); + addr->AsOp()->gtOp1 = m_compiler->gtNewLclVarAddrNode(lclNum); + addr->AsOp()->gtOp2 = m_compiler->gtNewIconNode(offset, TYP_I_IMPL); } // Local address nodes never have side effects (nor any other flags, at least at this point). @@ -904,8 +874,50 @@ class LocalAddressVisitor final : public GenTreeVisitor } //------------------------------------------------------------------------ - // MorphLocalIndir: Change a tree that represents an indirect access to a struct - // variable to a canonical shape (one of "IndirTransform"s). + // MorphLocalIndir: Change a tree that represents indirect access to a + // local variable to OBJ/BLK/IND(LCL_ADDR). + // + // Arguments: + // val - a value that represents the local indirection + // + // Notes: + // This morphing is performed when the access cannot be turned into a + // a local node, e. g. it is volatile or "wide". + // + void MorphWideLocalIndir(const Value& val) + { + assert(val.Node()->OperIsIndir() || val.Node()->OperIs(GT_FIELD)); + + GenTree* node = val.Node(); + GenTree* addr = node->gtGetOp1(); + + MorphLocalAddress(addr, val.LclNum(), val.Offset()); + + if (node->OperIs(GT_FIELD)) + { + if (node->TypeIs(TYP_STRUCT)) + { + ClassLayout* layout = node->GetLayout(m_compiler); + node->SetOper(GT_OBJ); + node->AsBlk()->SetLayout(layout); + node->AsBlk()->gtBlkOpKind = GenTreeBlk::BlkOpKindInvalid; +#ifndef JIT32_GCENCODER + node->AsBlk()->gtBlkOpGcUnsafe = false; +#endif // !JIT32_GCENCODER + } + else + { + node->SetOper(GT_IND); + } + } + + // GLOB_REF may not be set already in the "large offset" case. Add it. + node->gtFlags |= GTF_GLOB_REF; + } + + //------------------------------------------------------------------------ + // MorphLocalIndir: Change a tree that represents indirect access to a + // local variable to a canonical shape (one of "IndirTransform"s). // // Arguments: // val - a value that represents the local indirection @@ -1357,6 +1369,24 @@ class LocalAddressVisitor final : public GenTreeVisitor } } + //------------------------------------------------------------------------ + // IsValidLclAddr: Can the given local address be represented as "LCL_FLD_ADDR"? + // + // Local address nodes cannot point beyond the local and can only store + // 16 bits worth of offset. + // + // Arguments: + // lclNum - The local's number + // offset - The address' offset + // + // Return Value: + // Whether "LCL_FLD_ADDR [+offset]" would be valid IR. + // + bool IsValidLclAddr(unsigned lclNum, unsigned offset) const + { + return (offset < UINT16_MAX) && (offset < m_compiler->lvaLclExactSize(lclNum)); + } + //------------------------------------------------------------------------ // IsUnused: is the given node unused? // diff --git a/src/coreclr/jit/lclvars.cpp b/src/coreclr/jit/lclvars.cpp index 3f0809e782ebb9..6897755c8cc0bf 100644 --- a/src/coreclr/jit/lclvars.cpp +++ b/src/coreclr/jit/lclvars.cpp @@ -4236,7 +4236,7 @@ void Compiler::lvaMarkLclRefs(GenTree* tree, BasicBlock* block, Statement* stmt, if (tree->OperIsLocalAddr()) { LclVarDsc* varDsc = lvaGetDesc(tree->AsLclVarCommon()); - assert(varDsc->IsAddressExposed()); + assert(varDsc->IsAddressExposed() || varDsc->IsHiddenBufferStructArg()); varDsc->incRefCnts(weight, this); return; } diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 4950d387e0773c..b9ca2f14b69b76 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -11274,33 +11274,29 @@ GenTree* Compiler::fgOptimizeAddition(GenTreeOp* add) if (opts.OptimizationEnabled()) { - // Reduce local addresses: "ADD(ADDR(LCL_VAR), OFFSET)" => "ADDR(LCL_FLD OFFSET)". - // TODO-ADDR: do "ADD(LCL_FLD/VAR_ADDR, OFFSET)" => "LCL_FLD_ADDR" instead. + // Reduce local addresses: "ADD(LCL_ADDR, OFFSET)" => "LCL_FLD_ADDR". // - if (op1->OperIs(GT_ADDR) && op2->IsCnsIntOrI() && op1->gtGetOp1()->OperIsLocalRead()) + if (op1->OperIsLocalAddr() && op2->IsCnsIntOrI()) { - GenTreeUnOp* addrNode = op1->AsUnOp(); - GenTreeLclVarCommon* lclNode = addrNode->gtGetOp1()->AsLclVarCommon(); - GenTreeIntCon* offsetNode = op2->AsIntCon(); + GenTreeLclVarCommon* lclAddrNode = op1->AsLclVarCommon(); + GenTreeIntCon* offsetNode = op2->AsIntCon(); if (FitsIn(offsetNode->IconValue())) { - unsigned offset = lclNode->GetLclOffs() + static_cast(offsetNode->IconValue()); + unsigned offset = lclAddrNode->GetLclOffs() + static_cast(offsetNode->IconValue()); // Note: the emitter does not expect out-of-bounds access for LCL_FLD_ADDR. - if (FitsIn(offset) && (offset < lvaLclExactSize(lclNode->GetLclNum()))) + if (FitsIn(offset) && (offset < lvaLclExactSize(lclAddrNode->GetLclNum()))) { - // Types of location nodes under ADDRs do not matter. We arbitrarily choose TYP_UBYTE. - lclNode->ChangeType(TYP_UBYTE); - lclNode->SetOper(GT_LCL_FLD); - lclNode->AsLclFld()->SetLclOffs(offset); - lvaSetVarDoNotEnregister(lclNode->GetLclNum() DEBUGARG(DoNotEnregisterReason::LocalField)); + lclAddrNode->SetOper(GT_LCL_FLD_ADDR); + lclAddrNode->AsLclFld()->SetLclOffs(offset); + assert(lvaGetDesc(lclAddrNode)->lvDoNotEnregister); - addrNode->SetVNsFromNode(add); + lclAddrNode->SetVNsFromNode(add); DEBUG_DESTROY_NODE(offsetNode); DEBUG_DESTROY_NODE(add); - return addrNode; + return lclAddrNode; } } } diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index fd8ddb1e915199..6b6b2e8cab55d3 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -6021,27 +6021,37 @@ bool Compiler::optIsVarAssignedWithDesc(Statement* stmt, isVarAssgDsc* dsc) return WALK_CONTINUE; } - // Check for calls and determine what's written. + // Determine what's written and check for calls. // - GenTree* dest = nullptr; if (tree->OperIs(GT_CALL)) { m_dsc->ivaMaskCall = optCallInterf(tree->AsCall()); - - dest = m_compiler->gtCallGetDefinedRetBufLclAddr(tree->AsCall()); - if (dest == nullptr) - { - return WALK_CONTINUE; - } - - dest = dest->AsOp()->gtOp1; } else { - dest = tree->AsOp()->gtOp1; - } + assert(tree->OperIs(GT_ASG)); - genTreeOps const destOper = dest->OperGet(); + genTreeOps destOper = tree->gtGetOp1()->OperGet(); + if (destOper == GT_LCL_FLD) + { + // We can't track every field of every var. Moreover, indirections + // may access different parts of the var as different (but + // overlapping) fields. So just treat them as indirect accesses + // + // unsigned lclNum = dest->AsLclFld()->GetLclNum(); + // noway_assert(lvaTable[lclNum].lvAddrTaken); + // + varRefKinds refs = varTypeIsGC(tree->TypeGet()) ? VR_IND_REF : VR_IND_SCL; + m_dsc->ivaMaskInd = varRefKinds(m_dsc->ivaMaskInd | refs); + } + else if (destOper == GT_IND) + { + // Set the proper indirection bits + // + varRefKinds refs = varTypeIsGC(tree->TypeGet()) ? VR_IND_REF : VR_IND_SCL; + m_dsc->ivaMaskInd = varRefKinds(m_dsc->ivaMaskInd | refs); + } + } // Determine if the tree modifies a particular local // @@ -6069,26 +6079,6 @@ bool Compiler::optIsVarAssignedWithDesc(Statement* stmt, isVarAssgDsc* dsc) } } - if (destOper == GT_LCL_FLD) - { - // We can't track every field of every var. Moreover, indirections - // may access different parts of the var as different (but - // overlapping) fields. So just treat them as indirect accesses - // - // unsigned lclNum = dest->AsLclFld()->GetLclNum(); - // noway_assert(lvaTable[lclNum].lvAddrTaken); - // - varRefKinds refs = varTypeIsGC(tree->TypeGet()) ? VR_IND_REF : VR_IND_SCL; - m_dsc->ivaMaskInd = varRefKinds(m_dsc->ivaMaskInd | refs); - } - else if (destOper == GT_IND) - { - // Set the proper indirection bits - // - varRefKinds refs = varTypeIsGC(tree->TypeGet()) ? VR_IND_REF : VR_IND_SCL; - m_dsc->ivaMaskInd = varRefKinds(m_dsc->ivaMaskInd | refs); - } - return WALK_CONTINUE; } }; diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index f8be72076d7150..4a8c2ad0976bf8 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -8818,9 +8818,14 @@ void Compiler::fgValueNumberTree(GenTree* tree) { case GT_LCL_VAR_ADDR: case GT_LCL_FLD_ADDR: - assert(lvaVarAddrExposed(tree->AsLclVarCommon()->GetLclNum())); - tree->gtVNPair.SetBoth(vnStore->VNForExpr(compCurBB, tree->TypeGet())); - break; + { + unsigned lclNum = tree->AsLclVarCommon()->GetLclNum(); + unsigned lclOffs = tree->AsLclVarCommon()->GetLclOffs(); + tree->gtVNPair.SetBoth(vnStore->VNForFunc(TYP_BYREF, VNF_PtrToLoc, vnStore->VNForIntCon(lclNum), + vnStore->VNForIntPtrCon(lclOffs))); + assert(lvaGetDesc(lclNum)->IsAddressExposed() || lvaGetDesc(lclNum)->IsHiddenBufferStructArg()); + } + break; case GT_LCL_VAR: {