Skip to content

Commit

Permalink
Update the JIT to track Span.Length and ReadOnlySpan.Length as "n…
Browse files Browse the repository at this point in the history
…ever negative" (#81055)

* Update the JIT to track `Span.Length` and `ReadOnlySpan.Length` as "never negative"

* Updating the "is never negative" info to be tracked in LclVarDsc

* Apply formatting patch

* Ensure lvIsNeverNegative is propagated to shadows

* Update src/coreclr/jit/gentree.h

Co-authored-by: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com>

* Responding to PR feedback

* Adding two asserts

* Rename GenTreeField::IsNeverNegative to IsSpanLength and add a comment

* Fix an assert

Co-authored-by: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com>
  • Loading branch information
tannergooding and SingleAccretion authored Jan 26, 2023
1 parent c156b64 commit f54716d
Show file tree
Hide file tree
Showing 7 changed files with 130 additions and 5 deletions.
20 changes: 19 additions & 1 deletion src/coreclr/jit/assertionprop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -169,11 +169,20 @@ bool IntegralRange::Contains(int64_t value) const
break;

case GT_LCL_VAR:
if (compiler->lvaGetDesc(node->AsLclVar())->lvNormalizeOnStore())
{
LclVarDsc* const varDsc = compiler->lvaGetDesc(node->AsLclVar());

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

if (varDsc->IsNeverNegative())
{
return {SymbolicIntegerValue::Zero, UpperBoundForType(rangeType)};
}
break;
}

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

case GT_FIELD:
{
if (node->AsField()->IsSpanLength())
{
return {SymbolicIntegerValue::Zero, UpperBoundForType(rangeType)};
}
break;
}

default:
break;
}
Expand Down
16 changes: 16 additions & 0 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,10 @@ class LclVarDsc
unsigned char lvIsOSRLocal : 1; // Root method local in an OSR method. Any stack home will be on the Tier0 frame.
// Initial value will be defined by Tier0. Requires special handing in prolog.

private:
unsigned char lvIsNeverNegative : 1; // The local is known to be never negative

public:
union {
unsigned lvFieldLclStart; // The index of the local var representing the first field in the promoted struct
// local. For implicit byref parameters, this gets hijacked between
Expand Down Expand Up @@ -958,6 +962,18 @@ class LclVarDsc
}
#endif

// Is this is local never negative?
bool IsNeverNegative() const
{
return lvIsNeverNegative;
}

// Is this is local never negative?
void SetIsNeverNegative(bool value)
{
lvIsNeverNegative = value;
}

/////////////////////

regNumber GetArgInitReg() const
Expand Down
47 changes: 47 additions & 0 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8433,6 +8433,11 @@ GenTree* Compiler::gtClone(GenTree* tree, bool complexOK)
#ifdef FEATURE_READYTORUN
copy->AsField()->gtFieldLookup = tree->AsField()->gtFieldLookup;
#endif

if (tree->AsField()->IsSpanLength())
{
copy->AsField()->SetIsSpanLength(true);
}
}
else if (tree->OperIs(GT_ADD, GT_SUB))
{
Expand Down Expand Up @@ -8628,13 +8633,17 @@ GenTree* Compiler::gtCloneExpr(
goto DONE;

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

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

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

if ((oper == GT_FIELD) && tree->AsField()->IsSpanLength())
{
copy->AsField()->SetIsSpanLength(true);
}
break;

case GT_BOX:
Expand Down Expand Up @@ -24662,6 +24676,21 @@ ClassLayout* GenTreeLclVarCommon::GetLayout(Compiler* compiler) const
return AsLclFld()->GetLayout();
}

//------------------------------------------------------------------------
// GenTreeLclVar::IsNeverNegative: Gets true if the lcl var is never negative; otherwise false.
//
// Arguments:
// comp - the compiler instance
//
// Return Value:
// true if the lcl var is never negative; otherwise false.
//
bool GenTreeLclVar::IsNeverNegative(Compiler* comp) const
{
assert(OperIs(GT_LCL_VAR));
return comp->lvaGetDesc(GetLclNum())->IsNeverNegative();
}

#if defined(TARGET_XARCH) && defined(FEATURE_HW_INTRINSICS)
//------------------------------------------------------------------------
// GetResultOpNumForFMA: check if the result is written into one of the operands.
Expand Down Expand Up @@ -24755,6 +24784,24 @@ bool GenTree::IsNeverNegative(Compiler* comp) const
{
return AsIntConCommon()->IntegralValue() >= 0;
}

if (OperIs(GT_LCL_VAR))
{
if (AsLclVar()->IsNeverNegative(comp))
{
// This is an early exit, it doesn't cover all cases
return true;
}
}
else if (OperIs(GT_FIELD))
{
if (AsField()->IsSpanLength())
{
// This is an early exit, it doesn't cover all cases
return true;
}
}

// TODO-Casts: extend IntegralRange to handle constants
return IntegralRange::ForNode((GenTree*)this, comp).IsPositive();
}
33 changes: 31 additions & 2 deletions src/coreclr/jit/gentree.h
Original file line number Diff line number Diff line change
Expand Up @@ -3794,6 +3794,8 @@ struct GenTreeLclVar : public GenTreeLclVarCommon
unsigned int GetFieldCount(Compiler* compiler) const;
var_types GetFieldTypeByIndex(Compiler* compiler, unsigned idx);

