Skip to content

Commit c156e4b

Browse files
JosephTremouletJosephTremoulet
committed
Morph implicit byrefs after struct promotion
Change the phase ordering with respect to address-exposed analysis, struct promotion, and implicit byref rewriting. This is done to subject implicit byref parameters to struct promotion; when an implicit byref is to be promoted, generate a new local of the struct type, promote it, and insert code at method entry to initialize the new local by struct-copy from the parameter. As part of the reordering, run address-exposed analysis before implicit byref rewriting, so that the results of address-exposed analysis can be consulted in determining whether promoting each implicit byref is beneficial (in light of the cost of the struct copy), and effectively avoid promoting those which are not deemed beneficial. This change treats all implicit byref promotions as NOT beneficial; it reorders the phases but produces no asm diffs. A subsequent change will identify beneficial implicit byref promotions. To make this work, the implicit byref rewriting happens in a few stages: - fgMarkImplicitByRefArgs now does nothing more than flag which args are implicit byrefs, and runs before struct promotion. - fgRetypeImplicitByRefArgs actually retypes the parameters (from struct to pointer), and decides which promotions are beneficial. It inserts the initializing struct copies for beneficial ones, and annotates non-beneficial ones so their appearances will be rewritten via the pointer parameter. It runs after struct promotion and address-escape analysis. - fgMorphImplicitByRefArgs is moved out of fgMarkAddressExposedLocals, and into fgMorphBlocks. It rewrites any implicit byref parameters which have not been promoted to have the extra indirection at all their references. - fgMarkDemotedImplicitByRefArgs runs after fgMorphBlocks, and cleans up some of the LclVarDsc annotations used to communicate across the previous pieces. Commit migrated from dotnet/coreclr@a6a8bd2
1 parent 57a3586 commit c156e4b

File tree

5 files changed

+565
-131
lines changed

5 files changed

+565
-131
lines changed

‎src/coreclr/src/jit/compiler.h‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -714,6 +714,8 @@ class LclVarDsc
714714

715715
CORINFO_CLASS_HANDLE lvClassHnd; // class handle for the local, or null if not known
716716

717+
CORINFO_FIELD_HANDLE lvFieldHnd; // field handle for promoted struct fields
718+
717719
BYTE* lvGcLayout; // GC layout info for structs
718720

719721
#if ASSERTION_PROP
@@ -4865,8 +4867,22 @@ class Compiler
48654867
void fgPromoteStructs();
48664868
fgWalkResult fgMorphStructField(GenTreePtr tree, fgWalkData* fgWalkPre);
48674869
fgWalkResult fgMorphLocalField(GenTreePtr tree, fgWalkData* fgWalkPre);
4870+
4871+
// Identify which parameters are implicit byrefs, and flag their LclVarDscs.
48684872
void fgMarkImplicitByRefArgs();
4869-
bool fgMorphImplicitByRefArgs(GenTree** pTree, fgWalkData* fgWalkPre);
4873+
4874+
// Change implicit byrefs' types from struct to pointer, and for any that were
4875+
// promoted, create new promoted struct temps.
4876+
void fgRetypeImplicitByRefArgs();
4877+
4878+
// Rewrite appearances of implicit byrefs (manifest the implied additional level of indirection).
4879+
bool fgMorphImplicitByRefArgs(GenTreePtr tree);
4880+
GenTreePtr fgMorphImplicitByRefArgs(GenTreePtr tree, bool isAddr);
4881+
4882+
// Clear up annotations for any struct promotion temps created for implicit byrefs that
4883+
// wound up unused (due e.g. to being address-exposed and not worth promoting).
4884+
void fgMarkDemotedImplicitByRefArgs();
4885+
48704886
static fgWalkPreFn fgMarkAddrTakenLocalsPreCB;
48714887
static fgWalkPostFn fgMarkAddrTakenLocalsPostCB;
48724888
void fgMarkAddressExposedLocals();

