Skip to content
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.

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

Update the JIT to track Span.Length and ReadOnlySpan.Length as "never negative" #81055

Merged
merged 9 commits into from
Jan 26, 2023
16 changes: 16 additions & 0 deletions src/coreclr/jit/assertionprop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -169,11 +169,18 @@ bool IntegralRange::Contains(int64_t value) const
break;

case GT_LCL_VAR:
{
if (compiler->lvaGetDesc(node->AsLclVar())->lvNormalizeOnStore())
{
rangeType = compiler->lvaGetDesc(node->AsLclVar())->TypeGet();
}

if ((node->gtFlags & GTF_VAR_NEVER_NEGATIVE) != 0)
{
return {SymbolicIntegerValue::Zero, UpperBoundForType(rangeType)};
}
break;
}

case GT_CNS_INT:
if (node->IsIntegralConst(0) || node->IsIntegralConst(1))
Expand Down Expand Up @@ -218,6 +225,15 @@ bool IntegralRange::Contains(int64_t value) const
break;
#endif // defined(FEATURE_HW_INTRINSICS)

case GT_FIELD:
{
if ((node->gtFlags & GTF_FLD_NEVER_NEGATIVE) != 0)
{
return {SymbolicIntegerValue::Zero, UpperBoundForType(rangeType)};
}
break;
}

default:
break;
}
Expand Down
37 changes: 37 additions & 0 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8407,6 +8407,13 @@ GenTree* Compiler::gtClone(GenTree* tree, bool complexOK)
tree->gtFlags |= GTF_VAR_CLONED;
copy->AsLclVarCommon()->SetSsaNum(tree->AsLclVarCommon()->GetSsaNum());
assert(!copy->AsLclVarCommon()->HasSsaName() || ((copy->gtFlags & GTF_VAR_DEF) == 0));

if ((tree->gtFlags & GTF_VAR_NEVER_NEGATIVE) != 0)
{
// We should only see this on LCL_VAR today
assert(tree->gtOper == GT_LCL_VAR);
copy->gtFlags |= GTF_VAR_NEVER_NEGATIVE;
}
break;

default:
Expand All @@ -8433,6 +8440,11 @@ GenTree* Compiler::gtClone(GenTree* tree, bool complexOK)
#ifdef FEATURE_READYTORUN
copy->AsField()->gtFieldLookup = tree->AsField()->gtFieldLookup;
#endif

