Skip to content

Commit

Permalink
JIT: update non-null assertion prop to destructure VNs
Browse files Browse the repository at this point in the history
In addition to checking for assertions based on the VN of an address, try and
destructure the VN to find the "base" address, and check its VNs as well.

This lets us get rid of some extra null checks, typically ones that are at
an offset from an existing non-null pointer.

Closes dotnet#49180.
  • Loading branch information
AndyAyersMS committed Mar 5, 2021
1 parent b0ba53a commit 940a046
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 31 deletions.
124 changes: 94 additions & 30 deletions src/coreclr/jit/assertionprop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3685,16 +3685,21 @@ GenTree* Compiler::optAssertionProp_Comma(ASSERT_VALARG_TP assertions, GenTree*
return nullptr;
}

/*****************************************************************************
*
* Given a tree consisting of a Ind and a set of available assertions, we try
* to propagate an assertion and modify the Ind tree if we can. We pass in the
* root of the tree via 'stmt', for local copy prop 'stmt' will be nullptr.
*
* Returns the modified tree, or nullptr if no assertion prop took place.
*
*/

//------------------------------------------------------------------------
// optAssertionProp_Ind: see if we can prove the indirection can't cause
// and exception.
//
// Arguments:
// assertions - set of live assertions
// tree - tree to possibly optimize
// stmt - statement containing the tree
//
// Returns:
// The modified tree, or nullptr if no assertion prop took place.
//
// Notes:
// stmt may be nullptr during local assertion prop
//
GenTree* Compiler::optAssertionProp_Ind(ASSERT_VALARG_TP assertions, GenTree* tree, Statement* stmt)
{
assert(tree->OperIsIndir());
Expand Down Expand Up @@ -3742,16 +3747,26 @@ GenTree* Compiler::optAssertionProp_Ind(ASSERT_VALARG_TP assertions, GenTree* tr
return nullptr;
}

/*****************************************************************************
* Check if a non-null assertion can be made about the input operand "op"
* from the set of "assertions," or implicitly from the value number on "op."
*
* Sets "pVnBased" if the assertion is value number based. If no matching
* assertions are found from the table, then returns "NO_ASSERTION_INDEX."
*
* Note: If both VN and assertion table yield a matching assertion, "pVnBased"
* is only set and the return value is "NO_ASSERTION_INDEX."
*/
//------------------------------------------------------------------------
// optAssertionIsNonNull: see if we can prove a tree's value will be non-null
// based on assertions
//
// Arguments:
// op - tree to check
// assertions - set of live assertions
// pVnBased - [out] set to true if value numbers were used
// pIndex - [out] the assertion used in the proof
//
// Returns:
// true if the tree's value will be non-null
//
// Notes:
// Sets "pVnBased" if the assertion is value number based. If no matching
// assertions are found from the table, then returns "NO_ASSERTION_INDEX."
//
// If both VN and assertion table yield a matching assertion, "pVnBased"
// is only set and the return value is "NO_ASSERTION_INDEX."
//
bool Compiler::optAssertionIsNonNull(GenTree* op,
ASSERT_VALARG_TP assertions DEBUGARG(bool* pVnBased)
DEBUGARG(AssertionIndex* pIndex))
Expand All @@ -3769,20 +3784,35 @@ bool Compiler::optAssertionIsNonNull(GenTree* op,
return true;
}

AssertionIndex index = optAssertionIsNonNullInternal(op, assertions);
AssertionIndex index = optAssertionIsNonNullInternal(op, assertions DEBUGARG(pVnBased));
#ifdef DEBUG
*pIndex = index;
#endif
return index != NO_ASSERTION_INDEX;
}

/*****************************************************************************
* Check if a non-null assertion can be made about the input operand "op"
* from the set of "assertions."
*
*/
AssertionIndex Compiler::optAssertionIsNonNullInternal(GenTree* op, ASSERT_VALARG_TP assertions)
//------------------------------------------------------------------------
// optAssertionIsNonNullInternal: see if we can prove a tree's value will
// be non-null based on assertions
//
// Arguments:
// op - tree to check
// assertions - set of live assertions
// pVnBased - [out] set to true if value numbers were used
//
// Returns:
// index of assertion, or NO_ASSERTION_INDEX
//
AssertionIndex Compiler::optAssertionIsNonNullInternal(GenTree* op,
ASSERT_VALARG_TP assertions DEBUGARG(bool* pVnBased))
{

#ifdef DEBUG
// Initialize the out param
//
*pVnBased = false;
#endif

// If local assertion prop use lcl comparison, else use VN comparison.
if (!optLocalAssertionProp)
{
Expand All @@ -3791,9 +3821,32 @@ AssertionIndex Compiler::optAssertionIsNonNullInternal(GenTree* op, ASSERT_VALAR
return NO_ASSERTION_INDEX;
}

ValueNum vn = op->gtVNPair.GetConservative();
// Look at both the top-level vn, and
// the vn we get by stripping off any constant adds.
//
ValueNum vn = vnStore->VNConservativeNormalValue(op->gtVNPair);
ValueNum vnBase = vn;
VNFuncApp funcAttr;

// Check each assertion to find if we have a vn == or != null assertion.
while (vnStore->GetVNFunc(vnBase, &funcAttr) && (funcAttr.m_func == (VNFunc)GT_ADD))
{
if (vnStore->IsVNConstant(funcAttr.m_args[1]) && varTypeIsIntegral(vnStore->TypeOfVN(funcAttr.m_args[1])))
{
vnBase = funcAttr.m_args[0];
}
else if (vnStore->IsVNConstant(funcAttr.m_args[0]) &&
varTypeIsIntegral(vnStore->TypeOfVN(funcAttr.m_args[0])))
{
vnBase = funcAttr.m_args[1];
}
else
{
break;
}
}

// Check each assertion to find if we have a vn != null assertion.
//
BitVecOps::Iter iter(apTraits, assertions);
unsigned index = 0;
while (iter.NextElem(&index))
Expand All @@ -3808,10 +3861,21 @@ AssertionIndex Compiler::optAssertionIsNonNullInternal(GenTree* op, ASSERT_VALAR
{
continue;
}
if (curAssertion->op1.vn != vn || curAssertion->op2.vn != ValueNumStore::VNForNull())

if (curAssertion->op2.vn != ValueNumStore::VNForNull())
{
continue;
}

if ((curAssertion->op1.vn != vn) && (curAssertion->op1.vn != vnBase))
{
continue;
}

#ifdef DEBUG
*pVnBased = true;
#endif

return assertionIndex;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -7259,7 +7259,7 @@ class Compiler
var_types toType,
ASSERT_VALARG_TP assertions);
AssertionIndex optAssertionIsSubtype(GenTree* tree, GenTree* methodTableArg, ASSERT_VALARG_TP assertions);
AssertionIndex optAssertionIsNonNullInternal(GenTree* op, ASSERT_VALARG_TP assertions);
AssertionIndex optAssertionIsNonNullInternal(GenTree* op, ASSERT_VALARG_TP assertions DEBUGARG(bool* pVnBased));
bool optAssertionIsNonNull(GenTree* op,
ASSERT_VALARG_TP assertions DEBUGARG(bool* pVnBased) DEBUGARG(AssertionIndex* pIndex));

Expand Down

0 comments on commit 940a046

Please sign in to comment.