From 5dac74f6cd101430be5ed747623247413a7a6586 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Mon, 23 Jan 2023 13:18:56 -0800 Subject: [PATCH 1/9] Update the JIT to track `Span.Length` and `ReadOnlySpan.Length` as "never negative" --- src/coreclr/jit/assertionprop.cpp | 16 +++++++++++++ src/coreclr/jit/gentree.cpp | 37 +++++++++++++++++++++++++++++++ src/coreclr/jit/gentree.h | 10 +++++---- src/coreclr/jit/importercalls.cpp | 9 ++++++-- src/coreclr/jit/lclmorph.cpp | 5 +++++ 5 files changed, 71 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 648a01774dbbe..8f8f7ecf6be6e 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -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)) @@ -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; } diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index dd1dc0e70d371..36db3d0177279 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -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: @@ -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)) { @@ -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; @@ -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; @@ -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"); @@ -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: diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 790d14f284177..b2aecbbe4a22a 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -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. + 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_* @@ -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 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 diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 5feba97149b04..6e62cefc7fe40 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); + + 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 @@ -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: diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp index 33da0308c4f1d..6e9153d4728bf 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/src/coreclr/jit/lclmorph.cpp @@ -1392,6 +1392,11 @@ class LocalAddressVisitor final : public GenTreeVisitor { 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; From c88ae01c30035bd97e2e0f7db305e2f40debddba Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 24 Jan 2023 08:13:14 -0800 Subject: [PATCH 2/9] Updating the "is never negative" info to be tracked in LclVarDsc --- src/coreclr/jit/assertionprop.cpp | 8 ++-- src/coreclr/jit/compiler.h | 16 ++++++++ src/coreclr/jit/gentree.cpp | 63 ++++++++++++++++++------------- src/coreclr/jit/gentree.h | 21 +++++++++-- src/coreclr/jit/importercalls.cpp | 6 +-- src/coreclr/jit/lclmorph.cpp | 4 +- 6 files changed, 80 insertions(+), 38 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 8f8f7ecf6be6e..7b8b0f5c00ef3 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -170,12 +170,14 @@ bool IntegralRange::Contains(int64_t value) const 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 ((node->gtFlags & GTF_VAR_NEVER_NEGATIVE) != 0) + if (varDsc->IsNeverNegative()) { return {SymbolicIntegerValue::Zero, UpperBoundForType(rangeType)}; } @@ -227,7 +229,7 @@ bool IntegralRange::Contains(int64_t value) const case GT_FIELD: { - if ((node->gtFlags & GTF_FLD_NEVER_NEGATIVE) != 0) + if (node->AsField()->IsNeverNegative()) { return {SymbolicIntegerValue::Zero, UpperBoundForType(rangeType)}; } diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index ee283567b5566..5679281470a31 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 36db3d0177279..9748dfb49270b 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -8407,13 +8407,6 @@ 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: @@ -8441,9 +8434,9 @@ GenTree* Compiler::gtClone(GenTree* tree, bool complexOK) copy->AsField()->gtFieldLookup = tree->AsField()->gtFieldLookup; #endif - if ((tree->gtFlags & GTF_FLD_NEVER_NEGATIVE) != 0) + if (tree->AsField()->IsNeverNegative()) { - copy->gtFlags |= GTF_FLD_NEVER_NEGATIVE; + copy->AsField()->SetIsNeverNegative(true); } } else if (tree->OperIs(GT_ADD, GT_SUB)) @@ -8587,11 +8580,6 @@ 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; @@ -8609,9 +8597,6 @@ 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; @@ -8650,10 +8635,6 @@ GenTree* Compiler::gtCloneExpr( 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; } @@ -8661,10 +8642,6 @@ GenTree* Compiler::gtCloneExpr( { 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; } @@ -8805,9 +8782,9 @@ GenTree* Compiler::gtCloneExpr( copy->AsField()->gtFieldLookup = tree->AsField()->gtFieldLookup; #endif - if ((tree->gtFlags & GTF_FLD_NEVER_NEGATIVE) != 0) + if (tree->AsField()->IsNeverNegative()) { - copy->gtFlags |= GTF_FLD_NEVER_NEGATIVE; + copy->AsField()->SetIsNeverNegative(true); } break; @@ -24655,6 +24632,20 @@ ClassLayout* GenTreeLclVarCommon::GetLayout(Compiler* compiler) const return AsLclFld()->GetLayout(); } +//------------------------------------------------------------------------ +// GenTreeLclVarCommon::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 GenTreeLclVarCommon::IsNeverNegative(Compiler* comp) const +{ + 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. @@ -24748,6 +24739,24 @@ bool GenTree::IsNeverNegative(Compiler* comp) const { return AsIntConCommon()->IntegralValue() >= 0; } + + if (IsLocal()) + { + if (AsLclVarCommon()->IsNeverNegative(comp)) + { + // This is an early exit, it doesn't cover all cases + return true; + } + } + else if (OperIs(GT_FIELD)) + { + if (AsField()->IsNeverNegative()) + { + // 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 b2aecbbe4a22a..596ebe4a345e6 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -474,7 +474,6 @@ enum GenTreeFlags : unsigned int 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_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_* @@ -493,7 +492,6 @@ 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 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 @@ -3654,6 +3652,8 @@ struct GenTreeLclVarCommon : public GenTreeUnOp return m_ssaNum.IsComposite(); } + bool IsNeverNegative(Compiler* comp) const; + #if DEBUGGABLE_GENTREE GenTreeLclVarCommon() : GenTreeUnOp() { @@ -4006,7 +4006,12 @@ struct GenTreeField : public GenTreeUnOp { CORINFO_FIELD_HANDLE gtFldHnd; DWORD gtFldOffset; - bool gtFldMayOverlap; + bool gtFldMayOverlap : 1; + +private: + bool gtFldIsNeverNegative : 1; + +public: #ifdef FEATURE_READYTORUN CORINFO_CONST_LOOKUP gtFieldLookup; #endif @@ -4039,6 +4044,16 @@ struct GenTreeField : public GenTreeUnOp return (gtFlags & GTF_FLD_VOLATILE) != 0; } + bool IsNeverNegative() const + { + return gtFldIsNeverNegative; + } + + void SetIsNeverNegative(bool value) + { + gtFldIsNeverNegative = value; + } + bool IsInstance() const { return GetFldObj() != nullptr; diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 6e62cefc7fe40..7e17cecc4c7a5 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -2782,8 +2782,8 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, CORINFO_FIELD_HANDLE lengthHnd = info.compCompHnd->getFieldInClass(clsHnd, 1); const unsigned lengthOffset = info.compCompHnd->getFieldOffset(lengthHnd); - GenTree* length = gtNewFieldRef(TYP_INT, lengthHnd, ptrToSpan, lengthOffset); - length->gtFlags |= GTF_FLD_NEVER_NEGATIVE; + GenTreeField* length = gtNewFieldRef(TYP_INT, lengthHnd, ptrToSpan, lengthOffset); + length->SetIsNeverNegative(true); GenTree* boundsCheck = new (this, GT_BOUNDS_CHECK) GenTreeBoundsChk(index, length, SCK_RNGCHK_FAIL); @@ -2844,7 +2844,7 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, const unsigned lengthOffset = info.compCompHnd->getFieldOffset(lengthHnd); GenTreeField* fieldRef = gtNewFieldRef(TYP_INT, lengthHnd, ptrToSpan, lengthOffset); - fieldRef->gtFlags |= GTF_FLD_NEVER_NEGATIVE; + fieldRef->SetIsNeverNegative(true); return fieldRef; } diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp index 6e9153d4728bf..0bea95599d83f 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/src/coreclr/jit/lclmorph.cpp @@ -1392,9 +1392,9 @@ class LocalAddressVisitor final : public GenTreeVisitor { GenTreeFlags lclVarFlags = node->gtFlags & (GTF_NODE_MASK | GTF_DONT_CSE); - if (node->OperIs(GT_FIELD) && (node->gtFlags & GTF_FLD_NEVER_NEGATIVE)) + if (node->OperIs(GT_FIELD) && node->AsField()->IsNeverNegative()) { - lclVarFlags |= GTF_VAR_NEVER_NEGATIVE; + m_compiler->lvaGetDesc(fieldLclNum)->SetIsNeverNegative(true); } if ((user != nullptr) && user->OperIs(GT_ASG) && (user->AsOp()->gtOp1 == node)) From 820a5d854751bb5fb0cb63ecc14e311fbc78ae16 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 24 Jan 2023 08:24:58 -0800 Subject: [PATCH 3/9] Apply formatting patch --- src/coreclr/jit/gentree.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 596ebe4a345e6..ba33f65fae94f 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -4009,7 +4009,7 @@ struct GenTreeField : public GenTreeUnOp bool gtFldMayOverlap : 1; private: - bool gtFldIsNeverNegative : 1; + bool gtFldIsNeverNegative : 1; public: #ifdef FEATURE_READYTORUN From 618226ee47eb2ba3a627e92730344d5999ee2808 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 24 Jan 2023 08:39:37 -0800 Subject: [PATCH 4/9] Ensure lvIsNeverNegative is propagated to shadows --- src/coreclr/jit/gentree.h | 6 +++++- src/coreclr/jit/gschecks.cpp | 5 +++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index ba33f65fae94f..a97c179bad271 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -4017,7 +4017,11 @@ struct GenTreeField : public GenTreeUnOp #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) + , gtFldIsNeverNegative(false) { #ifdef FEATURE_READYTORUN gtFieldLookup.addr = 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) { From e5d4b1398e52c3564fb5cc320d94af04c2ca6fe2 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Wed, 25 Jan 2023 07:48:24 -0800 Subject: [PATCH 5/9] Update src/coreclr/jit/gentree.h Co-authored-by: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com> --- src/coreclr/jit/gentree.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index a97c179bad271..f4927ab0bb89e 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -470,10 +470,10 @@ 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. // For additional flags for GT_CALL node see GTF_CALL_M_* From d7006158474f492cc3efb6413854090879db6130 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Wed, 25 Jan 2023 08:07:10 -0800 Subject: [PATCH 6/9] Responding to PR feedback --- src/coreclr/jit/gentree.cpp | 8 ++++---- src/coreclr/jit/gentree.h | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 9748dfb49270b..9e13e80973568 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -24633,7 +24633,7 @@ ClassLayout* GenTreeLclVarCommon::GetLayout(Compiler* compiler) const } //------------------------------------------------------------------------ -// GenTreeLclVarCommon::IsNeverNegative: Gets true if the lcl var is never negative; otherwise false. +// GenTreeLclVar::IsNeverNegative: Gets true if the lcl var is never negative; otherwise false. // // Arguments: // comp - the compiler instance @@ -24641,7 +24641,7 @@ ClassLayout* GenTreeLclVarCommon::GetLayout(Compiler* compiler) const // Return Value: // true if the lcl var is never negative; otherwise false. // -bool GenTreeLclVarCommon::IsNeverNegative(Compiler* comp) const +bool GenTreeLclVar::IsNeverNegative(Compiler* comp) const { return comp->lvaGetDesc(GetLclNum())->IsNeverNegative(); } @@ -24740,9 +24740,9 @@ bool GenTree::IsNeverNegative(Compiler* comp) const return AsIntConCommon()->IntegralValue() >= 0; } - if (IsLocal()) + if (OperIs(GT_LCL_VAR)) { - if (AsLclVarCommon()->IsNeverNegative(comp)) + if (AsLclVar()->IsNeverNegative(comp)) { // This is an early exit, it doesn't cover all cases return true; diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index f4927ab0bb89e..03e10bca79cc1 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -3652,8 +3652,6 @@ struct GenTreeLclVarCommon : public GenTreeUnOp return m_ssaNum.IsComposite(); } - bool IsNeverNegative(Compiler* comp) const; - #if DEBUGGABLE_GENTREE GenTreeLclVarCommon() : GenTreeUnOp() { @@ -3796,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 // From 60ffb5a0742d3710f7604ceb9c4ceedacf5575f0 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Wed, 25 Jan 2023 08:12:45 -0800 Subject: [PATCH 7/9] Adding two asserts --- src/coreclr/jit/gentree.cpp | 1 + src/coreclr/jit/gentree.h | 1 + 2 files changed, 2 insertions(+) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 9e13e80973568..168022d08c884 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -24643,6 +24643,7 @@ ClassLayout* GenTreeLclVarCommon::GetLayout(Compiler* compiler) const // bool GenTreeLclVar::IsNeverNegative(Compiler* comp) const { + assert(OperIs(GT_LCL_VAR)); return comp->lvaGetDesc(GetLclNum())->IsNeverNegative(); } diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 03e10bca79cc1..3f7356218dabf 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -4050,6 +4050,7 @@ struct GenTreeField : public GenTreeUnOp bool IsNeverNegative() const { + assert(OperIs(GT_FIELD)); return gtFldIsNeverNegative; } From 2f882d802ec84afad58c5b277f9b9a99ec2a8a1b Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Wed, 25 Jan 2023 09:26:31 -0800 Subject: [PATCH 8/9] Rename GenTreeField::IsNeverNegative to IsSpanLength and add a comment --- src/coreclr/jit/assertionprop.cpp | 2 +- src/coreclr/jit/gentree.cpp | 10 +++++----- src/coreclr/jit/gentree.h | 19 +++++++++++++------ src/coreclr/jit/importercalls.cpp | 4 ++-- src/coreclr/jit/lclmorph.cpp | 2 +- 5 files changed, 22 insertions(+), 15 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 7b8b0f5c00ef3..12e5a977aff02 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -229,7 +229,7 @@ bool IntegralRange::Contains(int64_t value) const case GT_FIELD: { - if (node->AsField()->IsNeverNegative()) + if (node->AsField()->IsSpanLength()) { return {SymbolicIntegerValue::Zero, UpperBoundForType(rangeType)}; } diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 168022d08c884..a4fc60080de9c 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -8434,9 +8434,9 @@ GenTree* Compiler::gtClone(GenTree* tree, bool complexOK) copy->AsField()->gtFieldLookup = tree->AsField()->gtFieldLookup; #endif - if (tree->AsField()->IsNeverNegative()) + if (tree->AsField()->IsSpanLength()) { - copy->AsField()->SetIsNeverNegative(true); + copy->AsField()->SetIsSpanLength(true); } } else if (tree->OperIs(GT_ADD, GT_SUB)) @@ -8782,9 +8782,9 @@ GenTree* Compiler::gtCloneExpr( copy->AsField()->gtFieldLookup = tree->AsField()->gtFieldLookup; #endif - if (tree->AsField()->IsNeverNegative()) + if (tree->AsField()->IsSpanLength()) { - copy->AsField()->SetIsNeverNegative(true); + copy->AsField()->SetIsSpanLength(true); } break; @@ -24751,7 +24751,7 @@ bool GenTree::IsNeverNegative(Compiler* comp) const } else if (OperIs(GT_FIELD)) { - if (AsField()->IsNeverNegative()) + if (AsField()->IsSpanLength()) { // This is an early exit, it doesn't cover all cases return true; diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 3f7356218dabf..247d10bbb5b87 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -4009,7 +4009,7 @@ struct GenTreeField : public GenTreeUnOp bool gtFldMayOverlap : 1; private: - bool gtFldIsNeverNegative : 1; + bool gtFldIsSpanLength : 1; public: #ifdef FEATURE_READYTORUN @@ -4021,7 +4021,7 @@ struct GenTreeField : public GenTreeUnOp , gtFldHnd(fldHnd) , gtFldOffset(offs) , gtFldMayOverlap(false) - , gtFldIsNeverNegative(false) + , gtFldIsSpanLength(false) { #ifdef FEATURE_READYTORUN gtFieldLookup.addr = nullptr; @@ -4048,15 +4048,22 @@ struct GenTreeField : public GenTreeUnOp return (gtFlags & GTF_FLD_VOLATILE) != 0; } - bool IsNeverNegative() const + 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 gtFldIsNeverNegative; + return gtFldIsSpanLength; } - void SetIsNeverNegative(bool value) + void SetIsSpanLength(bool value) { - gtFldIsNeverNegative = value; + gtFldIsSpanLength = value; } bool IsInstance() const diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 7e17cecc4c7a5..8479ee433b4d4 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -2783,7 +2783,7 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, const unsigned lengthOffset = info.compCompHnd->getFieldOffset(lengthHnd); GenTreeField* length = gtNewFieldRef(TYP_INT, lengthHnd, ptrToSpan, lengthOffset); - length->SetIsNeverNegative(true); + length->SetIsSpanLength(true); GenTree* boundsCheck = new (this, GT_BOUNDS_CHECK) GenTreeBoundsChk(index, length, SCK_RNGCHK_FAIL); @@ -2844,7 +2844,7 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, const unsigned lengthOffset = info.compCompHnd->getFieldOffset(lengthHnd); GenTreeField* fieldRef = gtNewFieldRef(TYP_INT, lengthHnd, ptrToSpan, lengthOffset); - fieldRef->SetIsNeverNegative(true); + fieldRef->SetIsSpanLength(true); return fieldRef; } diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp index 0bea95599d83f..cb093a8569a44 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/src/coreclr/jit/lclmorph.cpp @@ -1392,7 +1392,7 @@ class LocalAddressVisitor final : public GenTreeVisitor { GenTreeFlags lclVarFlags = node->gtFlags & (GTF_NODE_MASK | GTF_DONT_CSE); - if (node->OperIs(GT_FIELD) && node->AsField()->IsNeverNegative()) + if (node->OperIs(GT_FIELD) && node->AsField()->IsSpanLength()) { m_compiler->lvaGetDesc(fieldLclNum)->SetIsNeverNegative(true); } From c224a3d051353ebe31c5276e8264d0b06996a078 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Wed, 25 Jan 2023 09:48:16 -0800 Subject: [PATCH 9/9] Fix an assert --- src/coreclr/jit/gentree.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index a4fc60080de9c..c25a1433e8d0b 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -8782,7 +8782,7 @@ GenTree* Compiler::gtCloneExpr( copy->AsField()->gtFieldLookup = tree->AsField()->gtFieldLookup; #endif - if (tree->AsField()->IsSpanLength()) + if ((oper == GT_FIELD) && tree->AsField()->IsSpanLength()) { copy->AsField()->SetIsSpanLength(true); }