From 75c563b05336fb113246a374068d7a8777b82924 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Fri, 7 Oct 2022 19:19:11 +0300 Subject: [PATCH 01/14] GenTree::GetLayout field FIELDs --- src/coreclr/jit/gentree.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index b1c35b5350ba25..08c7866fcdf34c 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -592,6 +592,7 @@ ClassLayout* GenTree::GetLayout(Compiler* compiler) const { assert(varTypeIsStruct(TypeGet())); + CORINFO_CLASS_HANDLE structHnd = NO_CLASS_HANDLE; switch (OperGet()) { case GT_LCL_VAR: @@ -605,11 +606,18 @@ ClassLayout* GenTree::GetLayout(Compiler* compiler) const return AsBlk()->GetLayout(); case GT_MKREFANY: - return compiler->typGetObjLayout(compiler->impGetRefAnyClass()); + structHnd = compiler->impGetRefAnyClass(); + break; + + case GT_FIELD: + compiler->eeGetFieldType(AsField()->gtFldHnd, &structHnd); + break; default: unreached(); } + + return compiler->typGetObjLayout(structHnd); } //----------------------------------------------------------- From a756ccbf949bb8828971ea80dc9338b5f8a96cfe Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Fri, 7 Oct 2022 19:22:17 +0300 Subject: [PATCH 02/14] Do not create invalid IR when replacing promoted fields Instead, create constructions like "OBJ(ADDR(LCL_VAR))" that block morphing can recognize on its own. --- src/coreclr/jit/lclmorph.cpp | 2 +- src/coreclr/jit/morph.cpp | 152 ++++++++++++----------------------- 2 files changed, 53 insertions(+), 101 deletions(-) diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp index 56dfa69e0f5d14..e7419908731fda 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/src/coreclr/jit/lclmorph.cpp @@ -1098,7 +1098,7 @@ class LocalAddressVisitor final : public GenTreeVisitor assert(node->OperIs(GT_FIELD)); // TODO-Cleanup: Move fgMorphStructField implementation here, it's not used anywhere else. m_compiler->fgMorphStructField(node, user); - m_stmtModified |= node->OperIs(GT_LCL_VAR); + m_stmtModified |= !node->OperIs(GT_FIELD); } //------------------------------------------------------------------------ diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 9bbd6fe7f9e3d7..20577414353c98 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -16189,123 +16189,75 @@ void Compiler::fgMorphStructField(GenTree* tree, GenTree* parent) unsigned lclNum = obj->AsLclVarCommon()->GetLclNum(); const LclVarDsc* varDsc = lvaGetDesc(lclNum); - if (varTypeIsStruct(obj)) + if (varDsc->lvPromoted) { - if (varDsc->lvPromoted) + // Promoted struct + unsigned fldOffset = field->gtFldOffset; + unsigned fieldLclNum = lvaGetFieldLocal(varDsc, fldOffset); + + if (fieldLclNum == BAD_VAR_NUM) { - // Promoted struct - unsigned fldOffset = field->gtFldOffset; - unsigned fieldLclIndex = lvaGetFieldLocal(varDsc, fldOffset); + // Access a promoted struct's field with an offset that doesn't correspond to any field. + // It can happen if the struct was cast to another struct with different offsets. + return; + } - if (fieldLclIndex == BAD_VAR_NUM) - { - // Access a promoted struct's field with an offset that doesn't correspond to any field. - // It can happen if the struct was cast to another struct with different offsets. - return; - } + const LclVarDsc* fieldDsc = lvaGetDesc(fieldLclNum); + var_types fieldType = fieldDsc->TypeGet(); + GenTree* lclVarNode = nullptr; + GenTreeFlags lclVarFlags = field->gtFlags & (GTF_NODE_MASK | GTF_DONT_CSE); - const LclVarDsc* fieldDsc = lvaGetDesc(fieldLclIndex); - var_types fieldType = fieldDsc->TypeGet(); + assert(fieldType != TYP_STRUCT); // promoted LCL_VAR can't have a struct type. + if ((field->TypeGet() == fieldType) || parent->OperIs(GT_ADDR)) + { + lclVarNode = field; - assert(fieldType != TYP_STRUCT); // promoted LCL_VAR can't have a struct type. - if (tree->TypeGet() != fieldType) + if (parent->OperIs(GT_ASG) && (parent->AsOp()->gtOp1 == field)) { - if (tree->TypeGet() != TYP_STRUCT) - { - // This is going to be an incorrect instruction promotion. - // For example when we try to read int as long. - return; - } - - if (field->gtFldHnd != fieldDsc->lvFieldHnd) - { - CORINFO_CLASS_HANDLE fieldTreeClass = nullptr, fieldDscClass = nullptr; - - CorInfoType fieldTreeType = info.compCompHnd->getFieldType(field->gtFldHnd, &fieldTreeClass); - CorInfoType fieldDscType = info.compCompHnd->getFieldType(fieldDsc->lvFieldHnd, &fieldDscClass); - if (fieldTreeType != fieldDscType || fieldTreeClass != fieldDscClass) - { - // Access the promoted field with a different class handle, can't check that types match. - return; - } - // Access the promoted field as a field of a non-promoted struct with the same class handle. - } - else - { - // As we already checked this above, we must have a tree with a TYP_STRUCT type - // - assert(tree->TypeGet() == TYP_STRUCT); - - // The field tree accesses it as a struct, but the promoted LCL_VAR field - // says that it has another type. This happens when struct promotion unwraps - // a single field struct to get to its ultimate type. - // - // Note that currently, we cannot have a promoted LCL_VAR field with a struct type. - // - // This mismatch in types can lead to problems for some parent node type like GT_RETURN. - // So we check the parent node and only allow this optimization when we have - // a GT_ADDR or a GT_ASG. - // - // Note that for a GT_ASG we have to do some additional work, - // see below after the SetOper(GT_LCL_VAR) - // - if (!parent->OperIs(GT_ADDR, GT_ASG)) - { - // Don't transform other operations such as GT_RETURN - // - return; - } -#ifdef DEBUG - // This is an additional DEBUG-only sanity check - // - assert(structPromotionHelper != nullptr); - structPromotionHelper->CheckRetypedAsScalar(field->gtFldHnd, fieldType); -#endif // DEBUG - } + lclVarFlags |= GTF_VAR_DEF; } - - tree->SetOper(GT_LCL_VAR); - tree->AsLclVarCommon()->SetLclNum(fieldLclIndex); - tree->gtType = fieldType; - tree->gtFlags &= GTF_NODE_MASK; // Note: that clears all flags except `GTF_COLON_COND`. - - if (parent->gtOper == GT_ASG) + else if (parent->OperIs(GT_ADDR)) { - // If we are changing the left side of an assignment, we need to set - // these two flags: - // - if (parent->AsOp()->gtOp1 == tree) - { - tree->gtFlags |= GTF_VAR_DEF; - tree->gtFlags |= GTF_DONT_CSE; - } - + // TODO-Bug: delete this quirk. + lclVarFlags &= ~GTF_DONT_CSE; + } + } + else // Here we will turn "FIELD(ADDR(LCL_VAR))" into "OBJ/IND(ADDR(LCL_VAR))". + { + if (field->TypeIs(TYP_STRUCT)) + { +#ifdef DEBUG // Promotion of struct containing struct fields where the field // is a struct with a single pointer sized scalar type field: in // this case struct promotion uses the type of the underlying // scalar field as the type of struct field instead of recursively - // promoting. This can lead to a case where we have a block-asgn - // with its RHS replaced with a scalar type. Mark RHS value as - // DONT_CSE so that assertion prop will not do const propagation. - // The reason this is required is that if RHS of a block-asg is a - // constant, then it is interpreted as init-block incorrectly. - // - // TODO - This can also be avoided if we implement recursive struct - // promotion, tracked by #10019. - if (varTypeIsStruct(parent) && parent->AsOp()->gtOp2 == tree && !varTypeIsStruct(tree)) - { - tree->gtFlags |= GTF_DONT_CSE; - } + // promoting. + structPromotionHelper->CheckRetypedAsScalar(field->gtFldHnd, fieldType); +#endif // DEBUG + ClassLayout* layout = field->GetLayout(this); + field->SetOper(GT_OBJ); + field->AsBlk()->SetLayout(layout); + field->AsBlk()->gtBlkOpKind = GenTreeBlk::BlkOpKindInvalid; +#ifndef JIT32_GCENCODER + field->AsBlk()->gtBlkOpGcUnsafe = false; +#endif // !JIT32_GCENCODER } -#ifdef DEBUG - if (verbose) + else { - printf("Replacing the field in promoted struct with local var V%02u\n", fieldLclIndex); + field->SetOper(GT_IND); } -#endif // DEBUG + + lclVarNode = obj; } + + lclVarNode->SetOper(GT_LCL_VAR); + lclVarNode->AsLclVarCommon()->SetLclNum(fieldLclNum); + lclVarNode->gtType = fieldType; + lclVarNode->gtFlags = lclVarFlags; + + JITDUMP("Replacing the field in promoted struct with local var V%02u\n", fieldLclNum); } - else + else if (!varTypeIsStruct(obj)) { // Normed struct // A "normed struct" is a struct that the VM tells us is a basic type. This can only happen if From d41deeac283dd2914dc9ffde92fe57af2cb0d097 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Fri, 7 Oct 2022 19:54:17 +0300 Subject: [PATCH 03/14] Delete lvFieldHnd --- src/coreclr/jit/compiler.h | 2 -- src/coreclr/jit/lclvars.cpp | 1 - 2 files changed, 3 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 1987fc90a92923..84631288faf068 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -1009,8 +1009,6 @@ class LclVarDsc return structHnd; } - CORINFO_FIELD_HANDLE lvFieldHnd; // field handle for promoted struct fields - private: ClassLayout* m_layout; // layout info for structs diff --git a/src/coreclr/jit/lclvars.cpp b/src/coreclr/jit/lclvars.cpp index 35867c43601c15..1ee1e18ca79db8 100644 --- a/src/coreclr/jit/lclvars.cpp +++ b/src/coreclr/jit/lclvars.cpp @@ -2496,7 +2496,6 @@ void Compiler::StructPromotionHelper::PromoteStructVar(unsigned lclNum) fieldVarDsc->lvType = pFieldInfo->fldType; fieldVarDsc->lvExactSize = pFieldInfo->fldSize; fieldVarDsc->lvIsStructField = true; - fieldVarDsc->lvFieldHnd = pFieldInfo->fldHnd; fieldVarDsc->lvFldOffset = pFieldInfo->fldOffset; fieldVarDsc->lvFldOrdinal = pFieldInfo->fldOrdinal; fieldVarDsc->lvParentLcl = lclNum; From 9764377d64b030e7a176ea25c52088a3615e48c2 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Fri, 7 Oct 2022 20:06:31 +0300 Subject: [PATCH 04/14] GenTree::GetLayout - CALL/COMMA --- src/coreclr/jit/gentree.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 08c7866fcdf34c..36bf700e7cd3e9 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -590,7 +590,7 @@ void Compiler::fgWalkAllTreesPre(fgWalkPreFn* visitor, void* pCallBackData) // ClassLayout* GenTree::GetLayout(Compiler* compiler) const { - assert(varTypeIsStruct(TypeGet())); + assert(TypeIs(TYP_STRUCT)); CORINFO_CLASS_HANDLE structHnd = NO_CLASS_HANDLE; switch (OperGet()) @@ -605,6 +605,9 @@ ClassLayout* GenTree::GetLayout(Compiler* compiler) const case GT_BLK: return AsBlk()->GetLayout(); + case GT_COMMA: + return AsOp()->gtOp2->GetLayout(compiler); + case GT_MKREFANY: structHnd = compiler->impGetRefAnyClass(); break; @@ -613,6 +616,10 @@ ClassLayout* GenTree::GetLayout(Compiler* compiler) const compiler->eeGetFieldType(AsField()->gtFldHnd, &structHnd); break; + case GT_CALL: + structHnd = AsCall()->gtRetClsHnd; + break; + default: unreached(); } From 4f2e763c10c113c2ca8b1fb366a9c721ba046337 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Tue, 19 Jul 2022 19:51:15 +0300 Subject: [PATCH 05/14] GenTreeHWIntrinsic::GetLayout --- src/coreclr/jit/gentree.cpp | 37 +++++++++++++++++++++++++++++++++++++ src/coreclr/jit/gentree.h | 2 ++ 2 files changed, 39 insertions(+) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 36bf700e7cd3e9..67be36f828a8a0 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -22769,6 +22769,43 @@ bool GenTreeHWIntrinsic::OperIsMemoryLoadOrStore() const return OperIsMemoryLoad() || OperIsMemoryStore(); } +//------------------------------------------------------------------------ +// GetLayout: Get the layout for this TYP_STRUCT HWI node. +// +// Arguments: +// compiler - The Compiler instance +// +// Return Value: +// The struct layout for the node. +// +// Notes: +// Currently, this method synthesizes block layouts, since there is not +// enough space in the GenTreeHWIntrinsic node to store a proper layout +// pointer or number. +// +ClassLayout* GenTreeHWIntrinsic::GetLayout(Compiler* compiler) const +{ + assert(TypeIs(TYP_STRUCT)); + + switch (GetHWIntrinsicId()) + { +#ifdef TARGET_ARM64 + case NI_AdvSimd_Arm64_LoadPairScalarVector64: + case NI_AdvSimd_Arm64_LoadPairScalarVector64NonTemporal: + case NI_AdvSimd_Arm64_LoadPairVector64: + case NI_AdvSimd_Arm64_LoadPairVector64NonTemporal: + return compiler->typGetBlkLayout(16); + + case NI_AdvSimd_Arm64_LoadPairVector128: + case NI_AdvSimd_Arm64_LoadPairVector128NonTemporal: + return compiler->typGetBlkLayout(32); +#endif // TARGET_ARM64 + + default: + unreached(); + } +} + NamedIntrinsic GenTreeHWIntrinsic::GetHWIntrinsicId() const { NamedIntrinsic id = gtHWIntrinsicId; diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 70abd0a98ebb19..966e7e7a9b4f14 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -6250,6 +6250,8 @@ struct GenTreeHWIntrinsic : public GenTreeJitIntrinsic unsigned GetResultOpNumForFMA(GenTree* use, GenTree* op1, GenTree* op2, GenTree* op3); + ClassLayout* GetLayout(Compiler* compiler) const; + NamedIntrinsic GetHWIntrinsicId() const; //--------------------------------------------------------------------------------------- From 9b05ca1f712bbff2d54ac6bfa179447f9166b6bb Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Fri, 7 Oct 2022 20:07:23 +0300 Subject: [PATCH 06/14] Add an assert that LHS and RHS match --- src/coreclr/jit/morphblock.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/coreclr/jit/morphblock.cpp b/src/coreclr/jit/morphblock.cpp index 288f55657cc4d0..4f9e6e445cce64 100644 --- a/src/coreclr/jit/morphblock.cpp +++ b/src/coreclr/jit/morphblock.cpp @@ -795,6 +795,14 @@ void MorphCopyBlockHelper::PrepareSrc() m_srcLclNum = m_srcLclNode->GetLclNum(); m_srcVarDsc = m_comp->lvaGetDesc(m_srcLclNum); } + + // Verify that the types on the LHS and RHS match. + assert(m_dst->TypeGet() == m_src->TypeGet()); + // TODO-1stClassStructs: delete the "!IND" condition once "IND" nodes are no more. + if (m_dst->TypeIs(TYP_STRUCT) && !m_src->OperIs(GT_IND)) + { + assert(ClassLayout::AreCompatible(m_blockLayout, m_src->GetLayout(m_comp))); + } } // TrySpecialCases: check special cases that require special transformations. From 274b78c2d59a72625ea93ae7e0478f807f88379f Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Fri, 7 Oct 2022 20:31:49 +0300 Subject: [PATCH 07/14] Fix assert violation And simplify the code... --- src/coreclr/jit/gentree.cpp | 7 ++++++- src/coreclr/jit/morph.cpp | 17 +++-------------- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 67be36f828a8a0..e1f919d3f2ecc9 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -590,7 +590,7 @@ void Compiler::fgWalkAllTreesPre(fgWalkPreFn* visitor, void* pCallBackData) // ClassLayout* GenTree::GetLayout(Compiler* compiler) const { - assert(TypeIs(TYP_STRUCT)); + assert(varTypeIsStruct(TypeGet())); CORINFO_CLASS_HANDLE structHnd = NO_CLASS_HANDLE; switch (OperGet()) @@ -608,6 +608,11 @@ ClassLayout* GenTree::GetLayout(Compiler* compiler) const case GT_COMMA: return AsOp()->gtOp2->GetLayout(compiler); +#ifdef FEATURE_HW_INTRINSICS + case GT_HWINTRINSIC: + return AsHWIntrinsic()->GetLayout(compiler); +#endif // FEATURE_HW_INTRINSICS + case GT_MKREFANY: structHnd = compiler->impGetRefAnyClass(); break; diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 20577414353c98..f123bd49e3b568 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -16408,18 +16408,6 @@ PhaseStatus Compiler::fgRetypeImplicitByRefArgs() { madeChanges = true; - unsigned size; - - if (varDsc->lvSize() > REGSIZE_BYTES) - { - size = varDsc->lvSize(); - } - else - { - CORINFO_CLASS_HANDLE typeHnd = varDsc->GetStructHnd(); - size = info.compCompHnd->getClassSize(typeHnd); - } - if (varDsc->lvPromoted) { // This implicit-by-ref was promoted; create a new temp to represent the @@ -16498,8 +16486,9 @@ PhaseStatus Compiler::fgRetypeImplicitByRefArgs() fgEnsureFirstBBisScratch(); GenTree* lhs = gtNewLclvNode(newLclNum, varDsc->lvType); // RHS is an indirection (using GT_OBJ) off the parameter. - GenTree* addr = gtNewLclvNode(lclNum, TYP_BYREF); - GenTree* rhs = new (this, GT_BLK) GenTreeBlk(GT_BLK, TYP_STRUCT, addr, typGetBlkLayout(size)); + GenTree* addr = gtNewLclvNode(lclNum, TYP_BYREF); + GenTree* rhs = (varDsc->TypeGet() == TYP_STRUCT) ? gtNewObjNode(varDsc->GetLayout(), addr) + : gtNewIndir(varDsc->TypeGet(), addr); GenTree* assign = gtNewAssignNode(lhs, rhs); fgNewStmtAtBeg(fgFirstBB, assign); } From 2c46495f20b8b072d4adef25329059c5e4a4030d Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Fri, 7 Oct 2022 22:54:16 +0300 Subject: [PATCH 08/14] Delete RetypedAsScalarFieldsMap Without handle equality, the assert no longer holds, for example: ```cs private StructWithInt Problem(StructWithInt b, StructWithInt a) { a = ((StructWithStructWithInt*)&b)->StructWithInt; return a; } ``` --- src/coreclr/jit/compiler.h | 10 ---------- src/coreclr/jit/lclvars.cpp | 35 ++--------------------------------- src/coreclr/jit/lower.cpp | 3 +-- src/coreclr/jit/morph.cpp | 13 +++++-------- 4 files changed, 8 insertions(+), 53 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 84631288faf068..0eda4686973268 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -3356,10 +3356,6 @@ class Compiler structPromotionInfo.typeHnd = NO_CLASS_HANDLE; } -#ifdef DEBUG - void CheckRetypedAsScalar(CORINFO_FIELD_HANDLE fieldHnd, var_types requestedType); -#endif // DEBUG - private: bool CanPromoteStructVar(unsigned lclNum); bool ShouldPromoteStructVar(unsigned lclNum); @@ -3374,12 +3370,6 @@ class Compiler private: Compiler* compiler; lvaStructPromotionInfo structPromotionInfo; - -#ifdef DEBUG - typedef JitHashTable, var_types> - RetypedAsScalarFieldsMap; - RetypedAsScalarFieldsMap retypedFieldsMap; -#endif // DEBUG }; StructPromotionHelper* structPromotionHelper; diff --git a/src/coreclr/jit/lclvars.cpp b/src/coreclr/jit/lclvars.cpp index 1ee1e18ca79db8..fde7559a388c64 100644 --- a/src/coreclr/jit/lclvars.cpp +++ b/src/coreclr/jit/lclvars.cpp @@ -1692,12 +1692,7 @@ bool Compiler::lvaFieldOffsetCmp::operator()(const lvaStructFieldInfo& field1, c // Arguments: // compiler - pointer to a compiler to get access to an allocator, compHandle etc. // -Compiler::StructPromotionHelper::StructPromotionHelper(Compiler* compiler) - : compiler(compiler) - , structPromotionInfo() -#ifdef DEBUG - , retypedFieldsMap(compiler->getAllocator(CMK_DebugOnly)) -#endif // DEBUG +Compiler::StructPromotionHelper::StructPromotionHelper(Compiler* compiler) : compiler(compiler), structPromotionInfo() { } @@ -1730,27 +1725,6 @@ bool Compiler::StructPromotionHelper::TryPromoteStructVar(unsigned lclNum) return false; } -#ifdef DEBUG -//-------------------------------------------------------------------------------------------- -// CheckRetypedAsScalar - check that the fldType for this fieldHnd was retyped as requested type. -// -// Arguments: -// fieldHnd - the field handle; -// requestedType - as which type the field was accessed; -// -// Notes: -// For example it can happen when such struct A { struct B { long c } } is compiled and we access A.B.c, -// it could look like "GT_FIELD struct B.c -> ADDR -> GT_FIELD struct A.B -> ADDR -> LCL_VAR A" , but -// "GT_FIELD struct A.B -> ADDR -> LCL_VAR A" can be promoted to "LCL_VAR long A.B" and then -// there is type mistmatch between "GT_FIELD struct B.c" and "LCL_VAR long A.B". -// -void Compiler::StructPromotionHelper::CheckRetypedAsScalar(CORINFO_FIELD_HANDLE fieldHnd, var_types requestedType) -{ - assert(retypedFieldsMap.Lookup(fieldHnd)); - assert(retypedFieldsMap[fieldHnd] == requestedType); -} -#endif // DEBUG - //-------------------------------------------------------------------------------------------- // CanPromoteStructType - checks if the struct type can be promoted. // @@ -2309,9 +2283,6 @@ Compiler::lvaStructFieldInfo Compiler::StructPromotionHelper::GetFieldInfo(CORIN { fieldInfo.fldType = compiler->getSIMDTypeForSize(simdSize); fieldInfo.fldSize = simdSize; -#ifdef DEBUG - retypedFieldsMap.Set(fieldInfo.fldHnd, fieldInfo.fldType, RetypedAsScalarFieldsMap::Overwrite); -#endif // DEBUG } } } @@ -2412,9 +2383,7 @@ bool Compiler::StructPromotionHelper::TryPromoteStructField(lvaStructFieldInfo& // (tracked by #10019). fieldInfo.fldType = fieldVarType; fieldInfo.fldSize = fieldSize; -#ifdef DEBUG - retypedFieldsMap.Set(fieldInfo.fldHnd, fieldInfo.fldType, RetypedAsScalarFieldsMap::Overwrite); -#endif // DEBUG + return true; } diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index bb8d617104e7a6..6aac258faa5530 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -3340,8 +3340,7 @@ void Lowering::LowerRet(GenTreeUnOp* ret) bool constStructInit = retVal->IsConstInitVal(); bool implicitCastFromSameOrBiggerSize = (genTypeSize(retActualType) <= genTypeSize(retValActualType)); - // This could happen if we have retyped op1 as a primitive type during struct promotion, - // check `retypedFieldsMap` for details. + // This could happen if we have retyped op1 as a primitive type during struct promotion. bool actualTypesMatch = (retActualType == retValActualType); assert(actualTypesMatch || constStructInit || implicitCastFromSameOrBiggerSize); diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index f123bd49e3b568..0d0501ec6596bd 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -16224,16 +16224,13 @@ void Compiler::fgMorphStructField(GenTree* tree, GenTree* parent) } else // Here we will turn "FIELD(ADDR(LCL_VAR))" into "OBJ/IND(ADDR(LCL_VAR))". { + // This type mismatch is somewhat common due to how we retype fields of struct type that + // recursively simplify down to a primitive. E. g. for "struct { struct { int a } A, B }", + // the promoted local would look like "{ int a, B }", while the IR would contain "FIELD" + // nodes for the outer struct "A". + // if (field->TypeIs(TYP_STRUCT)) { -#ifdef DEBUG - // Promotion of struct containing struct fields where the field - // is a struct with a single pointer sized scalar type field: in - // this case struct promotion uses the type of the underlying - // scalar field as the type of struct field instead of recursively - // promoting. - structPromotionHelper->CheckRetypedAsScalar(field->gtFldHnd, fieldType); -#endif // DEBUG ClassLayout* layout = field->GetLayout(this); field->SetOper(GT_OBJ); field->AsBlk()->SetLayout(layout); From c32fc2785fbcc6ff45b8194b09572dc5b6bdb3e7 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Fri, 7 Oct 2022 22:55:45 +0300 Subject: [PATCH 09/14] Delete a bit of code Dead / unncessary. No diffs. --- src/coreclr/jit/morph.cpp | 59 --------------------------------------- 1 file changed, 59 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 0d0501ec6596bd..d2af94d85a29f3 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -16254,58 +16254,6 @@ void Compiler::fgMorphStructField(GenTree* tree, GenTree* parent) JITDUMP("Replacing the field in promoted struct with local var V%02u\n", fieldLclNum); } - else if (!varTypeIsStruct(obj)) - { - // Normed struct - // A "normed struct" is a struct that the VM tells us is a basic type. This can only happen if - // the struct contains a single element, and that element is 4 bytes (on x64 it can also be 8 - // bytes). Normally, the type of the local var and the type of GT_FIELD are equivalent. However, - // there is one extremely rare case where that won't be true. An enum type is a special value type - // that contains exactly one element of a primitive integer type (that, for CLS programs is named - // "value__"). The VM tells us that a local var of that enum type is the primitive type of the - // enum's single field. It turns out that it is legal for IL to access this field using ldflda or - // ldfld. For example: - // - // .class public auto ansi sealed mynamespace.e_t extends [mscorlib]System.Enum - // { - // .field public specialname rtspecialname int16 value__ - // .field public static literal valuetype mynamespace.e_t one = int16(0x0000) - // } - // .method public hidebysig static void Main() cil managed - // { - // .locals init (valuetype mynamespace.e_t V_0) - // ... - // ldloca.s V_0 - // ldflda int16 mynamespace.e_t::value__ - // ... - // } - // - // Normally, compilers will not generate the ldflda, since it is superfluous. - // - // In the example, the lclVar is short, but the JIT promotes all trees using this local to the - // "actual type", that is, INT. But the GT_FIELD is still SHORT. So, in the case of a type - // mismatch like this, don't do this morphing. The local var may end up getting marked as - // address taken, and the appropriate SHORT load will be done from memory in that case. - - if (tree->TypeGet() == obj->TypeGet()) - { - tree->ChangeOper(GT_LCL_VAR); - tree->AsLclVarCommon()->SetLclNum(lclNum); - tree->gtFlags &= GTF_NODE_MASK; - - if ((parent->gtOper == GT_ASG) && (parent->AsOp()->gtOp1 == tree)) - { - tree->gtFlags |= GTF_VAR_DEF; - tree->gtFlags |= GTF_DONT_CSE; - } -#ifdef DEBUG - if (verbose) - { - printf("Replacing the field in normed struct with local var V%02u\n", lclNum); - } -#endif // DEBUG - } - } } } @@ -16369,13 +16317,6 @@ void Compiler::fgMorphLocalField(GenTree* tree, GenTree* parent) #endif // DEBUG } } - else if (varTypeIsSIMD(varDsc) && (genTypeSize(tree->TypeGet()) == genTypeSize(varDsc))) - { - assert(tree->AsLclFld()->GetLclOffs() == 0); - tree->gtType = varDsc->TypeGet(); - tree->ChangeOper(GT_LCL_VAR); - JITDUMP("Replacing GT_LCL_FLD of struct with local var V%02u\n", lclNum); - } } } From 0c2aa914981e432c76d55323c4d2fa3e32d596cb Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Fri, 7 Oct 2022 23:05:16 +0300 Subject: [PATCH 10/14] Fold promoted locals in local morph --- src/coreclr/jit/lclmorph.cpp | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp index e7419908731fda..1109710b89233e 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/src/coreclr/jit/lclmorph.cpp @@ -933,8 +933,6 @@ class LocalAddressVisitor final : public GenTreeVisitor indir->AsLclFld()->SetLayout(indirLayout); lclNode = indir->AsLclVarCommon(); - // Promoted locals aren't currently handled here so partial access can't be - // later be transformed into a LCL_VAR and the variable cannot be enregistered. m_compiler->lvaSetVarDoNotEnregister(lclNum DEBUGARG(DoNotEnregisterReason::LocalField)); break; @@ -1000,13 +998,6 @@ class LocalAddressVisitor final : public GenTreeVisitor if (indir->TypeGet() != TYP_STRUCT) { - if (varDsc->lvPromoted) - { - // TODO-ADDR: support promoted locals here by moving the promotion morphing - // from pre-order to post-order. - return IndirTransform::None; - } - if (indir->TypeGet() == varDsc->TypeGet()) { return IndirTransform::LclVar; From 75769d93144a905eadf11ab5d412d3d388b60748 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Fri, 7 Oct 2022 23:50:54 +0300 Subject: [PATCH 11/14] Support GT_IND in MorphStructField This is significantly simpler than moving the promotion logic to post-order because we don't need to fiddle with the ref counting process and complexities of intermediate states, e. g. "IND(ADDR(FIELD(ADDR(LCL_VAR))))" can be naturally turned into "BITCAST(LCL_VAR)" like this. --- src/coreclr/jit/compiler.h | 2 +- src/coreclr/jit/lclmorph.cpp | 17 ++++---- src/coreclr/jit/morph.cpp | 75 +++++++++++++++++++++--------------- 3 files changed, 53 insertions(+), 41 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 0eda4686973268..f8b41f358fee36 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -5855,7 +5855,7 @@ class Compiler #endif PhaseStatus fgPromoteStructs(); - void fgMorphStructField(GenTree* tree, GenTree* parent); + bool fgMorphStructField(GenTree* indir, GenTree* parent); void fgMorphLocalField(GenTree* tree, GenTree* parent); // Reset the refCount for implicit byrefs. diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp index 1109710b89233e..4a27aa268a1de0 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/src/coreclr/jit/lclmorph.cpp @@ -381,7 +381,7 @@ class LocalAddressVisitor final : public GenTreeVisitor { GenTree* const node = *use; - if (node->OperIs(GT_FIELD)) + if (node->OperIs(GT_IND, GT_FIELD)) { MorphStructField(node, user); } @@ -1073,23 +1073,22 @@ class LocalAddressVisitor final : public GenTreeVisitor } //------------------------------------------------------------------------ - // MorphStructField: Replaces a GT_FIELD based promoted/normed struct field access - // (e.g. FIELD(ADDR(LCL_VAR))) with a GT_LCL_VAR that references the struct field. + // MorphStructField: Reduces indirect access to a promoted local (e.g. + // FIELD(ADDR(LCL_VAR))) to a GT_LCL_VAR that references the struct field. // // Arguments: - // node - the GT_FIELD node + // node - the GT_IND/GT_FIELD node // user - the node that uses the field // // Notes: - // This does not do anything if the field access does not denote - // a promoted/normed struct field. + // This does not do anything if the access does not denote a promoted + // struct field. // void MorphStructField(GenTree* node, GenTree* user) { - assert(node->OperIs(GT_FIELD)); + assert(node->OperIs(GT_IND, GT_FIELD)); // TODO-Cleanup: Move fgMorphStructField implementation here, it's not used anywhere else. - m_compiler->fgMorphStructField(node, user); - m_stmtModified |= !node->OperIs(GT_FIELD); + m_stmtModified |= m_compiler->fgMorphStructField(node, user); } //------------------------------------------------------------------------ diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index d2af94d85a29f3..84f5602d9d4bbe 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -16173,75 +16173,85 @@ PhaseStatus Compiler::fgPromoteStructs() return madeChanges ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING; } -void Compiler::fgMorphStructField(GenTree* tree, GenTree* parent) +bool Compiler::fgMorphStructField(GenTree* indir, GenTree* parent) { - noway_assert(tree->OperGet() == GT_FIELD); + assert(indir->OperIs(GT_IND, GT_FIELD)); - GenTreeField* field = tree->AsField(); - GenTree* objRef = field->GetFldObj(); - GenTree* obj = ((objRef != nullptr) && (objRef->gtOper == GT_ADDR)) ? objRef->AsOp()->gtOp1 : nullptr; - noway_assert((tree->gtFlags & GTF_GLOB_REF) || ((obj != nullptr) && (obj->gtOper == GT_LCL_VAR))); - - /* Is this an instance data member? */ + GenTree* objRef = indir->AsUnOp()->gtOp1; + GenTree* obj = ((objRef != nullptr) && objRef->OperIs(GT_ADDR)) ? objRef->AsOp()->gtOp1 : nullptr; + if (indir->OperIs(GT_FIELD)) + { + noway_assert(((indir->gtFlags & GTF_GLOB_REF) != 0) || ((obj != nullptr) && obj->OperIs(GT_LCL_VAR))); + } - if ((obj != nullptr) && (obj->gtOper == GT_LCL_VAR)) + // TODO-Bug: this code does not pay attention to "GTF_IND_VOLATILE". + if ((obj != nullptr) && obj->OperIs(GT_LCL_VAR)) { unsigned lclNum = obj->AsLclVarCommon()->GetLclNum(); const LclVarDsc* varDsc = lvaGetDesc(lclNum); if (varDsc->lvPromoted) { - // Promoted struct - unsigned fldOffset = field->gtFldOffset; - unsigned fieldLclNum = lvaGetFieldLocal(varDsc, fldOffset); + unsigned fieldOffset = indir->OperIs(GT_FIELD) ? indir->AsField()->gtFldOffset : 0; + unsigned fieldLclNum = lvaGetFieldLocal(varDsc, fieldOffset); if (fieldLclNum == BAD_VAR_NUM) { // Access a promoted struct's field with an offset that doesn't correspond to any field. // It can happen if the struct was cast to another struct with different offsets. - return; + return false; } const LclVarDsc* fieldDsc = lvaGetDesc(fieldLclNum); var_types fieldType = fieldDsc->TypeGet(); GenTree* lclVarNode = nullptr; - GenTreeFlags lclVarFlags = field->gtFlags & (GTF_NODE_MASK | GTF_DONT_CSE); + GenTreeFlags lclVarFlags = indir->gtFlags & (GTF_NODE_MASK | GTF_DONT_CSE); assert(fieldType != TYP_STRUCT); // promoted LCL_VAR can't have a struct type. - if ((field->TypeGet() == fieldType) || parent->OperIs(GT_ADDR)) + if ((indir->TypeGet() == fieldType) || ((parent != nullptr) && parent->OperIs(GT_ADDR))) { - lclVarNode = field; + lclVarNode = indir; - if (parent->OperIs(GT_ASG) && (parent->AsOp()->gtOp1 == field)) - { - lclVarFlags |= GTF_VAR_DEF; - } - else if (parent->OperIs(GT_ADDR)) + if (parent != nullptr) { - // TODO-Bug: delete this quirk. - lclVarFlags &= ~GTF_DONT_CSE; + if (parent->OperIs(GT_ASG) && (parent->AsOp()->gtOp1 == indir)) + { + lclVarFlags |= GTF_VAR_DEF; + } + else if (parent->OperIs(GT_ADDR)) + { + // TODO-ADDR: delete this quirk. + lclVarFlags &= ~GTF_DONT_CSE; + } } } - else // Here we will turn "FIELD(ADDR(LCL_VAR))" into "OBJ/IND(ADDR(LCL_VAR))". + else // Here we will turn "FIELD/IND(ADDR(LCL_VAR))" into "OBJ/IND(ADDR(LCL_VAR))". { // This type mismatch is somewhat common due to how we retype fields of struct type that // recursively simplify down to a primitive. E. g. for "struct { struct { int a } A, B }", // the promoted local would look like "{ int a, B }", while the IR would contain "FIELD" // nodes for the outer struct "A". // - if (field->TypeIs(TYP_STRUCT)) + if (indir->TypeIs(TYP_STRUCT)) { - ClassLayout* layout = field->GetLayout(this); - field->SetOper(GT_OBJ); - field->AsBlk()->SetLayout(layout); - field->AsBlk()->gtBlkOpKind = GenTreeBlk::BlkOpKindInvalid; + // TODO-1stClassStructs: delete this once "IND" nodes are no more. + if (indir->OperIs(GT_IND)) + { + // We do not have a layout for this node. + return false; + } + + ClassLayout* layout = indir->GetLayout(this); + indir->SetOper(GT_OBJ); + indir->AsBlk()->SetLayout(layout); + indir->AsBlk()->gtBlkOpKind = GenTreeBlk::BlkOpKindInvalid; #ifndef JIT32_GCENCODER - field->AsBlk()->gtBlkOpGcUnsafe = false; + indir->AsBlk()->gtBlkOpGcUnsafe = false; #endif // !JIT32_GCENCODER } else { - field->SetOper(GT_IND); + indir->SetOper(GT_IND); } lclVarNode = obj; @@ -16253,8 +16263,11 @@ void Compiler::fgMorphStructField(GenTree* tree, GenTree* parent) lclVarNode->gtFlags = lclVarFlags; JITDUMP("Replacing the field in promoted struct with local var V%02u\n", fieldLclNum); + return true; } } + + return false; } void Compiler::fgMorphLocalField(GenTree* tree, GenTree* parent) From 1706f113e32d851209d275208ed74e96bc53cbc0 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Sat, 8 Oct 2022 00:01:37 +0300 Subject: [PATCH 12/14] More code removal --- src/coreclr/jit/morph.cpp | 33 --------------------------------- 1 file changed, 33 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 84f5602d9d4bbe..d1f97ad2ade20d 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -11194,39 +11194,6 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA { foldAndReturnTemp = true; } - else if (temp->OperIsLocal()) - { - unsigned lclNum = temp->AsLclVarCommon()->GetLclNum(); - LclVarDsc* varDsc = lvaGetDesc(lclNum); - - // We will try to optimize when we have a promoted struct promoted with a zero lvFldOffset - if (varDsc->lvPromoted && (varDsc->lvFldOffset == 0)) - { - noway_assert(varTypeIsStruct(varDsc)); - - // We will try to optimize when we have a single field struct that is being struct promoted - if (varDsc->lvFieldCnt == 1) - { - unsigned lclNumFld = varDsc->lvFieldLclStart; - // just grab the promoted field - LclVarDsc* fieldVarDsc = lvaGetDesc(lclNumFld); - - // Also make sure that the tree type matches the fieldVarType and that it's lvFldOffset - // is same as that of tree's offset. - if (fieldVarDsc->TypeGet() == typ && - (fieldVarDsc->lvFldOffset == temp->AsLclVarCommon()->GetLclOffs())) - { - // We can just use the existing promoted field LclNum - temp->AsLclVarCommon()->SetLclNum(lclNumFld); - temp->gtType = fieldVarDsc->TypeGet(); - - foldAndReturnTemp = true; - } - } - } - // Otherwise will will fold this into a GT_LCL_FLD below - // where we check (temp != nullptr) - } } else { From fdb0c19c4daa75d94dbbea3de7fcef3b285e951b Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Sat, 8 Oct 2022 00:15:35 +0300 Subject: [PATCH 13/14] Move "fgMorphStructField" to local morph --- src/coreclr/jit/compiler.h | 1 - src/coreclr/jit/lclmorph.cpp | 99 +++++++++++++++++++++++++++++++++--- src/coreclr/jit/morph.cpp | 97 ----------------------------------- 3 files changed, 93 insertions(+), 104 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index f8b41f358fee36..22b959505b87d9 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -5855,7 +5855,6 @@ class Compiler #endif PhaseStatus fgPromoteStructs(); - bool fgMorphStructField(GenTree* indir, GenTree* parent); void fgMorphLocalField(GenTree* tree, GenTree* parent); // Reset the refCount for implicit byrefs. diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp index 4a27aa268a1de0..0124ad9ddb8cde 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/src/coreclr/jit/lclmorph.cpp @@ -1077,18 +1077,105 @@ class LocalAddressVisitor final : public GenTreeVisitor // FIELD(ADDR(LCL_VAR))) to a GT_LCL_VAR that references the struct field. // // Arguments: - // node - the GT_IND/GT_FIELD node - // user - the node that uses the field + // indir - the GT_IND/GT_FIELD node + // user - the node that uses the field // // Notes: // This does not do anything if the access does not denote a promoted // struct field. // - void MorphStructField(GenTree* node, GenTree* user) + void MorphStructField(GenTree* indir, GenTree* user) { - assert(node->OperIs(GT_IND, GT_FIELD)); - // TODO-Cleanup: Move fgMorphStructField implementation here, it's not used anywhere else. - m_stmtModified |= m_compiler->fgMorphStructField(node, user); + assert(indir->OperIs(GT_IND, GT_FIELD)); + + GenTree* objRef = indir->AsUnOp()->gtOp1; + GenTree* obj = ((objRef != nullptr) && objRef->OperIs(GT_ADDR)) ? objRef->AsOp()->gtOp1 : nullptr; + if (indir->OperIs(GT_FIELD)) + { + noway_assert(((indir->gtFlags & GTF_GLOB_REF) != 0) || ((obj != nullptr) && obj->OperIs(GT_LCL_VAR))); + } + + // TODO-Bug: this code does not pay attention to "GTF_IND_VOLATILE". + if ((obj != nullptr) && obj->OperIs(GT_LCL_VAR)) + { + const LclVarDsc* varDsc = m_compiler->lvaGetDesc(obj->AsLclVarCommon()); + + if (varDsc->lvPromoted) + { + unsigned fieldOffset = indir->OperIs(GT_FIELD) ? indir->AsField()->gtFldOffset : 0; + unsigned fieldLclNum = m_compiler->lvaGetFieldLocal(varDsc, fieldOffset); + + if (fieldLclNum == BAD_VAR_NUM) + { + // Access a promoted struct's field with an offset that doesn't correspond to any field. + // It can happen if the struct was cast to another struct with different offsets. + return; + } + + const LclVarDsc* fieldDsc = m_compiler->lvaGetDesc(fieldLclNum); + var_types fieldType = fieldDsc->TypeGet(); + GenTree* lclVarNode = nullptr; + GenTreeFlags lclVarFlags = indir->gtFlags & (GTF_NODE_MASK | GTF_DONT_CSE); + + assert(fieldType != TYP_STRUCT); // promoted LCL_VAR can't have a struct type. + if ((indir->TypeGet() == fieldType) || ((user != nullptr) && user->OperIs(GT_ADDR))) + { + lclVarNode = indir; + + if (user != nullptr) + { + if (user->OperIs(GT_ASG) && (user->AsOp()->gtOp1 == indir)) + { + lclVarFlags |= GTF_VAR_DEF; + } + else if (user->OperIs(GT_ADDR)) + { + // TODO-ADDR: delete this quirk. + lclVarFlags &= ~GTF_DONT_CSE; + } + } + } + else // Here we will turn "FIELD/IND(ADDR(LCL_VAR))" into "OBJ/IND(ADDR(LCL_VAR))". + { + // This type mismatch is somewhat common due to how we retype fields of struct type that + // recursively simplify down to a primitive. E. g. for "struct { struct { int a } A, B }", + // the promoted local would look like "{ int a, B }", while the IR would contain "FIELD" + // nodes for the outer struct "A". + // + if (indir->TypeIs(TYP_STRUCT)) + { + // TODO-1stClassStructs: delete this once "IND" nodes are no more. + if (indir->OperIs(GT_IND)) + { + // We do not have a layout for this node. + return; + } + + ClassLayout* layout = indir->GetLayout(m_compiler); + indir->SetOper(GT_OBJ); + indir->AsBlk()->SetLayout(layout); + indir->AsBlk()->gtBlkOpKind = GenTreeBlk::BlkOpKindInvalid; +#ifndef JIT32_GCENCODER + indir->AsBlk()->gtBlkOpGcUnsafe = false; +#endif // !JIT32_GCENCODER + } + else + { + indir->SetOper(GT_IND); + } + + lclVarNode = obj; + } + + lclVarNode->SetOper(GT_LCL_VAR); + lclVarNode->AsLclVarCommon()->SetLclNum(fieldLclNum); + lclVarNode->gtType = fieldType; + lclVarNode->gtFlags = lclVarFlags; + + JITDUMP("Replacing the field in promoted struct with local var V%02u\n", fieldLclNum); + m_stmtModified = true; + } + } } //------------------------------------------------------------------------ diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index d1f97ad2ade20d..85d88b07240d2c 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -16140,103 +16140,6 @@ PhaseStatus Compiler::fgPromoteStructs() return madeChanges ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING; } -bool Compiler::fgMorphStructField(GenTree* indir, GenTree* parent) -{ - assert(indir->OperIs(GT_IND, GT_FIELD)); - - GenTree* objRef = indir->AsUnOp()->gtOp1; - GenTree* obj = ((objRef != nullptr) && objRef->OperIs(GT_ADDR)) ? objRef->AsOp()->gtOp1 : nullptr; - if (indir->OperIs(GT_FIELD)) - { - noway_assert(((indir->gtFlags & GTF_GLOB_REF) != 0) || ((obj != nullptr) && obj->OperIs(GT_LCL_VAR))); - } - - // TODO-Bug: this code does not pay attention to "GTF_IND_VOLATILE". - if ((obj != nullptr) && obj->OperIs(GT_LCL_VAR)) - { - unsigned lclNum = obj->AsLclVarCommon()->GetLclNum(); - const LclVarDsc* varDsc = lvaGetDesc(lclNum); - - if (varDsc->lvPromoted) - { - unsigned fieldOffset = indir->OperIs(GT_FIELD) ? indir->AsField()->gtFldOffset : 0; - unsigned fieldLclNum = lvaGetFieldLocal(varDsc, fieldOffset); - - if (fieldLclNum == BAD_VAR_NUM) - { - // Access a promoted struct's field with an offset that doesn't correspond to any field. - // It can happen if the struct was cast to another struct with different offsets. - return false; - } - - const LclVarDsc* fieldDsc = lvaGetDesc(fieldLclNum); - var_types fieldType = fieldDsc->TypeGet(); - GenTree* lclVarNode = nullptr; - GenTreeFlags lclVarFlags = indir->gtFlags & (GTF_NODE_MASK | GTF_DONT_CSE); - - assert(fieldType != TYP_STRUCT); // promoted LCL_VAR can't have a struct type. - if ((indir->TypeGet() == fieldType) || ((parent != nullptr) && parent->OperIs(GT_ADDR))) - { - lclVarNode = indir; - - if (parent != nullptr) - { - if (parent->OperIs(GT_ASG) && (parent->AsOp()->gtOp1 == indir)) - { - lclVarFlags |= GTF_VAR_DEF; - } - else if (parent->OperIs(GT_ADDR)) - { - // TODO-ADDR: delete this quirk. - lclVarFlags &= ~GTF_DONT_CSE; - } - } - } - else // Here we will turn "FIELD/IND(ADDR(LCL_VAR))" into "OBJ/IND(ADDR(LCL_VAR))". - { - // This type mismatch is somewhat common due to how we retype fields of struct type that - // recursively simplify down to a primitive. E. g. for "struct { struct { int a } A, B }", - // the promoted local would look like "{ int a, B }", while the IR would contain "FIELD" - // nodes for the outer struct "A". - // - if (indir->TypeIs(TYP_STRUCT)) - { - // TODO-1stClassStructs: delete this once "IND" nodes are no more. - if (indir->OperIs(GT_IND)) - { - // We do not have a layout for this node. - return false; - } - - ClassLayout* layout = indir->GetLayout(this); - indir->SetOper(GT_OBJ); - indir->AsBlk()->SetLayout(layout); - indir->AsBlk()->gtBlkOpKind = GenTreeBlk::BlkOpKindInvalid; -#ifndef JIT32_GCENCODER - indir->AsBlk()->gtBlkOpGcUnsafe = false; -#endif // !JIT32_GCENCODER - } - else - { - indir->SetOper(GT_IND); - } - - lclVarNode = obj; - } - - lclVarNode->SetOper(GT_LCL_VAR); - lclVarNode->AsLclVarCommon()->SetLclNum(fieldLclNum); - lclVarNode->gtType = fieldType; - lclVarNode->gtFlags = lclVarFlags; - - JITDUMP("Replacing the field in promoted struct with local var V%02u\n", fieldLclNum); - return true; - } - } - - return false; -} - void Compiler::fgMorphLocalField(GenTree* tree, GenTree* parent) { noway_assert(tree->OperGet() == GT_LCL_FLD); From 1e8654101c91344de8192bfb6544b657ff3101cf Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Sat, 8 Oct 2022 00:35:25 +0300 Subject: [PATCH 14/14] TP tuning With this, we have only a very small regression: Base: 1297463478, Diff: 1297750893, +0.0222% LocalAddressVisitor::MorphStructField : 345099 : NA : 31.52% : +0.0266% LocalAddressVisitor::PreOrderVisit : 224684 : +3.53% : 20.52% : +0.0173% memset : 88591 : +0.99% : 8.09% : +0.0068% Compiler::fgMorphSmpOp : -19170 : -0.06% : 1.75% : -0.0015% Compiler::fgMorphStructField : -367474 : -100.00% : 33.56% : -0.0283% (This is with a dummy field on LclVarDsc) Losing the assert does not seem worrysome as we have an identical one in "fgMorphField". --- src/coreclr/jit/lclmorph.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp index 0124ad9ddb8cde..57d0187cee910c 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/src/coreclr/jit/lclmorph.cpp @@ -1090,13 +1090,9 @@ class LocalAddressVisitor final : public GenTreeVisitor GenTree* objRef = indir->AsUnOp()->gtOp1; GenTree* obj = ((objRef != nullptr) && objRef->OperIs(GT_ADDR)) ? objRef->AsOp()->gtOp1 : nullptr; - if (indir->OperIs(GT_FIELD)) - { - noway_assert(((indir->gtFlags & GTF_GLOB_REF) != 0) || ((obj != nullptr) && obj->OperIs(GT_LCL_VAR))); - } // TODO-Bug: this code does not pay attention to "GTF_IND_VOLATILE". - if ((obj != nullptr) && obj->OperIs(GT_LCL_VAR)) + if ((obj != nullptr) && obj->OperIs(GT_LCL_VAR) && varTypeIsStruct(obj)) { const LclVarDsc* varDsc = m_compiler->lvaGetDesc(obj->AsLclVarCommon());