if ((tree->gtFlags & GTF_FLD_NEVER_NEGATIVE) != 0)
{
copy->gtFlags |= GTF_FLD_NEVER_NEGATIVE;
}
}
else if (tree->OperIs(GT_ADD, GT_SUB))
{
Expand Down Expand Up @@ -8575,6 +8587,11 @@ GenTree* Compiler::gtCloneExpr(
copy = gtNewLclvNode(tree->AsLclVar()->GetLclNum(),
tree->gtType DEBUGARG(tree->AsLclVar()->gtLclILoffs));
copy->AsLclVarCommon()->SetSsaNum(tree->AsLclVarCommon()->GetSsaNum());

if ((tree->gtFlags & GTF_VAR_NEVER_NEGATIVE) != 0)
{
copy->gtFlags |= GTF_VAR_NEVER_NEGATIVE;
}
}
goto DONE;

Expand All @@ -8592,6 +8609,9 @@ GenTree* Compiler::gtCloneExpr(
GenTreeLclFld(GT_LCL_FLD, tree->TypeGet(), tree->AsLclFld()->GetLclNum(),
tree->AsLclFld()->GetLclOffs(), tree->AsLclFld()->GetLayout());
copy->AsLclFld()->SetSsaNum(tree->AsLclFld()->GetSsaNum());

// We shouldn't see this on LCL_FLD today
assert((tree->gtFlags & GTF_VAR_NEVER_NEGATIVE) == 0);
}
goto DONE;

Expand Down Expand Up @@ -8628,13 +8648,25 @@ GenTree* Compiler::gtCloneExpr(
goto DONE;

case GT_LCL_VAR_ADDR:
{
copy = new (this, oper) GenTreeLclVar(oper, tree->TypeGet(), tree->AsLclVar()->GetLclNum());

// We shouldn't see this on LCL_VAR_ADDR today
assert((tree->gtFlags & GTF_VAR_NEVER_NEGATIVE) == 0);

goto DONE;
}

case GT_LCL_FLD_ADDR:
{
copy = new (this, oper)
GenTreeLclFld(oper, tree->TypeGet(), tree->AsLclFld()->GetLclNum(), tree->AsLclFld()->GetLclOffs());

// We shouldn't see this on LCL_FLD_ADDR today
assert((tree->gtFlags & GTF_VAR_NEVER_NEGATIVE) == 0);

goto DONE;
}

default:
NO_WAY("Cloning of node not supported");
Expand Down Expand Up @@ -8772,6 +8804,11 @@ GenTree* Compiler::gtCloneExpr(
#ifdef FEATURE_READYTORUN
copy->AsField()->gtFieldLookup = tree->AsField()->gtFieldLookup;
#endif

if ((tree->gtFlags & GTF_FLD_NEVER_NEGATIVE) != 0)
{
copy->gtFlags |= GTF_FLD_NEVER_NEGATIVE;
}
break;

case GT_BOX:
Expand Down
10 changes: 6 additions & 4 deletions src/coreclr/jit/gentree.h
Original file line number Diff line number Diff line change
Expand Up @@ -470,10 +470,11 @@ enum GenTreeFlags : unsigned int

GTF_LIVENESS_MASK = GTF_VAR_DEF | GTF_VAR_USEASG | GTF_VAR_DEATH_MASK,

GTF_VAR_ITERATOR = 0x01000000, // GT_LCL_VAR -- this is a iterator reference in the loop condition
GTF_VAR_CLONED = 0x00800000, // GT_LCL_VAR -- this node has been cloned or is a clone
GTF_VAR_CONTEXT = 0x00400000, // GT_LCL_VAR -- this node is part of a runtime lookup
GTF_VAR_EXPLICIT_INIT = 0x00200000, // GT_LCL_VAR -- this node is an "explicit init" store. Valid until rationalization.
GTF_VAR_ITERATOR = 0x01000000, // GT_LCL_VAR -- this is a iterator reference in the loop condition
GTF_VAR_CLONED = 0x00800000, // GT_LCL_VAR -- this node has been cloned or is a clone
GTF_VAR_CONTEXT = 0x00400000, // GT_LCL_VAR -- this node is part of a runtime lookup
GTF_VAR_EXPLICIT_INIT = 0x00200000, // GT_LCL_VAR -- this node is an "explicit init" store. Valid until rationalization.
tannergooding marked this conversation as resolved.
Show resolved Hide resolved
GTF_VAR_NEVER_NEGATIVE = 0x00100000, // GT_LCL_VAR -- this node can be assumed to never return a negative value

// For additional flags for GT_CALL node see GTF_CALL_M_*

Expand All @@ -492,6 +493,7 @@ enum GenTreeFlags : unsigned int
GTF_MEMORYBARRIER_LOAD = 0x40000000, // GT_MEMORYBARRIER -- Load barrier

GTF_FLD_TLS = 0x80000000, // GT_FIELD_ADDR -- field address is a Windows x86 TLS reference
GTF_FLD_NEVER_NEGATIVE = 0x80000000, // GT_FIELD -- this field can be assumed to never return a negative value
tannergooding marked this conversation as resolved.
Show resolved Hide resolved
GTF_FLD_VOLATILE = 0x40000000, // GT_FIELD -- same as GTF_IND_VOLATILE
GTF_FLD_INITCLASS = 0x20000000, // GT_FIELD/GT_FIELD_ADDR -- field access requires preceding class/static init helper
GTF_FLD_TGT_HEAP = 0x10000000, // GT_FIELD -- same as GTF_IND_TGT_HEAP
Expand Down
9 changes: 7 additions & 2 deletions src/coreclr/jit/importercalls.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2781,7 +2781,10 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis,
// Bounds check
CORINFO_FIELD_HANDLE lengthHnd = info.compCompHnd->getFieldInClass(clsHnd, 1);
const unsigned lengthOffset = info.compCompHnd->getFieldOffset(lengthHnd);
GenTree* length = gtNewFieldRef(TYP_INT, lengthHnd, ptrToSpan, lengthOffset);

GenTree* length = gtNewFieldRef(TYP_INT, lengthHnd, ptrToSpan, lengthOffset);
length->gtFlags |= GTF_FLD_NEVER_NEGATIVE;

GenTree* boundsCheck = new (this, GT_BOUNDS_CHECK) GenTreeBoundsChk(index, length, SCK_RNGCHK_FAIL);

// Element access
Expand Down Expand Up @@ -2840,7 +2843,9 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis,
CORINFO_FIELD_HANDLE lengthHnd = info.compCompHnd->getFieldInClass(clsHnd, 1);
const unsigned lengthOffset = info.compCompHnd->getFieldOffset(lengthHnd);

return gtNewFieldRef(TYP_INT, lengthHnd, ptrToSpan, lengthOffset);
GenTreeField* fieldRef = gtNewFieldRef(TYP_INT, lengthHnd, ptrToSpan, lengthOffset);
fieldRef->gtFlags |= GTF_FLD_NEVER_NEGATIVE;
return fieldRef;
}

case NI_System_RuntimeTypeHandle_GetValueInternal:
Expand Down
5 changes: 5 additions & 0 deletions src/coreclr/jit/lclmorph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1392,6 +1392,11 @@ class LocalAddressVisitor final : public GenTreeVisitor<LocalAddressVisitor>
{
GenTreeFlags lclVarFlags = node->gtFlags & (GTF_NODE_MASK | GTF_DONT_CSE);

if (node->OperIs(GT_FIELD) && (node->gtFlags & GTF_FLD_NEVER_NEGATIVE))
{
lclVarFlags |= GTF_VAR_NEVER_NEGATIVE;
}

if ((user != nullptr) && user->OperIs(GT_ASG) && (user->AsOp()->gtOp1 == node))
{
lclVarFlags |= GTF_VAR_DEF;
Expand Down