diff --git a/src/coreclr/jit/fginline.cpp b/src/coreclr/jit/fginline.cpp index 473c5ffcc32ec9..efb9a4d6c70ba3 100644 --- a/src/coreclr/jit/fginline.cpp +++ b/src/coreclr/jit/fginline.cpp @@ -393,15 +393,8 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitorgtNewLclvNode(lclNum, varDsc->TypeGet()); - GenTree* addr = m_compiler->gtNewOperNode(GT_ADDR, TYP_I_IMPL, lcl); - addr = m_compiler->gtNewOperNode(GT_COMMA, addr->TypeGet(), asg, addr); - GenTree* obj = m_compiler->gtNewObjNode(varDsc->GetLayout(), addr); - - return obj; + GenTree* lcl = m_compiler->gtNewLclvNode(lclNum, varDsc->TypeGet()); + return m_compiler->gtNewOperNode(GT_COMMA, lcl->TypeGet(), asg, lcl); } #endif // FEATURE_MULTIREG_RET diff --git a/src/coreclr/jit/morphblock.cpp b/src/coreclr/jit/morphblock.cpp index 2912b19d7f1409..ee1973db24f897 100644 --- a/src/coreclr/jit/morphblock.cpp +++ b/src/coreclr/jit/morphblock.cpp @@ -27,12 +27,10 @@ class MorphInitBlockHelper return "MorphInitBlock"; } - static GenTree* MorphBlock(Compiler* comp, GenTree* tree, bool isDest); - static GenTree* MorphCommaBlock(Compiler* comp, GenTreeOp* firstComma); - private: - void TryInitFieldByField(); - void TryPrimitiveInit(); + void TryInitFieldByField(); + void TryPrimitiveInit(); + GenTree* EliminateCommas(GenTree** commaPool); protected: Compiler* m_comp; @@ -127,6 +125,9 @@ GenTree* MorphInitBlockHelper::Morph() { JITDUMP("%s:\n", GetHelperName()); + GenTree* commaPool; + GenTree* sideEffects = EliminateCommas(&commaPool); + PrepareDst(); PrepareSrc(); PropagateBlockAssertions(); @@ -147,12 +148,33 @@ GenTree* MorphInitBlockHelper::Morph() { m_result->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED; } - if (m_comp->verbose) +#endif + + while (sideEffects != nullptr) { - printf("%s (after):\n", GetHelperName()); - m_comp->gtDispTree(m_result); + if (commaPool != nullptr) + { + GenTree* comma = commaPool; + commaPool = commaPool->gtNext; + + assert(comma->OperIs(GT_COMMA)); + comma->AsOp()->gtOp1 = sideEffects; + comma->AsOp()->gtOp2 = m_result; + comma->gtFlags = (sideEffects->gtFlags | m_result->gtFlags) & GTF_ALL_EFFECT; + + m_result = comma; + } + else + { + m_result = m_comp->gtNewOperNode(GT_COMMA, m_result->TypeGet(), sideEffects, m_result); + } + INDEBUG(m_result->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); + + sideEffects = sideEffects->gtNext; } -#endif // DEBUG + + JITDUMP("%s (after):\n", GetHelperName()); + DISPTREE(m_result); return m_result; } @@ -317,125 +339,6 @@ void MorphInitBlockHelper::MorphStructCases() } } -//------------------------------------------------------------------------ -// MorphBlock: Morph a block node preparatory to morphing a block assignment. -// -// Arguments: -// comp - a compiler instance; -// tree - a struct type node; -// isDest - true if this is the destination of an assignment; -// -// Return Value: -// Returns the possibly-morphed node. The caller is responsible for updating -// the parent of this node. -// -// static -GenTree* MorphInitBlockHelper::MorphBlock(Compiler* comp, GenTree* tree, bool isDest) -{ - JITDUMP("MorphBlock for %s tree, before:\n", (isDest ? "dst" : "src")); - DISPTREE(tree); - - assert(varTypeIsStruct(tree)); - - if (tree->OperIs(GT_COMMA)) - { - // TODO-Cleanup: this block is not needed for not struct nodes, but - // TryPrimitiveCopy works wrong without this transformation. - tree = MorphCommaBlock(comp, tree->AsOp()); - if (isDest) - { - tree->SetDoNotCSE(); - } - } - - assert(!tree->OperIsIndir() || varTypeIsI(genActualType(tree->AsIndir()->Addr()))); - - JITDUMP("MorphBlock after:\n"); - DISPTREE(tree); - return tree; -} - -//------------------------------------------------------------------------ -// MorphCommaBlock: transform COMMA(X) as OBJ(COMMA byref(ADDR(X)). -// -// Notes: -// In order to CSE and value number array index expressions and bounds checks, -// the commas in which they are contained need to match. -// The pattern is that the COMMA should be the address expression. -// Therefore, we insert a GT_ADDR just above the node, and wrap it in an obj or ind. -// TODO-1stClassStructs: Consider whether this can be improved. -// Example: -// before: [3] comma struct <- [2] comma struct <- [1] LCL_VAR struct -// after: [5] obj <- [3] comma byref <- [2] comma byref <- [4] addr byref <- [1] LCL_VAR struct -// -// static -GenTree* MorphInitBlockHelper::MorphCommaBlock(Compiler* comp, GenTreeOp* firstComma) -{ - assert(firstComma->OperIs(GT_COMMA)); - - ArrayStack commas(comp->getAllocator(CMK_ArrayStack)); - for (GenTree* currComma = firstComma; currComma != nullptr && currComma->OperIs(GT_COMMA); - currComma = currComma->gtGetOp2()) - { - commas.Push(currComma); - } - - GenTree* lastComma = commas.Top(); - - GenTree* effectiveVal = lastComma->gtGetOp2(); - - if (!effectiveVal->OperIsIndir() && !effectiveVal->IsLocal()) - { - return firstComma; - } - - assert(effectiveVal == firstComma->gtEffectiveVal()); - - GenTree* effectiveValAddr = comp->gtNewOperNode(GT_ADDR, TYP_BYREF, effectiveVal); - - INDEBUG(effectiveValAddr->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); - - lastComma->AsOp()->gtOp2 = effectiveValAddr; - - while (!commas.Empty()) - { - GenTree* comma = commas.Pop(); - comma->gtType = TYP_BYREF; - - // The "IND(COMMA)" => "COMMA(IND)" transform may have set NO_CSEs on these COMMAs, clear them. - comma->ClearDoNotCSE(); - comp->gtUpdateNodeSideEffects(comma); - } - - const var_types blockType = effectiveVal->TypeGet(); - GenTree* addr = firstComma; - - GenTree* res; - - if (blockType == TYP_STRUCT) - { - CORINFO_CLASS_HANDLE structHnd = comp->gtGetStructHandleIfPresent(effectiveVal); - if (structHnd == NO_CLASS_HANDLE) - { - // TODO-1stClassStructs: get rid of all such cases. - res = comp->gtNewIndir(blockType, addr); - } - else - { - res = comp->gtNewObjNode(structHnd, addr); - comp->gtSetObjGcInfo(res->AsObj()); - } - } - else - { - res = comp->gtNewIndir(blockType, addr); - } - - comp->gtUpdateNodeSideEffects(res); - INDEBUG(res->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); - return res; -} - //------------------------------------------------------------------------ // InitFieldByField: Attempts to promote a local block init tree to a tree // of promoted field initialization assignments. @@ -663,6 +566,119 @@ void MorphInitBlockHelper::TryPrimitiveInit() } } +//------------------------------------------------------------------------ +// EliminateCommas: Prepare for block morphing by removing commas from the +// source operand of the assignment. +// +// Parameters: +// commaPool - [out] Pool of GT_COMMA nodes linked by their gtNext nodes that +// can be used by the caller to avoid unnecessarily creating +// new commas. +// +// Returns: +// Extracted side effects, in reverse order, linked via the gtNext fields of +// the nodes. +// +// Notes: +// We have a tree like the following (note that location-valued commas are +// illegal, so there cannot be a comma on the left): +// +// ASG +// / \. +// IND COMMA +// | / \. +// B C D +// +// We'd like downstream code to just see and be expand ASG(IND(B), D). +// We will produce: +// +// COMMA +// / \. +// ASG COMMA +// / \ / \. +// tmp B C ASG +// / \. +// IND D +// | +// tmp +// +// If the ASG has GTF_REVERSE_OPS then we will produce: +// +// COMMA +// / \. +// C ASG +// / \. +// IND D +// | +// B +// +// While keeping the GTF_REVERSE_OPS. +// +// Note that the final resulting tree is created in the caller since it also +// needs to propagate side effect flags from the decomposed assignment to all +// the created commas. Therefore this function just returns a linked list of +// the side effects to be used for that purpose. +// +GenTree* MorphInitBlockHelper::EliminateCommas(GenTree** commaPool) +{ + *commaPool = nullptr; + + GenTree* sideEffects = nullptr; + auto addSideEffect = [&sideEffects](GenTree* sideEff) { + sideEff->gtNext = sideEffects; + sideEffects = sideEff; + }; + + auto addComma = [commaPool, &addSideEffect](GenTree* comma) { + addSideEffect(comma->gtGetOp1()); + comma->gtNext = *commaPool; + *commaPool = comma; + }; + + GenTree* lhs = m_asg->gtGetOp1(); + assert(lhs->OperIsIndir() || lhs->OperIsLocal()); + + GenTree* rhs = m_asg->gtGetOp2(); + + if (m_asg->IsReverseOp()) + { + while (rhs->OperIs(GT_COMMA)) + { + addComma(rhs); + rhs = rhs->gtGetOp2(); + } + } + else + { + if (lhs->OperIsIndir() && rhs->OperIs(GT_COMMA)) + { + GenTree* addr = lhs->gtGetOp1(); + if (((addr->gtFlags & GTF_ALL_EFFECT) != 0) || (((rhs->gtFlags & GTF_ASG) != 0) && !addr->IsInvariant())) + { + unsigned lhsAddrLclNum = m_comp->lvaGrabTemp(true DEBUGARG("Block morph LHS addr")); + + addSideEffect(m_comp->gtNewTempAssign(lhsAddrLclNum, addr)); + lhs->AsUnOp()->gtOp1 = m_comp->gtNewLclvNode(lhsAddrLclNum, genActualType(addr)); + m_comp->gtUpdateNodeSideEffects(lhs); + } + } + + while (rhs->OperIs(GT_COMMA)) + { + addComma(rhs); + rhs = rhs->gtGetOp2(); + } + } + + if (sideEffects != nullptr) + { + m_asg->gtOp2 = rhs; + m_comp->gtUpdateNodeSideEffects(m_asg); + } + + return sideEffects; +} + class MorphCopyBlockHelper : public MorphInitBlockHelper { public: @@ -733,12 +749,7 @@ MorphCopyBlockHelper::MorphCopyBlockHelper(Compiler* comp, GenTree* asg) : Morph // void MorphCopyBlockHelper::PrepareSrc() { - GenTree* origSrc = m_asg->gtGetOp2(); - m_src = MorphBlock(m_comp, origSrc, false); - if (m_src != origSrc) - { - m_asg->gtOp2 = m_src; - } + m_src = m_asg->gtGetOp2(); if (m_src->IsLocal()) { diff --git a/src/coreclr/jit/rationalize.cpp b/src/coreclr/jit/rationalize.cpp index d0acce7b1c9106..c1d37ef0ee326b 100644 --- a/src/coreclr/jit/rationalize.cpp +++ b/src/coreclr/jit/rationalize.cpp @@ -69,8 +69,8 @@ void Rationalizer::RewriteIndir(LIR::Use& use) // Arguments: // use - A use of a GT_IND node of SIMD type // -// TODO-ADDR: delete this once block morphing stops taking addresses of locals -// under COMMAs. +// TODO-ADDR: today this only exists because the xarch backend does not handle +// IND(LCL_VAR_ADDR/LCL_FLD_ADDR) when the address is contained correctly. // void Rationalizer::RewriteSIMDIndir(LIR::Use& use) {