Skip to content

Commit b583258

Browse files
authored
JIT: run local struct to field update in morph both pre and post order for returns (#75304)
* JIT: run local assertion prop before morphing return of local struct to field In some rare cases (currently only jitstress) morph may miss out in updating a return of a "struct that can be replaced by its field" with the field, because we run local assertion prop afterward we check for this update, and assertion prop, and it may change the local from one that could not be updated into one that could be. So do a special run of assertion prop before this update, so that the local we analyze for possible update is same the one we'll be using in the end. I tried moving all of this to post-order so local assertion prop only ran once but that lead to more diffs and some regressions. Fixes #74900. * revise to try the update pre and post with ap in between
1 parent 6214022 commit b583258

File tree

2 files changed

+56
-26
lines changed

2 files changed

+56
-26
lines changed

src/coreclr/jit/compiler.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5749,6 +5749,7 @@ class Compiler
57495749
GenTree* fgMorphStoreDynBlock(GenTreeStoreDynBlk* tree);
57505750
GenTree* fgMorphForRegisterFP(GenTree* tree);
57515751
GenTree* fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optAssertionPropDone = nullptr);
5752+
void fgTryReplaceStructLocalWithField(GenTree* tree);
57525753
GenTree* fgOptimizeCast(GenTreeCast* cast);
57535754
GenTree* fgOptimizeCastOnAssignment(GenTreeOp* asg);
57545755
GenTree* fgOptimizeEqualityComparisonWithConst(GenTreeOp* cmp);

src/coreclr/jit/morph.cpp

Lines changed: 55 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -10213,32 +10213,8 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA
1021310213
{
1021410214
op1 = fgMorphRetInd(tree->AsUnOp());
1021510215
}
10216-
if (op1->OperIs(GT_LCL_VAR))
10217-
{
10218-
// With a `genReturnBB` this `RETURN(src)` tree will be replaced by a `ASG(genReturnLocal, src)`
10219-
// and `ASG` will be transformed into field by field copy without parent local referencing if
10220-
// possible.
10221-
GenTreeLclVar* lclVar = op1->AsLclVar();
10222-
unsigned lclNum = lclVar->GetLclNum();
10223-
if ((genReturnLocal == BAD_VAR_NUM) || (genReturnLocal == lclNum))
10224-
{
10225-
LclVarDsc* varDsc = lvaGetDesc(lclVar);
10226-
if (varDsc->CanBeReplacedWithItsField(this))
10227-
{
10228-
// We can replace the struct with its only field and allow copy propagation to replace
10229-
// return value that was written as a field.
10230-
unsigned fieldLclNum = varDsc->lvFieldLclStart;
10231-
LclVarDsc* fieldDsc = lvaGetDesc(fieldLclNum);
10232-
10233-
JITDUMP("Replacing an independently promoted local var V%02u with its only field "
10234-
"V%02u for "
10235-
"the return [%06u]\n",
10236-
lclVar->GetLclNum(), fieldLclNum, dspTreeID(tree));
10237-
lclVar->SetLclNum(fieldLclNum);
10238-
lclVar->ChangeType(fieldDsc->lvType);
10239-
}
10240-
}
10241-
}
10216+
10217+
fgTryReplaceStructLocalWithField(op1);
1024210218
}
1024310219

1024410220
// normalize small integer return values
@@ -11652,6 +11628,17 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA
1165211628
}
1165311629
break;
1165411630

11631+
case GT_RETURN:
11632+
11633+
// Retry updating op1 to a field -- assertion
11634+
// prop done when morphing op1 changed the local.
11635+
//
11636+
if (op1 != nullptr)
11637+
{
11638+
fgTryReplaceStructLocalWithField(op1);
11639+
}
11640+
break;
11641+
1165511642
default:
1165611643
break;
1165711644
}
@@ -11696,6 +11683,48 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA
1169611683
return tree;
1169711684
}
1169811685

11686+
//------------------------------------------------------------------------
11687+
// fgTryReplaceStructLocalWithField: see if a struct use can be replaced
11688+
// with an equivalent field use
11689+
//
11690+
// Arguments:
11691+
// tree - tree to examine and possibly modify
11692+
//
11693+
// Notes:
11694+
// Currently only called when the tree parent is a GT_RETURN.
11695+
//
11696+
void Compiler::fgTryReplaceStructLocalWithField(GenTree* tree)
11697+
{
11698+
if (!tree->OperIs(GT_LCL_VAR))
11699+
{
11700+
return;
11701+
}
11702+
11703+
// With a `genReturnBB` this `RETURN(src)` tree will be replaced by a `ASG(genReturnLocal, src)`
11704+
// and `ASG` will be transformed into field by field copy without parent local referencing if
11705+
// possible.
11706+
GenTreeLclVar* lclVar = tree->AsLclVar();
11707+
unsigned lclNum = lclVar->GetLclNum();
11708+
if ((genReturnLocal == BAD_VAR_NUM) || (genReturnLocal == lclNum))
11709+
{
11710+
LclVarDsc* const varDsc = lvaGetDesc(lclVar);
11711+
if (varDsc->CanBeReplacedWithItsField(this))
11712+
{
11713+
// We can replace the struct with its only field and allow copy propagation to replace
11714+
// return value that was written as a field.
11715+
unsigned const fieldLclNum = varDsc->lvFieldLclStart;
11716+
LclVarDsc* const fieldDsc = lvaGetDesc(fieldLclNum);
11717+
11718+
JITDUMP("Replacing an independently promoted local var V%02u with its only field "
11719+
"V%02u for "
11720+
"the return [%06u]\n",
11721+
lclVar->GetLclNum(), fieldLclNum, dspTreeID(tree));
11722+
lclVar->SetLclNum(fieldLclNum);
11723+
lclVar->ChangeType(fieldDsc->lvType);
11724+
}
11725+
}
11726+
}
11727+
1169911728
//------------------------------------------------------------------------
1170011729
// fgOptimizeCast: Optimizes the supplied GT_CAST tree.
1170111730
//

0 commit comments

Comments
 (0)