Skip to content

Commit 161f13d

Browse files
Copy propagation tweaking (#64378)
* Relax the type check The type check is too conservative: it prevents partial definitions from being used in propagation: ``` LCL_FLD V00/1 [X] = { ... }; // Pushed on the stack as a def. USE LCL_VAR V01 // Has the same VN as V00/1, but the type // check prevented it from being replaced. ``` This new version is conservative too, but will do for now as we don't propagate on (most) partial uses. Another reason for this change is that in my upcoming refactoring of copy propagation (that will bring another 0.5% in TP gains), we will no longer have the "defNode" available. * Do not propagate shadowed "this" Ordinarily, shadowed parameters would not be used for propagation anyway, because of the liveness check, but "this" bypasses that checks, and so was used, which is presumably not what we want. Regardless of that, it is also not profitable to propagate "this" in such a situation as it extends its live range and makes the RA unhappy. * NO_SHADOW_COPY -> BAD_VAR_NUM
1 parent 1c80769 commit 161f13d

File tree

3 files changed

+17
-13
lines changed

3 files changed

+17
-13
lines changed

src/coreclr/jit/compiler.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -10893,7 +10893,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
1089310893
struct ShadowParamVarInfo
1089410894
{
1089510895
FixedBitVect* assignGroup; // the closure set of variables whose values depend on each other
10896-
unsigned shadowCopy; // Lcl var num, valid only if not set to NO_SHADOW_COPY
10896+
unsigned shadowCopy; // Lcl var num, if not valid set to BAD_VAR_NUM
1089710897

1089810898
static bool mayNeedShadowCopy(LclVarDsc* varDsc)
1089910899
{

src/coreclr/jit/copyprop.cpp

+12-6
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ void Compiler::optCopyProp(Statement* stmt,
161161
}
162162

163163
if ((gsShadowVarInfo != nullptr) && newLclVarDsc->lvIsParam &&
164-
(gsShadowVarInfo[newLclNum].shadowCopy == lclNum))
164+
(gsShadowVarInfo[newLclNum].shadowCopy != BAD_VAR_NUM))
165165
{
166166
continue;
167167
}
@@ -172,11 +172,6 @@ void Compiler::optCopyProp(Statement* stmt,
172172
continue;
173173
}
174174

175-
if (newLclDefNode->TypeGet() != tree->TypeGet())
176-
{
177-
continue;
178-
}
179-
180175
ValueNum lclDefVN = varDsc->GetPerSsaData(tree->GetSsaNum())->m_vnPair.GetConservative();
181176
if (newLclDefVN != lclDefVN)
182177
{
@@ -226,6 +221,17 @@ void Compiler::optCopyProp(Statement* stmt,
226221
continue;
227222
}
228223

224+
var_types newLclType = newLclVarDsc->TypeGet();
225+
if (!newLclVarDsc->lvNormalizeOnLoad())
226+
{
227+
newLclType = genActualType(newLclType);
228+
}
229+
230+
if (newLclType != tree->TypeGet())
231+
{
232+
continue;
233+
}
234+
229235
#ifdef DEBUG
230236
if (verbose)
231237
{

src/coreclr/jit/gschecks.cpp

+4-6
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,6 @@ void Compiler::gsGSChecksInitCookie()
3232
info.compCompHnd->getGSCookie(&gsGlobalSecurityCookieVal, &gsGlobalSecurityCookieAddr);
3333
}
3434

35-
const unsigned NO_SHADOW_COPY = UINT_MAX;
36-
3735
/*****************************************************************************
3836
* gsCopyShadowParams
3937
* The current function has an unsafe buffer on the stack. Search for vulnerable
@@ -368,7 +366,7 @@ void Compiler::gsParamsToShadows()
368366
for (UINT lclNum = 0; lclNum < lvaOldCount; lclNum++)
369367
{
370368
LclVarDsc* varDsc = lvaGetDesc(lclNum);
371-
gsShadowVarInfo[lclNum].shadowCopy = NO_SHADOW_COPY;
369+
gsShadowVarInfo[lclNum].shadowCopy = BAD_VAR_NUM;
372370

373371
// Only care about params whose values are on the stack
374372
if (!ShadowParamVarInfo::mayNeedShadowCopy(varDsc))
@@ -452,7 +450,7 @@ void Compiler::gsParamsToShadows()
452450
unsigned int lclNum = tree->AsLclVarCommon()->GetLclNum();
453451
unsigned int shadowLclNum = m_compiler->gsShadowVarInfo[lclNum].shadowCopy;
454452

455-
if (shadowLclNum != NO_SHADOW_COPY)
453+
if (shadowLclNum != BAD_VAR_NUM)
456454
{
457455
LclVarDsc* varDsc = m_compiler->lvaGetDesc(lclNum);
458456
assert(ShadowParamVarInfo::mayNeedShadowCopy(varDsc));
@@ -492,7 +490,7 @@ void Compiler::gsParamsToShadows()
492490
const LclVarDsc* varDsc = lvaGetDesc(lclNum);
493491

494492
const unsigned shadowVarNum = gsShadowVarInfo[lclNum].shadowCopy;
495-
if (shadowVarNum == NO_SHADOW_COPY)
493+
if (shadowVarNum == BAD_VAR_NUM)
496494
{
497495
continue;
498496
}
@@ -544,7 +542,7 @@ void Compiler::gsParamsToShadows()
544542
const LclVarDsc* varDsc = lvaGetDesc(lclNum);
545543

546544
const unsigned shadowVarNum = gsShadowVarInfo[lclNum].shadowCopy;
547-
if (shadowVarNum == NO_SHADOW_COPY)
545+
if (shadowVarNum == BAD_VAR_NUM)
548546
{
549547
continue;
550548
}

0 commit comments

Comments
 (0)