Skip to content

Propagate exception sets for assignments #64882

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jakobbotsch merged 4 commits into from
Feb 7, 2022
Merged

Propagate exception sets for assignments #64882

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d6946bd
Propagate exception sets for block assignments
SingleAccretion Jan 10, 2022
5f4a28c
Propagate exception sets for simple assignments
SingleAccretion Feb 6, 2022
ec13124
Fix fgValueNumberCastHelper
SingleAccretion Feb 6, 2022
fd976df
Enable the checker
SingleAccretion Feb 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 27 additions & 49 deletions src/coreclr/jit/valuenum.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7480,59 +7480,29 @@ void Compiler::fgValueNumberAssignment(GenTreeOp* tree)
{
assert(tree->OperIs(GT_ASG) && varTypeIsEnregisterable(tree));

GenTree* lhs = tree->AsOp()->gtOp1;
GenTree* rhs = tree->AsOp()->gtOp2;
GenTree* lhs = tree->gtGetOp1();
GenTree* rhs = tree->gtGetOp2();

ValueNumPair rhsVNPair = rhs->gtVNPair;
// Only normal values are to be stored in SSA defs, VN maps, etc.
ValueNumPair rhsExcSet;
ValueNumPair rhsVNPair;
vnStore->VNPUnpackExc(rhs->gtVNPair, &rhsVNPair, &rhsExcSet);

// Is the type being stored different from the type computed by the rhs?
if ((rhs->TypeGet() != lhs->TypeGet()) && (lhs->OperGet() != GT_BLK))
{
// This means that there is an implicit cast on the rhs value
//
// We will add a cast function to reflect the possible narrowing of the rhs value
//
var_types castToType = lhs->TypeGet();
var_types castFromType = rhs->TypeGet();
bool isUnsigned = varTypeIsUnsigned(castFromType);

rhsVNPair = vnStore->VNPairForCast(rhsVNPair, castToType, castFromType, isUnsigned);
}

if (tree->TypeGet() != TYP_VOID)
{
// Assignment operators, as expressions, return the value of the RHS.
tree->gtVNPair = rhsVNPair;
}

// Now that we've labeled the assignment as a whole, we don't care about exceptions.
rhsVNPair = vnStore->VNPNormalPair(rhsVNPair);

// Record the exception set for this 'tree' in vnExcSet.
// First we'll record the exception set for the rhs and
// later we will union in the exception set for the lhs.
//
ValueNum vnExcSet;

// Unpack, Norm,Exc for 'rhsVNPair'
ValueNum vnRhsLibNorm;
vnStore->VNUnpackExc(rhsVNPair.GetLiberal(), &vnRhsLibNorm, &vnExcSet);

// Now that we've saved the rhs exeception set, we we will use the normal values.
rhsVNPair = ValueNumPair(vnRhsLibNorm, vnStore->VNNormalValue(rhsVNPair.GetConservative()));

// If the types of the rhs and lhs are different then we
// may want to change the ValueNumber assigned to the lhs.
//
if (rhs->TypeGet() != lhs->TypeGet())
{
if (rhs->TypeGet() == TYP_REF)
{
// If we have an unsafe IL assignment of a TYP_REF to a non-ref (typically a TYP_BYREF)
// then don't propagate this ValueNumber to the lhs, instead create a new unique VN
//
// then don't propagate this ValueNumber to the lhs, instead create a new unique VN.
rhsVNPair.SetBoth(vnStore->VNForExpr(compCurBB, lhs->TypeGet()));
}
else if (lhs->OperGet() != GT_BLK)
{
// This means that there is an implicit cast on the rhs value
// We will add a cast function to reflect the possible narrowing of the rhs value
rhsVNPair = vnStore->VNPairForCast(rhsVNPair, lhs->TypeGet(), rhs->TypeGet());
}
}

// We have to handle the case where the LHS is a comma. In that case, we don't evaluate the comma,
Expand Down Expand Up @@ -8002,6 +7972,12 @@ void Compiler::fgValueNumberAssignment(GenTreeOp* tree)
default:
unreached();
}

// For exception sets, we need the contribution from COMMAs on the
// LHS. ASGs produce no values, and as such are given the "Void" VN.
ValueNumPair lhsExcSet = vnStore->VNPExceptionSet(tree->gtGetOp1()->gtVNPair);
ValueNumPair asgExcSet = vnStore->VNPExcSetUnion(lhsExcSet, rhsExcSet);
tree->gtVNPair = vnStore->VNPWithExc(vnStore->VNPForVoid(), asgExcSet);
}

//------------------------------------------------------------------------
Expand Down Expand Up @@ -8133,8 +8109,11 @@ void Compiler::fgValueNumberBlockAssignment(GenTree* tree)
fgMutateGcHeap(tree DEBUGARG("INITBLK/COPYBLK - non local"));
}

// Assignments produce no values so we give them the "Void" VN.
tree->gtVNPair = vnStore->VNPForVoid();
// Propagate the exception sets. Assignments produce no values so we give them the "Void" VN.
ValueNumPair vnpExcSet = ValueNumStore::VNPForEmptyExcSet();
vnpExcSet = vnStore->VNPUnionExcSet(lhs->gtVNPair, vnpExcSet);
vnpExcSet = vnStore->VNPUnionExcSet(rhs->gtVNPair, vnpExcSet);
tree->gtVNPair = vnStore->VNPWithExc(vnStore->VNPForVoid(), vnpExcSet);
}

//------------------------------------------------------------------------
Expand Down Expand Up @@ -10025,7 +10004,7 @@ void Compiler::fgValueNumberCastHelper(GenTreeCall* call)
unreached();
}

ValueNumPair argVNP = call->gtCallArgs->GetNode()->gtVNPair;
ValueNumPair argVNP = call->fgArgInfo->GetArgNode(0)->gtVNPair;
ValueNumPair castVNP = vnStore->VNPairForCast(argVNP, castToType, castFromType, srcIsUnsigned, hasOverflowCheck);

call->SetVNs(castVNP);
Expand Down Expand Up @@ -10941,9 +10920,8 @@ void Compiler::fgDebugCheckExceptionSets()
return GenTree::VisitResult::Continue;
});

// Currently, we fail to properly maintain the exception sets for trees with user
// calls or assignments.
if ((tree->gtFlags & (GTF_ASG | GTF_CALL)) != 0)
// Currently, we fail to properly maintain the exception sets for trees with user calls.
if ((tree->gtFlags & GTF_CALL) != 0)
{
return;
}
Expand Down