Skip to content

Commit

Permalink
Utilize VN-based non-null knowledge better
Browse files Browse the repository at this point in the history
Previously, global non-null assertion propagation would give up
on any non-"ADD(LCL_VAR, CONST)"-like trees. This is correct for
actual assertion-based propagation (since we record assertions
based on conservative VNs and those propagate only through locals),
but is unnecessarily conservative when it comes to utilizing
non-nullness provided by VN.

Fix this by moving the IR checks after the VN check.
  • Loading branch information
SingleAccretion committed May 28, 2022
1 parent 748dfb7 commit 21ea0b2
Showing 1 changed file with 16 additions and 21 deletions.
37 changes: 16 additions & 21 deletions src/coreclr/jit/assertionprop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4297,23 +4297,11 @@ GenTree* Compiler::optAssertionProp_Ind(ASSERT_VALARG_TP assertions, GenTree* tr
return nullptr;
}

// Check for add of a constant.
GenTree* op1 = tree->AsIndir()->Addr();
if ((op1->gtOper == GT_ADD) && (op1->AsOp()->gtOp2->gtOper == GT_CNS_INT))
{
op1 = op1->AsOp()->gtOp1;
}

if (op1->gtOper != GT_LCL_VAR)
{
return nullptr;
}

#ifdef DEBUG
bool vnBased = false;
AssertionIndex index = NO_ASSERTION_INDEX;
#endif
if (optAssertionIsNonNull(op1, assertions DEBUGARG(&vnBased) DEBUGARG(&index)))
if (optAssertionIsNonNull(tree->AsIndir()->Addr(), assertions DEBUGARG(&vnBased) DEBUGARG(&index)))
{
#ifdef DEBUG
if (verbose)
Expand Down Expand Up @@ -4359,19 +4347,28 @@ bool Compiler::optAssertionIsNonNull(GenTree* op,
ASSERT_VALARG_TP assertions DEBUGARG(bool* pVnBased)
DEBUGARG(AssertionIndex* pIndex))
{
if (op->OperIs(GT_ADD) && op->AsOp()->gtGetOp2()->IsCnsIntOrI() &&
!fgIsBigOffset(op->AsOp()->gtGetOp2()->AsIntCon()->IconValue()))
{
op = op->AsOp()->gtGetOp1();
}

bool vnBased = (!optLocalAssertionProp && vnStore->IsKnownNonNull(op->gtVNPair.GetConservative()));
#ifdef DEBUG
*pIndex = NO_ASSERTION_INDEX;
*pVnBased = vnBased;
#endif

if (vnBased)
{
#ifdef DEBUG
*pIndex = NO_ASSERTION_INDEX;
#endif
return true;
}

if (!op->OperIs(GT_LCL_VAR))
{
return false;
}

AssertionIndex index = optAssertionIsNonNullInternal(op, assertions DEBUGARG(pVnBased));
#ifdef DEBUG
*pIndex = index;
Expand Down Expand Up @@ -4494,16 +4491,13 @@ AssertionIndex Compiler::optAssertionIsNonNullInternal(GenTree* op,
*/
GenTree* Compiler::optNonNullAssertionProp_Call(ASSERT_VALARG_TP assertions, GenTreeCall* call)
{
if ((call->gtFlags & GTF_CALL_NULLCHECK) == 0)
if (!call->NeedsNullCheck())
{
return nullptr;
}

GenTree* op1 = call->gtArgs.GetThisArg()->GetNode();
noway_assert(op1 != nullptr);
if (op1->gtOper != GT_LCL_VAR)
{
return nullptr;
}

#ifdef DEBUG
bool vnBased = false;
Expand All @@ -4524,6 +4518,7 @@ GenTree* Compiler::optNonNullAssertionProp_Call(ASSERT_VALARG_TP assertions, Gen
noway_assert(call->gtFlags & GTF_SIDE_EFFECT);
return call;
}

return nullptr;
}

Expand Down

0 comments on commit 21ea0b2

Please sign in to comment.