diff options
author | Sergey Andreenko <seandree@microsoft.com> | 2018-10-05 17:07:41 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-10-05 17:07:41 -0700 |
commit | 768911897f0921ef77ef991088359d248cb99613 (patch) | |
tree | 4f18b1e0ca05c63cfeb739e559744aef412d2b85 /src/jit/morph.cpp | |
parent | 457148064c836daa746542f1559b7958d6420c22 (diff) | |
download | coreclr-768911897f0921ef77ef991088359d248cb99613.tar.gz coreclr-768911897f0921ef77ef991088359d248cb99613.tar.bz2 coreclr-768911897f0921ef77ef991088359d248cb99613.zip |
Refactoring of struct promotion code for the future fix. (#20216)
Create StructPromotionHelper.
* Add the check for promoted fields with changed type.
`CanPromoteStructType` can fake a field type if the field type is "structs of a single field of scalar types aligned at their natural boundary.". Add a check in debug that when we morph struct field access we have an expected type in the field handle.
Diffstat (limited to 'src/jit/morph.cpp')
-rw-r--r-- | src/jit/morph.cpp | 102 |
1 files changed, 52 insertions, 50 deletions
diff --git a/src/jit/morph.cpp b/src/jit/morph.cpp index 58c25feb82..f928479750 100644 --- a/src/jit/morph.cpp +++ b/src/jit/morph.cpp @@ -17039,7 +17039,7 @@ void Compiler::fgPromoteStructs() // Loop through the original lvaTable. Looking for struct locals to be promoted. // lvaStructPromotionInfo structPromotionInfo; - bool tooManyLocals = false; + bool tooManyLocalsReported = false; for (unsigned lclNum = 0; lclNum < startLvaCount; lclNum++) { @@ -17057,54 +17057,16 @@ void Compiler::fgPromoteStructs() else if (lvaHaveManyLocals()) { // Print the message first time when we detected this condition - if (!tooManyLocals) + if (!tooManyLocalsReported) { JITDUMP("Stopped promoting struct fields, due to too many locals.\n"); } - tooManyLocals = true; + tooManyLocalsReported = true; } else if (varTypeIsStruct(varDsc)) { - bool shouldPromote; - - lvaCanPromoteStructVar(lclNum, &structPromotionInfo); - if (structPromotionInfo.canPromote) - { - shouldPromote = lvaShouldPromoteStructVar(lclNum, &structPromotionInfo); - } - else - { - shouldPromote = false; - } - -#if 0 - // Often-useful debugging code: if you've narrowed down a struct-promotion problem to a single - // method, this allows you to select a subset of the vars to promote (by 1-based ordinal number). - static int structPromoVarNum = 0; - structPromoVarNum++; - if (atoi(getenv("structpromovarnumlo")) <= structPromoVarNum && structPromoVarNum <= atoi(getenv("structpromovarnumhi"))) -#endif // 0 - - if (shouldPromote) - { - // Promote the this struct local var. - lvaPromoteStructVar(lclNum, &structPromotionInfo); - promotedVar = true; - -#ifdef _TARGET_ARM_ - if (structPromotionInfo.requiresScratchVar) - { - // Ensure that the scratch variable is allocated, in case we - // pass a promoted struct as an argument. - if (lvaPromotedStructAssemblyScratchVar == BAD_VAR_NUM) - { - lvaPromotedStructAssemblyScratchVar = - lvaGrabTempWithImplicitUse(false DEBUGARG("promoted struct assembly scratch var.")); - lvaTable[lvaPromotedStructAssemblyScratchVar].lvType = TYP_I_IMPL; - } - } -#endif // _TARGET_ARM_ - } + assert(structPromotionHelper != nullptr); + promotedVar = structPromotionHelper->TryPromoteStructVar(lclNum); } if (!promotedVar && varDsc->lvIsSIMDType() && !varDsc->lvFieldAccessed) @@ -17115,6 +17077,20 @@ void Compiler::fgPromoteStructs() } } +#ifdef _TARGET_ARM_ + if (structPromotionHelper->GetRequiresScratchVar()) + { + // Ensure that the scratch variable is allocated, in case we + // pass a promoted struct as an argument. + if (lvaPromotedStructAssemblyScratchVar == BAD_VAR_NUM) + { + lvaPromotedStructAssemblyScratchVar = + lvaGrabTempWithImplicitUse(false DEBUGARG("promoted struct assembly scratch var.")); + lvaTable[lvaPromotedStructAssemblyScratchVar].lvType = TYP_I_IMPL; + } + } +#endif // _TARGET_ARM_ + #ifdef DEBUG if (verbose) { @@ -17128,29 +17104,55 @@ void Compiler::fgMorphStructField(GenTree* tree, GenTree* parent) { noway_assert(tree->OperGet() == GT_FIELD); - GenTree* objRef = tree->gtField.gtFldObj; - GenTree* obj = ((objRef != nullptr) && (objRef->gtOper == GT_ADDR)) ? objRef->gtOp.gtOp1 : nullptr; + GenTreeField* field = tree->AsField(); + GenTree* objRef = field->gtFldObj; + GenTree* obj = ((objRef != nullptr) && (objRef->gtOper == GT_ADDR)) ? objRef->gtOp.gtOp1 : nullptr; noway_assert((tree->gtFlags & GTF_GLOB_REF) || ((obj != nullptr) && (obj->gtOper == GT_LCL_VAR))); /* Is this an instance data member? */ if ((obj != nullptr) && (obj->gtOper == GT_LCL_VAR)) { - unsigned lclNum = obj->gtLclVarCommon.gtLclNum; - LclVarDsc* varDsc = &lvaTable[lclNum]; + unsigned lclNum = obj->gtLclVarCommon.gtLclNum; + const LclVarDsc* varDsc = &lvaTable[lclNum]; if (varTypeIsStruct(obj)) { if (varDsc->lvPromoted) { // Promoted struct - unsigned fldOffset = tree->gtField.gtFldOffset; + unsigned fldOffset = field->gtFldOffset; unsigned fieldLclIndex = lvaGetFieldLocal(varDsc, fldOffset); noway_assert(fieldLclIndex != BAD_VAR_NUM); + const LclVarDsc* fieldDsc = &lvaTable[fieldLclIndex]; + var_types fieldType = fieldDsc->TypeGet(); + + assert(fieldType != TYP_STRUCT); // promoted LCL_VAR can't have a struct type. + if (tree->TypeGet() != fieldType) + { + if (tree->TypeGet() != TYP_STRUCT) + { + // This is going to be an incorrect instruction promotion. + // For example when we try to read int as long. + // Tolerate this case now to keep no asm difs in this PR. + // TODO make a return. + } +#ifdef DEBUG + if (tree->TypeGet() == TYP_STRUCT) + { + // The field tree accesses it as a struct, but the promoted lcl var for the field + // says that it has another type. It can happen only if struct promotion faked + // field type for a struct of single field of scalar type aligned at their natural boundary. + assert(structPromotionHelper != nullptr); + structPromotionHelper->CheckRetypedAsScalar(field->gtFldHnd, fieldType); + } +#endif // DEBUG + } + tree->SetOper(GT_LCL_VAR); tree->gtLclVarCommon.SetLclNum(fieldLclIndex); - tree->gtType = lvaTable[fieldLclIndex].TypeGet(); + tree->gtType = fieldType; tree->gtFlags &= GTF_NODE_MASK; tree->gtFlags &= ~GTF_GLOB_REF; @@ -17173,7 +17175,7 @@ void Compiler::fgMorphStructField(GenTree* tree, GenTree* parent) // constant, then it is interpreted as init-block incorrectly. // // TODO - This can also be avoided if we implement recursive struct - // promotion. + // promotion, tracked by #10019. if (varTypeIsStruct(parent) && parent->gtOp.gtOp2 == tree && !varTypeIsStruct(tree)) { tree->gtFlags |= GTF_DONT_CSE; |