‎src/coreclr/src/jit/compphases.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,12 @@ CompPhaseNameMacro(PHASE_IMPORTATION, "Importation",
2727
CompPhaseNameMacro(PHASE_POST_IMPORT, "Post-import", "POST-IMP", false, -1, false)
2828
CompPhaseNameMacro(PHASE_MORPH_INIT, "Morph - Init", "MOR-INIT" ,false, -1, false)
2929
CompPhaseNameMacro(PHASE_MORPH_INLINE, "Morph - Inlining", "MOR-INL", false, -1, true)
30-
CompPhaseNameMacro(PHASE_MORPH_IMPBYREF, "Morph - ByRefs", "MOR-BYREF",false, -1, false)
3130
CompPhaseNameMacro(PHASE_EMPTY_TRY, "Remove empty try", "EMPTYTRY", false, -1, false)
3231
CompPhaseNameMacro(PHASE_EMPTY_FINALLY, "Remove empty finally", "EMPTYFIN", false, -1, false)
3332
CompPhaseNameMacro(PHASE_MERGE_FINALLY_CHAINS, "Merge callfinally chains", "MRGCFCHN", false, -1, false)
3433
CompPhaseNameMacro(PHASE_CLONE_FINALLY, "Clone finally", "CLONEFIN", false, -1, false)
3534
CompPhaseNameMacro(PHASE_STR_ADRLCL, "Morph - Structs/AddrExp", "MOR-STRAL",false, -1, false)
35+
CompPhaseNameMacro(PHASE_MORPH_IMPBYREF, "Morph - ByRefs", "MOR-BYREF",false, -1, false)
3636
CompPhaseNameMacro(PHASE_MORPH_GLOBAL, "Morph - Global", "MOR-GLOB", false, -1, false)
3737
CompPhaseNameMacro(PHASE_MORPH_END, "Morph - Finish", "MOR-END", false, -1, true)
3838
CompPhaseNameMacro(PHASE_GS_COOKIE, "GS Cookie", "GS-COOK", false, -1, false)

‎src/coreclr/src/jit/gentree.cpp‎

Lines changed: 50 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -10424,8 +10424,17 @@ void Compiler::gtDispNode(GenTreePtr tree, IndentStack* indentStack, __in __in_z
1042410424
}
1042510425
else if (varDsc->lvPromoted)
1042610426
{
10427-
assert(varTypeIsPromotable(varDsc));
10428-
printf("(P)"); // Promoted struct
10427+
if (varTypeIsPromotable(varDsc))
10428+
{
10429+
printf("(P)"); // Promoted struct
10430+
}
10431+
else
10432+
{
10433+
// Promoted implicit by-refs can have this state during
10434+
// global morph while they are being rewritten
10435+
assert(fgGlobalMorph);
10436+
printf("(P?!)"); // Promoted struct
10437+
}
1042910438
}
1043010439
}
1043110440

@@ -11010,44 +11019,51 @@ void Compiler::gtDispLeaf(GenTree* tree, IndentStack* indentStack)
1101011019