bool IsNeverNegative(Compiler* comp) const;

//-------------------------------------------------------------------
// clearOtherRegFlags: clear GTF_* flags associated with gtOtherRegs
//
Expand Down Expand Up @@ -4004,13 +4006,22 @@ struct GenTreeField : public GenTreeUnOp
{
CORINFO_FIELD_HANDLE gtFldHnd;
DWORD gtFldOffset;
bool gtFldMayOverlap;
bool gtFldMayOverlap : 1;

private:
bool gtFldIsSpanLength : 1;

public:
#ifdef FEATURE_READYTORUN
CORINFO_CONST_LOOKUP gtFieldLookup;
#endif

GenTreeField(genTreeOps oper, var_types type, GenTree* obj, CORINFO_FIELD_HANDLE fldHnd, DWORD offs)
: GenTreeUnOp(oper, type, obj), gtFldHnd(fldHnd), gtFldOffset(offs), gtFldMayOverlap(false)
: GenTreeUnOp(oper, type, obj)
, gtFldHnd(fldHnd)
, gtFldOffset(offs)
, gtFldMayOverlap(false)
, gtFldIsSpanLength(false)
{
#ifdef FEATURE_READYTORUN
gtFieldLookup.addr = nullptr;
Expand All @@ -4037,6 +4048,24 @@ struct GenTreeField : public GenTreeUnOp
return (gtFlags & GTF_FLD_VOLATILE) != 0;
}

bool IsSpanLength() const
{
// This is limited to span length today rather than a more general "IsNeverNegative"
// to help avoid confusion around propagating the value to promoted lcl vars.
//
// Extending this support more in the future will require additional work and
// considerations to help ensure it is correctly used since people may want
// or intend to use this as more of a "point in time" feature like GTF_IND_NONNULL

assert(OperIs(GT_FIELD));
return gtFldIsSpanLength;
}

void SetIsSpanLength(bool value)
{
gtFldIsSpanLength = value;
}

bool IsInstance() const
{
return GetFldObj() != nullptr;
Expand Down
5 changes: 5 additions & 0 deletions src/coreclr/jit/gschecks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,11 @@ void Compiler::gsParamsToShadows()
shadowVarDsc->lvIsUnsafeBuffer = varDsc->lvIsUnsafeBuffer;
shadowVarDsc->lvIsPtr = varDsc->lvIsPtr;

if (varDsc->IsNeverNegative())
{
shadowVarDsc->SetIsNeverNegative(true);
}

#ifdef DEBUG
if (verbose)
{
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);

GenTreeField* length = gtNewFieldRef(TYP_INT, lengthHnd, ptrToSpan, lengthOffset);
length->SetIsSpanLength(true);

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->SetIsSpanLength(true);
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 @@ -1398,6 +1398,11 @@ class LocalAddressVisitor final : public GenTreeVisitor<LocalAddressVisitor>
{
GenTreeFlags lclVarFlags = node->gtFlags & (GTF_NODE_MASK | GTF_DONT_CSE);

if (node->OperIs(GT_FIELD) && node->AsField()->IsSpanLength())
{
m_compiler->lvaGetDesc(fieldLclNum)->SetIsNeverNegative(true);
}

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

0 comments on commit f54716d

Please sign in to comment.