From f54716dabfd90610a8dedb0e6abdc68e20d48916 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Wed, 25 Jan 2023 17:24:09 -0800 Subject: [PATCH] Update the JIT to track `Span.Length` and `ReadOnlySpan.Length` as "never 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> --- src/coreclr/jit/assertionprop.cpp | 20 ++++++++++++- src/coreclr/jit/compiler.h | 16 +++++++++++ src/coreclr/jit/gentree.cpp | 47 +++++++++++++++++++++++++++++++ src/coreclr/jit/gentree.h | 33 ++++++++++++++++++++-- src/coreclr/jit/gschecks.cpp | 5 ++++ src/coreclr/jit/importercalls.cpp | 9 ++++-- src/coreclr/jit/lclmorph.cpp | 5 ++++ 7 files changed, 130 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index a45e8be2d41a5..5f2173b5cdc46 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -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)) @@ -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; } diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 1299b34bb431e..2d60be8a1f26e 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -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 @@ -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 diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 57ae740606282..feec2becd40ff 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -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)) { @@ -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"); @@ -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: @@ -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. @@ -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(); } diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index ea8731460bf7c..1e3f87248589d 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -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 // @@ -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; @@ -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; diff --git a/src/coreclr/jit/gschecks.cpp b/src/coreclr/jit/gschecks.cpp index 05d06ed112036..9890c19d49678 100644 --- a/src/coreclr/jit/gschecks.cpp +++ b/src/coreclr/jit/gschecks.cpp @@ -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) { diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 5feba97149b04..8479ee433b4d4 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -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 @@ -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: diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp index 8f2dbb29b3d11..ce52720fe33ed 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/src/coreclr/jit/lclmorph.cpp @@ -1398,6 +1398,11 @@ class LocalAddressVisitor final : public GenTreeVisitor { 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;