Skip to content

Commit

Permalink
response review.
Browse files Browse the repository at this point in the history
  • Loading branch information
Sergey committed Aug 24, 2021
1 parent fe65446 commit e438e3d
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 48 deletions.
2 changes: 2 additions & 0 deletions src/coreclr/jit/compiler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5208,6 +5208,8 @@ class Compiler
// Does value-numbering for a block assignment.
void fgValueNumberBlockAssignment(GenTree* tree);

bool fgValueNumberIsStructReinterpretation(GenTreeLclVarCommon* lhsLclVarTree, GenTreeLclVarCommon* rhsLclVarTree);

// Does value-numbering for a cast tree.
void fgValueNumberCastTree(GenTree* tree);

Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/lclmorph.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ class LocalAddressVisitor final : public GenTreeVisitor<LocalAddressVisitor>

haveCorrectFieldForVN =
compiler->info.compCompHnd->doesFieldBelongToClass(field->gtFldHnd, clsHnd);
assert(haveCorrectFieldForVN);
noway_assert(haveCorrectFieldForVN);
}
}
}
Expand Down
113 changes: 66 additions & 47 deletions src/coreclr/jit/valuenum.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7116,7 +7116,7 @@ void Compiler::fgValueNumberBlockAssignment(GenTree* tree)
unsigned lhsLclNum = lclVarTree->GetLclNum();
FieldSeqNode* lhsFldSeq = nullptr;
unsigned lclDefSsaNum = GetSsaNumForLocalVarDef(lclVarTree);
LclVarDsc* lhsVarDsc = lvaGetDesc(lhsLclNum);
LclVarDsc* lhsVarDsc = lvaGetDesc(lhsLclNum);
// If it's excluded from SSA, don't need to do anything.
if (lvaInSsa(lhsLclNum) && lclDefSsaNum != SsaConfig::RESERVED_SSA_NUM)
{
Expand Down Expand Up @@ -7269,52 +7269,9 @@ void Compiler::fgValueNumberBlockAssignment(GenTree* tree)
isNewUniq = true;
}

if (!isNewUniq && (rhsVarDsc != nullptr) && (lhsVarDsc != nullptr))
if (!isNewUniq && (lclVarTree != nullptr) && (rhsLclVarTree != nullptr))
{
if (rhsLclVarTree->TypeGet() == TYP_STRUCT)
{
if (rhsLclVarTree->TypeGet() == lclVarTree->TypeGet())
{
assert(rhsVarDsc->TypeGet() == TYP_STRUCT);
assert(lhsVarDsc->TypeGet() == TYP_STRUCT);

CORINFO_CLASS_HANDLE rhsStructHnd = NO_CLASS_HANDLE;
CORINFO_CLASS_HANDLE lhsStructHnd = NO_CLASS_HANDLE;

if (rhsFldSeq == nullptr)
{
rhsStructHnd = rhsVarDsc->GetStructHnd();
}
else
{
CORINFO_FIELD_HANDLE fldHnd = rhsFldSeq->GetTail()->GetFieldHandle();
rhsStructHnd = info.compCompHnd->getFieldClass(fldHnd);
}

if (lhsFldSeq == nullptr)
{
lhsStructHnd = lhsVarDsc->GetStructHnd();
}
else
{
CORINFO_FIELD_HANDLE fldHnd = lhsFldSeq->GetTail()->GetFieldHandle();
lhsStructHnd = info.compCompHnd->getFieldClass(fldHnd);
}

if (rhsStructHnd != lhsStructHnd)
{
// This can occur for nested structs or for unsafe casts, when we have IR like
// struct1 = struct2.
// Use an unique value number for the old map, as we don't have information about
// the dst field values using dst FIELD_HANDLE.
// Note that other asignments, like struct1 = IND struct(ADDR(LCL_VAR long))
// will be handled in `VNPairApplySelectorsAssign`, here we care only about
// `LCL_VAR structX = (*)LCL_VAR structY` cases.
JITDUMP(" *** Different struct handles for Dst/Src of COPYBLK\n");
isNewUniq = true;
}
}
}
isNewUniq = fgValueNumberIsStructReinterpretation(lclVarTree, rhsLclVarTree);
}