1101111020
if (varDsc->lvPromoted)
1101211021
{
11013-
assert(varTypeIsPromotable(varDsc) || varDsc->lvUnusedStruct);
11014-
11015-
CORINFO_CLASS_HANDLE typeHnd = varDsc->lvVerTypeInfo.GetClassHandle();
11016-
CORINFO_FIELD_HANDLE fldHnd;
11017-
11018-
for (unsigned i = varDsc->lvFieldLclStart; i < varDsc->lvFieldLclStart + varDsc->lvFieldCnt; ++i)
11022+
if (!varTypeIsPromotable(varDsc) && !varDsc->lvUnusedStruct)
1101911023
{
11020-
LclVarDsc* fieldVarDsc = &lvaTable[i];
11021-
const char* fieldName;
11022-
#if !defined(_TARGET_64BIT_)
11023-
if (varTypeIsLong(varDsc))
11024+
// Promoted implicit byrefs can get in this state while they are being rewritten
11025+
// in global morph.
11026+
assert(fgGlobalMorph);
11027+
}
11028+
else
11029+
{
11030+
CORINFO_CLASS_HANDLE typeHnd = varDsc->lvVerTypeInfo.GetClassHandle();
11031+
CORINFO_FIELD_HANDLE fldHnd;
11032+
11033+
for (unsigned i = varDsc->lvFieldLclStart; i < varDsc->lvFieldLclStart + varDsc->lvFieldCnt; ++i)
1102411034
{
11025-
fieldName = (i == 0) ? "lo" : "hi";
11026-
}
11027-
else
11035+
LclVarDsc* fieldVarDsc = &lvaTable[i];
11036+
const char* fieldName;
11037+
#if !defined(_TARGET_64BIT_)
11038+
if (varTypeIsLong(varDsc))
11039+
{
11040+
fieldName = (i == 0) ? "lo" : "hi";
11041+
}
11042+
else
1102811043
#endif // !defined(_TARGET_64BIT_)
11029-
{
11030-
fldHnd = info.compCompHnd->getFieldInClass(typeHnd, fieldVarDsc->lvFldOrdinal);
11031-
fieldName = eeGetFieldName(fldHnd);
11032-
}
11044+
{
11045+
fldHnd = info.compCompHnd->getFieldInClass(typeHnd, fieldVarDsc->lvFldOrdinal);
11046+
fieldName = eeGetFieldName(fldHnd);
11047+
}
1103311048

11034-
printf("\n");
11035-
printf(" ");
11036-
printIndent(indentStack);
11037-
printf(" %-6s V%02u.%s (offs=0x%02x) -> ", varTypeName(fieldVarDsc->TypeGet()),
11038-
tree->gtLclVarCommon.gtLclNum, fieldName, fieldVarDsc->lvFldOffset);
11039-
gtDispLclVar(i);
11049+
printf("\n");
11050+
printf(" ");
11051+
printIndent(indentStack);
11052+
printf(" %-6s V%02u.%s (offs=0x%02x) -> ", varTypeName(fieldVarDsc->TypeGet()),
11053+
tree->gtLclVarCommon.gtLclNum, fieldName, fieldVarDsc->lvFldOffset);
11054+
gtDispLclVar(i);
1104011055

11041-
if (fieldVarDsc->lvRegister)
11042-
{
11043-
printf(" ");
11044-
fieldVarDsc->PrintVarReg();
11045-
}
11056+
if (fieldVarDsc->lvRegister)
11057+
{
11058+
printf(" ");
11059+
fieldVarDsc->PrintVarReg();
11060+
}
1104611061

11047-
if (fieldVarDsc->lvTracked && fgLocalVarLivenessDone && // Includes local variable liveness
11048-
((tree->gtFlags & GTF_VAR_DEATH) != 0))
11049-
{
11050-
printf(" (last use)");
11062+
if (fieldVarDsc->lvTracked && fgLocalVarLivenessDone && // Includes local variable liveness
11063+
((tree->gtFlags & GTF_VAR_DEATH) != 0))
11064+
{
11065+
printf(" (last use)");
11066+
}
1105111067
}
1105211068
}
1105311069
}

‎src/coreclr/src/jit/lclvars.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1818,7 +1818,7 @@ bool Compiler::lvaShouldPromoteStructVar(unsigned lclNum, lvaStructPromotionInfo
18181818
shouldPromote = false;
18191819
}
18201820
#endif // _TARGET_AMD64_ || _TARGET_ARM64_
1821-
else if (varDsc->lvIsParam)
1821+
else if (varDsc->lvIsParam && !lvaIsImplicitByRefLocal(lclNum))
18221822
{
18231823
#if FEATURE_MULTIREG_STRUCT_PROMOTE
18241824
// Is this a variable holding a value with exactly two fields passed in
@@ -1924,6 +1924,7 @@ void Compiler::lvaPromoteStructVar(unsigned lclNum, lvaStructPromotionInfo* Stru
19241924
fieldVarDsc->lvType = pFieldInfo->fldType;
19251925
fieldVarDsc->lvExactSize = pFieldInfo->fldSize;
19261926
fieldVarDsc->lvIsStructField = true;
1927+
fieldVarDsc->lvFieldHnd = pFieldInfo->fldHnd;
19271928
fieldVarDsc->lvFldOffset = pFieldInfo->fldOffset;
19281929
fieldVarDsc->lvFldOrdinal = pFieldInfo->fldOrdinal;
19291930
fieldVarDsc->lvParentLcl = lclNum;

0 commit comments

Comments
 (0)