diff options
author | Sergey Andreenko <seandree@microsoft.com> | 2018-09-26 16:48:13 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-09-26 16:48:13 -0700 |
commit | 24bbc37eb70895b6d7cc9cc95b8453742cdc0cae (patch) | |
tree | f21037ccff1a6e1cbd528c645cc70881dd2f2e18 | |
parent | b1a9f1fdb0ef7a9dbf1659a419a0bf064c38106e (diff) | |
download | coreclr-24bbc37eb70895b6d7cc9cc95b8453742cdc0cae.tar.gz coreclr-24bbc37eb70895b6d7cc9cc95b8453742cdc0cae.tar.bz2 coreclr-24bbc37eb70895b6d7cc9cc95b8453742cdc0cae.zip |
Do not treat a custom layout as overlapping when trying to inline a struct method. (#20141)
* Do not treat custom layout as overlapping when trying to inline struct method.
That hack was added 4 years ago with independent struct promotion for parameters.
I was not able to find any related issues.
This change produces asm diffs because it allows us to inline more (lvaCanPromoteStructVar is a multiplier for
inlineIsProfitable parameter).
For System.Private.CoreLib we have total bytes of diff: 294 (0.01% of base).
For example, we started to inline methods from 'System.Threading.Tasks.ValueTask' that has '[StructLayout(LayoutKind.Auto)]'.
* Always sort fields in lcCanPromoteStructType.
It will be optimized away in the future commits.
* Delete the argument that is no longer used.
* Fix variable name according to jit-coding-conventions.
Rename 'StructPromotionInfo' to 'structPromotionInfo'.
-rw-r--r-- | src/jit/compiler.h | 9 | ||||
-rw-r--r-- | src/jit/importer.cpp | 2 | ||||
-rw-r--r-- | src/jit/lclvars.cpp | 83 |
3 files changed, 38 insertions, 56 deletions
diff --git a/src/jit/compiler.h b/src/jit/compiler.h index 77a6ecfc57..920ae9a880 100644 --- a/src/jit/compiler.h +++ b/src/jit/compiler.h @@ -2987,12 +2987,11 @@ public: }; static int __cdecl lvaFieldOffsetCmp(const void* field1, const void* field2); - void lvaCanPromoteStructType(CORINFO_CLASS_HANDLE typeHnd, - lvaStructPromotionInfo* StructPromotionInfo, - bool sortFields); - void lvaCanPromoteStructVar(unsigned lclNum, lvaStructPromotionInfo* StructPromotionInfo); + void lvaCanPromoteStructType(CORINFO_CLASS_HANDLE typeHnd, lvaStructPromotionInfo* structPromotionInfo); + void lvaCanPromoteStructVar(unsigned lclNum, lvaStructPromotionInfo* structPromotionInfo); + bool lvaShouldPromoteStructVar(unsigned lclNum, lvaStructPromotionInfo* structPromotionInfo); - void lvaPromoteStructVar(unsigned lclNum, lvaStructPromotionInfo* StructPromotionInfo); + void lvaPromoteStructVar(unsigned lclNum, lvaStructPromotionInfo* structPromotionInfo); #if !defined(_TARGET_64BIT_) void lvaPromoteLongVars(); #endif // !defined(_TARGET_64BIT_) diff --git a/src/jit/importer.cpp b/src/jit/importer.cpp index 43fa84840c..db098f25e0 100644 --- a/src/jit/importer.cpp +++ b/src/jit/importer.cpp @@ -17744,7 +17744,7 @@ void Compiler::impMakeDiscretionaryInlineObservations(InlineInfo* pInlineInfo, I if ((info.compClassAttr & CORINFO_FLG_VALUECLASS) != 0) { lvaStructPromotionInfo structPromotionInfo; - lvaCanPromoteStructType(info.compClassHnd, &structPromotionInfo, false); + lvaCanPromoteStructType(info.compClassHnd, &structPromotionInfo); if (structPromotionInfo.canPromote) { inlineResult->Note(InlineObservation::CALLEE_CLASS_PROMOTABLE); diff --git a/src/jit/lclvars.cpp b/src/jit/lclvars.cpp index 544da413eb..9587451bee 100644 --- a/src/jit/lclvars.cpp +++ b/src/jit/lclvars.cpp @@ -1482,13 +1482,11 @@ int __cdecl Compiler::lvaFieldOffsetCmp(const void* field1, const void* field2) /***************************************************************************** * Is this type promotable? */ -void Compiler::lvaCanPromoteStructType(CORINFO_CLASS_HANDLE typeHnd, - lvaStructPromotionInfo* StructPromotionInfo, - bool sortFields) +void Compiler::lvaCanPromoteStructType(CORINFO_CLASS_HANDLE typeHnd, lvaStructPromotionInfo* structPromotionInfo) { assert(eeIsValueClass(typeHnd)); - if (typeHnd != StructPromotionInfo->typeHnd) + if (typeHnd != structPromotionInfo->typeHnd) { // sizeof(double) represents the size of the largest primitive type that we can struct promote. // In the future this may be changing to XMM_REGSIZE_BYTES. @@ -1515,8 +1513,8 @@ void Compiler::lvaCanPromoteStructType(CORINFO_CLASS_HANDLE typeHnd, bool customLayout = false; bool containsGCpointers = false; - StructPromotionInfo->typeHnd = typeHnd; - StructPromotionInfo->canPromote = false; + structPromotionInfo->typeHnd = typeHnd; + structPromotionInfo->canPromote = false; unsigned structSize = info.compCompHnd->getClassSize(typeHnd); if (structSize > MaxOffset) @@ -1530,23 +1528,11 @@ void Compiler::lvaCanPromoteStructType(CORINFO_CLASS_HANDLE typeHnd, return; // struct must have between 1 and MAX_NumOfFieldsInPromotableStruct fields } - StructPromotionInfo->fieldCnt = (BYTE)fieldCnt; + structPromotionInfo->fieldCnt = (BYTE)fieldCnt; DWORD typeFlags = info.compCompHnd->getClassAttribs(typeHnd); - bool treatAsOverlapping = StructHasOverlappingFields(typeFlags); - -#if 1 // TODO-Cleanup: Consider removing this entire #if block in the future - - // This method has two callers. The one in Importer.cpp passes `sortFields == false` and the other passes - // `sortFields == true`. This is a workaround that leaves the inlining behavior the same as before while still - // performing extra struct promotion when compiling the method. - if (!sortFields) // the condition "!sortFields" really means "we are inlining" - { - treatAsOverlapping = StructHasCustomLayout(typeFlags); - } -#endif - - if (treatAsOverlapping) + bool overlappingFields = StructHasOverlappingFields(typeFlags); + if (overlappingFields) { return; } @@ -1573,7 +1559,7 @@ void Compiler::lvaCanPromoteStructType(CORINFO_CLASS_HANDLE typeHnd, for (BYTE ordinal = 0; ordinal < fieldCnt; ++ordinal) { - lvaStructFieldInfo* pFieldInfo = &StructPromotionInfo->fields[ordinal]; + lvaStructFieldInfo* pFieldInfo = &structPromotionInfo->fields[ordinal]; pFieldInfo->fldHnd = info.compCompHnd->getFieldInClass(typeHnd, ordinal); unsigned fldOffset = info.compCompHnd->getFieldOffset(pFieldInfo->fldHnd); @@ -1749,19 +1735,16 @@ void Compiler::lvaCanPromoteStructType(CORINFO_CLASS_HANDLE typeHnd, } // Cool, this struct is promotable. - StructPromotionInfo->canPromote = true; - StructPromotionInfo->requiresScratchVar = requiresScratchVar; - StructPromotionInfo->containsHoles = containsHoles; - StructPromotionInfo->customLayout = customLayout; + structPromotionInfo->canPromote = true; + structPromotionInfo->requiresScratchVar = requiresScratchVar; + structPromotionInfo->containsHoles = containsHoles; + structPromotionInfo->customLayout = customLayout; - if (sortFields) - { - // Sort the fields according to the increasing order of the field offset. - // This is needed because the fields need to be pushed on stack (when referenced - // as a struct) in order. - qsort(StructPromotionInfo->fields, StructPromotionInfo->fieldCnt, sizeof(*StructPromotionInfo->fields), - lvaFieldOffsetCmp); - } + // Sort the fields according to the increasing order of the field offset. + // This is needed because the fields need to be pushed on stack (when referenced + // as a struct) in order. + qsort(structPromotionInfo->fields, structPromotionInfo->fieldCnt, sizeof(*structPromotionInfo->fields), + lvaFieldOffsetCmp); } else { @@ -1774,7 +1757,7 @@ void Compiler::lvaCanPromoteStructType(CORINFO_CLASS_HANDLE typeHnd, /***************************************************************************** * Is this struct type local variable promotable? */ -void Compiler::lvaCanPromoteStructVar(unsigned lclNum, lvaStructPromotionInfo* StructPromotionInfo) +void Compiler::lvaCanPromoteStructVar(unsigned lclNum, lvaStructPromotionInfo* structPromotionInfo) { noway_assert(lclNum < lvaCount); @@ -1789,7 +1772,7 @@ void Compiler::lvaCanPromoteStructVar(unsigned lclNum, lvaStructPromotionInfo* S if (varDsc->lvIsUsedInSIMDIntrinsic()) { JITDUMP(" struct promotion of V%02u is disabled because lvIsUsedInSIMDIntrinsic()\n", lclNum); - StructPromotionInfo->canPromote = false; + structPromotionInfo->canPromote = false; return; } @@ -1798,7 +1781,7 @@ void Compiler::lvaCanPromoteStructVar(unsigned lclNum, lvaStructPromotionInfo* S if (varDsc->lvIsParam && compGSReorderStackLayout) { JITDUMP(" struct promotion of V%02u is disabled because lvIsParam and compGSReorderStackLayout\n", lclNum); - StructPromotionInfo->canPromote = false; + structPromotionInfo->canPromote = false; return; } @@ -1810,7 +1793,7 @@ void Compiler::lvaCanPromoteStructVar(unsigned lclNum, lvaStructPromotionInfo* S if (varDsc->lvIsHfaRegArg()) { JITDUMP(" struct promotion of V%02u is disabled because lvIsHfaRegArg()\n", lclNum); - StructPromotionInfo->canPromote = false; + structPromotionInfo->canPromote = false; return; } @@ -1818,7 +1801,7 @@ void Compiler::lvaCanPromoteStructVar(unsigned lclNum, lvaStructPromotionInfo* S if (varDsc->lvIsMultiRegArg) { JITDUMP(" struct promotion of V%02u is disabled because lvIsMultiRegArg\n", lclNum); - StructPromotionInfo->canPromote = false; + structPromotionInfo->canPromote = false; return; } #endif @@ -1826,12 +1809,12 @@ void Compiler::lvaCanPromoteStructVar(unsigned lclNum, lvaStructPromotionInfo* S if (varDsc->lvIsMultiRegRet) { JITDUMP(" struct promotion of V%02u is disabled because lvIsMultiRegRet\n", lclNum); - StructPromotionInfo->canPromote = false; + structPromotionInfo->canPromote = false; return; } CORINFO_CLASS_HANDLE typeHnd = varDsc->lvVerTypeInfo.GetClassHandle(); - lvaCanPromoteStructType(typeHnd, StructPromotionInfo, true); + lvaCanPromoteStructType(typeHnd, structPromotionInfo); } //-------------------------------------------------------------------------------------------- @@ -1938,21 +1921,21 @@ bool Compiler::lvaShouldPromoteStructVar(unsigned lclNum, lvaStructPromotionInfo /***************************************************************************** * Promote a struct type local */ -void Compiler::lvaPromoteStructVar(unsigned lclNum, lvaStructPromotionInfo* StructPromotionInfo) +void Compiler::lvaPromoteStructVar(unsigned lclNum, lvaStructPromotionInfo* structPromotionInfo) { LclVarDsc* varDsc = &lvaTable[lclNum]; // We should never see a reg-sized non-field-addressed struct here. noway_assert(!varDsc->lvRegStruct); - noway_assert(StructPromotionInfo->canPromote); - noway_assert(StructPromotionInfo->typeHnd == varDsc->lvVerTypeInfo.GetClassHandle()); + noway_assert(structPromotionInfo->canPromote); + noway_assert(structPromotionInfo->typeHnd == varDsc->lvVerTypeInfo.GetClassHandle()); - varDsc->lvFieldCnt = StructPromotionInfo->fieldCnt; + varDsc->lvFieldCnt = structPromotionInfo->fieldCnt; varDsc->lvFieldLclStart = lvaCount; varDsc->lvPromoted = true; - varDsc->lvContainsHoles = StructPromotionInfo->containsHoles; - varDsc->lvCustomLayout = StructPromotionInfo->customLayout; + varDsc->lvContainsHoles = structPromotionInfo->containsHoles; + varDsc->lvCustomLayout = structPromotionInfo->customLayout; #ifdef DEBUG // Don't change the source to a TYP_BLK either. @@ -1962,13 +1945,13 @@ void Compiler::lvaPromoteStructVar(unsigned lclNum, lvaStructPromotionInfo* Stru #ifdef DEBUG if (verbose) { - printf("\nPromoting struct local V%02u (%s):", lclNum, eeGetClassName(StructPromotionInfo->typeHnd)); + printf("\nPromoting struct local V%02u (%s):", lclNum, eeGetClassName(structPromotionInfo->typeHnd)); } #endif - for (unsigned index = 0; index < StructPromotionInfo->fieldCnt; ++index) + for (unsigned index = 0; index < structPromotionInfo->fieldCnt; ++index) { - lvaStructFieldInfo* pFieldInfo = &StructPromotionInfo->fields[index]; + lvaStructFieldInfo* pFieldInfo = &structPromotionInfo->fields[index]; if (varTypeIsFloating(pFieldInfo->fldType) || varTypeIsSIMD(pFieldInfo->fldType)) { |