if (isNewUniq)
Expand Down Expand Up @@ -7381,6 +7338,67 @@ void Compiler::fgValueNumberBlockAssignment(GenTree* tree)
}
}

//------------------------------------------------------------------------
// fgValueNumberIsStructReinterpretation: Checks if there is a struct reinterpretation that prevent VN propagation.
//
// Arguments:
// lhsLclVarTree - a lcl var tree on the lhs of the asg;
// rhsLclVarTree - a lcl var tree on the rhs of the asg;
//
// Return Value:
// True if the locals have different struct types and VN can't use rhs VN for lhs VN.
// False if locals have the same struct type or if this ASG is not a struct ASG.
//
bool Compiler::fgValueNumberIsStructReinterpretation(GenTreeLclVarCommon* lhsLclVarTree,
GenTreeLclVarCommon* rhsLclVarTree)
{
assert(lhsLclVarTree != nullptr);
assert(rhsLclVarTree != nullptr);

if (rhsLclVarTree->TypeGet() == TYP_STRUCT)
{
if (rhsLclVarTree->TypeGet() == lhsLclVarTree->TypeGet())
{
if (lhsLclVarTree->isLclField() || rhsLclVarTree->isLclField())
{
// Jit does not have a real support for `LCL_FLD struct [FldSeq]` because it can't determinate their
// size
// when the fieldSeq is `NotAField`, but by mistake we could have
// `BLK(ADDR byref(LCL_FLD struct Fseq[]))` nowadays in out IR.
// Generate a unique VN for now, it currently won't match the other side,
// otherwise we would not have 'OBJ struct2 (ADDR(LCL_FLD struct1))` cast.
return true;
}

assert(lhsLclVarTree->OperIs(GT_LCL_VAR));
assert(rhsLclVarTree->OperIs(GT_LCL_VAR));

const LclVarDsc* lhsVarDsc = lvaGetDesc(lhsLclVarTree);
const LclVarDsc* rhsVarDsc = lvaGetDesc(rhsLclVarTree);
assert(rhsVarDsc->TypeGet() == TYP_STRUCT);
assert(lhsVarDsc->TypeGet() == TYP_STRUCT);

CORINFO_CLASS_HANDLE rhsStructHnd = rhsVarDsc->GetStructHnd();
CORINFO_CLASS_HANDLE lhsStructHnd = lhsVarDsc->GetStructHnd();

if (rhsStructHnd != lhsStructHnd)
{
// This can occur for nested structs or for unsafe casts, when we have IR like
// struct1 = struct2.
// Use an unique value number for the old map, as we don't have information about
// the dst field values using dst FIELD_HANDLE.
// Note that other asignments, like struct1 = IND struct(ADDR(LCL_VAR long))
// will be handled in `VNPairApplySelectorsAssign`, here we care only about
// `LCL_VAR structX = (*)LCL_VAR structY` cases.
JITDUMP(" *** Different struct handles for Dst/Src of COPYBLK\n");
return true;
}
}
}

return false;
}

void Compiler::fgValueNumberTree(GenTree* tree)
{
genTreeOps oper = tree->OperGet();
Expand Down Expand Up @@ -8032,7 +8050,6 @@ void Compiler::fgValueNumberTree(GenTree* tree)
rhsVNPair.SetBoth(vnStore->VNForExpr(compCurBB, lclVarTree->TypeGet()));
}

unsigned lclDefSsaNum = GetSsaNumForLocalVarDef(lclVarTree);
ValueNumPair newLhsVNPair;
if (isEntire)
{
Expand All @@ -8050,6 +8067,8 @@ void Compiler::fgValueNumberTree(GenTree* tree)
lhs->TypeGet(), compCurBB);
}

unsigned lclDefSsaNum = GetSsaNumForLocalVarDef(lclVarTree);

if (lclDefSsaNum != SsaConfig::RESERVED_SSA_NUM)
{
lvaTable[lclNum].GetPerSsaData(lclDefSsaNum)->m_vnPair = newLhsVNPair;
Expand Down

0 comments on commit e438e3d

Please sign in to comment.