diff --git a/src/coreclr/src/jit/assertionprop.cpp b/src/coreclr/src/jit/assertionprop.cpp index 7bac5584bf92c..4737418e53822 100644 --- a/src/coreclr/src/jit/assertionprop.cpp +++ b/src/coreclr/src/jit/assertionprop.cpp @@ -1625,7 +1625,6 @@ void Compiler::optDebugCheckAssertion(AssertionDsc* assertion) assert(assertion->op2.u1.iconFlags != 0); break; case O1K_LCLVAR: - case O1K_ARR_BND: assert((lvaTable[assertion->op1.lcl.lclNum].lvType != TYP_REF) || (assertion->op2.u1.iconVal == 0)); break; case O1K_VALUE_NUMBER: @@ -1959,12 +1958,34 @@ AssertionInfo Compiler::optAssertionGenJtrue(GenTree* tree) { std::swap(op1, op2); } + + ValueNum op1VN = vnStore->VNConservativeNormalValue(op1->gtVNPair); // If op1 is lcl and op2 is const or lcl, create assertion. if ((op1->gtOper == GT_LCL_VAR) && ((op2->OperKind() & GTK_CONST) || (op2->gtOper == GT_LCL_VAR))) // Fix for Dev10 851483 { return optCreateJtrueAssertions(op1, op2, assertionKind); } + else if (vnStore->IsVNCheckedBound(op1VN) && op2->OperIs(GT_CNS_INT)) + { + AssertionDsc dsc; + dsc.assertionKind = OAK_EQUAL; + dsc.op1.vn = vnStore->VNConservativeNormalValue(relop->gtVNPair); + dsc.op1.kind = O1K_ARR_BND; + dsc.op1.bnd.vnIdx = vnStore->VNForIntCon(op2->AsIntCon()->IntegralValue() - 1); + dsc.op1.bnd.vnLen = op1VN; + dsc.op2.vn = vnStore->VNConservativeNormalValue(op2->gtVNPair); + dsc.op2.kind = O2K_CONST_INT; + dsc.op2.u1.iconFlags = 0; + dsc.op2.u1.iconVal = 0; + + AssertionIndex index = optAddAssertion(&dsc); + if (relop->OperIs(GT_NE)) + { + return AssertionInfo::ForNextEdge(index); + } + return index; + } // Check op1 and op2 for an indirection of a GT_LCL_VAR and keep it in op1. if (((op1->gtOper != GT_IND) || (op1->AsOp()->gtOp1->gtOper != GT_LCL_VAR)) && diff --git a/src/coreclr/src/jit/rangecheck.cpp b/src/coreclr/src/jit/rangecheck.cpp index 66ebafb1c747a..26cab8457d95e 100644 --- a/src/coreclr/src/jit/rangecheck.cpp +++ b/src/coreclr/src/jit/rangecheck.cpp @@ -5,6 +5,7 @@ #include "jitpch.h" #include "rangecheck.h" +#include "compiler.h" // Max stack depth (path length) in walking the UD chain. static const int MAX_SEARCH_DEPTH = 100; @@ -60,7 +61,7 @@ int RangeCheck::GetArrLength(ValueNum vn) } // Check if the computed range is within bounds. -bool RangeCheck::BetweenBounds(Range& range, int lower, GenTree* upper) +bool RangeCheck::BetweenBounds(Range& range, int lower, ValueNum uLimitVN, int arrSize DEBUGARG(GenTree* upper)) { #ifdef DEBUG if (m_pCompiler->verbose) @@ -73,10 +74,8 @@ bool RangeCheck::BetweenBounds(Range& range, int lower, GenTree* upper) ValueNumStore* vnStore = m_pCompiler->vnStore; - // Get the VN for the upper limit. - ValueNum uLimitVN = vnStore->VNConservativeNormalValue(upper->gtVNPair); - #ifdef DEBUG + assert(vnStore->VNConservativeNormalValue(upper->gtVNPair)); JITDUMP(FMT_VN " upper bound is: ", uLimitVN); if (m_pCompiler->verbose) { @@ -85,26 +84,7 @@ bool RangeCheck::BetweenBounds(Range& range, int lower, GenTree* upper) JITDUMP("\n"); #endif - int arrSize = 0; - - if (vnStore->IsVNConstant(uLimitVN)) - { - ssize_t constVal = -1; - unsigned iconFlags = 0; - - if (m_pCompiler->optIsTreeKnownIntValue(true, upper, &constVal, &iconFlags)) - { - arrSize = (int)constVal; - } - } - else if (vnStore->IsVNArrLen(uLimitVN)) - { - // Get the array reference from the length. - ValueNum arrRefVN = vnStore->GetArrForLenVn(uLimitVN); - // Check if array size can be obtained. - arrSize = vnStore->GetNewArrSize(arrRefVN); - } - else if (!vnStore->IsVNCheckedBound(uLimitVN)) + if ((arrSize <= 0) && !vnStore->IsVNCheckedBound(uLimitVN)) { // If the upper limit is not length, then bail. return false; @@ -231,6 +211,16 @@ void RangeCheck::OptimizeRangeCheck(BasicBlock* block, Statement* stmt, GenTree* #endif // FEATURE_SIMD { arrSize = GetArrLength(arrLenVn); + if (arrSize <= 0) + { + // see if there are any assertions about the array size we can use + Range arrLength = Range(Limit(Limit::keDependent)); + MergeEdgeAssertions(arrLenVn, block->bbAssertionIn, &arrLength); + if (arrLength.lLimit.IsConstant()) + { + arrSize = arrLength.lLimit.GetConstant(); + } + } } JITDUMP("ArrSize for lengthVN:%03X = %d\n", arrLenVn, arrSize); @@ -286,7 +276,7 @@ void RangeCheck::OptimizeRangeCheck(BasicBlock* block, Statement* stmt, GenTree* } // Is the range between the lower and upper bound values. - if (BetweenBounds(range, 0, bndsChk->gtArrLen)) + if (BetweenBounds(range, 0, arrLenVn, arrSize DEBUGARG(bndsChk->gtArrLen))) { JITDUMP("[RangeCheck::OptimizeRangeCheck] Between bounds\n"); m_pCompiler->optRemoveRangeCheck(treeParent, stmt); @@ -532,7 +522,6 @@ void RangeCheck::SetDef(UINT64 hash, Location* loc) } #endif -// Merge assertions on the edge flowing into the block about a variable. void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP assertions, Range* pRange) { if (BitVecOps::IsEmpty(m_pCompiler->apTraits, assertions)) @@ -544,6 +533,20 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP { return; } + + LclVarDsc* varDsc = m_pCompiler->lvaGetDesc(lcl); + if (varDsc->CanBeReplacedWithItsField(m_pCompiler)) + { + varDsc = m_pCompiler->lvaGetDesc(varDsc->lvFieldLclStart); + } + LclSsaVarDsc* ssaData = varDsc->GetPerSsaData(lcl->GetSsaNum()); + ValueNum normalLclVN = m_pCompiler->vnStore->VNConservativeNormalValue(ssaData->m_vnPair); + MergeEdgeAssertions(normalLclVN, assertions, pRange); +} + +// Merge assertions on the edge flowing into the block about a variable. +void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP assertions, Range* pRange) +{ // Walk through the "assertions" to check if the apply. BitVecOps::Iter iter(m_pCompiler->apTraits, assertions); unsigned index = 0; @@ -556,14 +559,6 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP Limit limit(Limit::keUndef); genTreeOps cmpOper = GT_NONE; - LclVarDsc* varDsc = m_pCompiler->lvaGetDesc(lcl); - if (varDsc->CanBeReplacedWithItsField(m_pCompiler)) - { - varDsc = m_pCompiler->lvaGetDesc(varDsc->lvFieldLclStart); - } - LclSsaVarDsc* ssaData = varDsc->GetPerSsaData(lcl->GetSsaNum()); - ValueNum normalLclVN = m_pCompiler->vnStore->VNConservativeNormalValue(ssaData->m_vnPair); - // Current assertion is of the form (i < len - cns) != 0 if (curAssertion->IsCheckedBoundArithBound()) { @@ -602,13 +597,20 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP m_pCompiler->vnStore->GetCompareCheckedBound(curAssertion->op1.vn, &info); // If we don't have the same variable we are comparing against, bail. - if (normalLclVN != info.cmpOp) + if (normalLclVN == info.cmpOp) + { + cmpOper = (genTreeOps)info.cmpOper; + limit = Limit(Limit::keBinOpArray, info.vnBound, 0); + } + else if (info.vnBound == info.vnBound) + { + cmpOper = GenTree::SwapRelop((genTreeOps)info.cmpOper); + limit = Limit(Limit::keBinOpArray, info.cmpOp, 0); + } + else { continue; } - - limit = Limit(Limit::keBinOpArray, info.vnBound, 0); - cmpOper = (genTreeOps)info.cmpOper; } // Current assertion is of the form (i < 100) != 0 else if (curAssertion->IsConstantBound()) @@ -627,6 +629,11 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP limit = Limit(Limit::keConstant, info.constVal); cmpOper = (genTreeOps)info.cmpOper; } + else if (IsConstantAssertion(curAssertion, normalLclVN)) + { + limit = Limit(Limit::keConstant, m_pCompiler->vnStore->ConstantValue(curAssertion->op2.vn)); + cmpOper = GT_EQ; + } // Current assertion is not supported, ignore it else { @@ -636,7 +643,8 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP assert(limit.IsBinOpArray() || limit.IsConstant()); // Make sure the assertion is of the form != 0 or == 0. - if (curAssertion->op2.vn != m_pCompiler->vnStore->VNZeroForType(TYP_INT)) + if ((curAssertion->op2.vn != m_pCompiler->vnStore->VNZeroForType(TYP_INT)) && + (cmpOper != GT_EQ)) { continue; } @@ -647,6 +655,13 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP } #endif + // Limits are sometimes made with the form vn + constant, where vn is a known constant + // see if we can simplify this to just a constant + if (limit.IsBinOpArray() && m_pCompiler->vnStore->IsVNInt32Constant(limit.vn)) + { + limit = Limit(Limit::keConstant, m_pCompiler->vnStore->ConstantValue(limit.vn) + limit.cns); + } + ValueNum arrLenVN = m_pCompiler->vnStore->VNConservativeNormalValue(m_pCurBndsChk->gtArrLen->gtVNPair); if (m_pCompiler->vnStore->IsVNConstant(arrLenVN)) @@ -663,6 +678,7 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP // (i < length) != 0 // (i < 100) == 0 // (i < 100) != 0 + // i == 100 // // At this point, we have detected that op1.vn is (i < length) or (i < length + cns) or // (i < 100) and the op2.vn is 0. @@ -673,12 +689,12 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP // If we have an assertion of the form == 0 (i.e., equals false), then reverse relop. // The relop has to be reversed because we have: (i < length) is false which is the same // as (i >= length). - if (curAssertion->assertionKind == Compiler::OAK_EQUAL) + if ((curAssertion->assertionKind == Compiler::OAK_EQUAL) && (cmpOper != GT_EQ)) { cmpOper = GenTree::ReverseRelop(cmpOper); } - // Bounds are inclusive, so add -1 for upper bound when "<". But make sure we won't overflow. + // Bounds are inclusive, so add -1 for upper bound when "<". But make sure we won't underflow. if (cmpOper == GT_LT && !limit.AddConstant(-1)) { continue; @@ -744,6 +760,10 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP pRange->lLimit = limit; break; + case GT_EQ: + pRange->uLimit = limit; + pRange->lLimit = limit; + default: // All other 'cmpOper' kinds leave lLimit/uLimit unchanged break; diff --git a/src/coreclr/src/jit/rangecheck.h b/src/coreclr/src/jit/rangecheck.h index 754f64cbc2ea6..ac013a4558c73 100644 --- a/src/coreclr/src/jit/rangecheck.h +++ b/src/coreclr/src/jit/rangecheck.h @@ -165,6 +165,7 @@ struct Limit } return false; } + #ifdef DEBUG const char* ToString(CompAllocator alloc) { @@ -376,6 +377,21 @@ struct RangeOps result.uLimit = r2hi; } } + if (r1lo.IsBinOpArray() && r2lo.IsBinOpArray() && r1lo.vn == r2lo.vn) + { + result.lLimit = r2lo.GetConstant() < r1lo.GetConstant() ? r2lo : r1lo; + } + + if (r1lo.IsConstant() && r1lo.GetConstant() >= 0 && r2lo.IsBinOpArray() && + r2lo.GetConstant() >= r1lo.GetConstant()) + { + result.lLimit = r1lo; + } + else if (r2lo.IsConstant() && r2lo.GetConstant() >= 0 && r1lo.IsBinOpArray() && + r1lo.GetConstant() >= r2lo.GetConstant()) + { + result.lLimit = r2lo; + } return result; } }; @@ -438,7 +454,7 @@ class RangeCheck // assumes that the lower range is resolved and upper range is symbolic as in an // increasing loop. // TODO-CQ: This is not general enough. - bool BetweenBounds(Range& range, int lower, GenTree* upper); + bool BetweenBounds(Range& range, int lower, ValueNum uLimitVN, int arrSize DEBUGARG(GenTree* upper)); // Entry point to optimize range checks in the block. Assumes value numbering // and assertion prop phases are completed. @@ -474,6 +490,8 @@ class RangeCheck // refine the "pRange" value. void MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP assertions, Range* pRange); + void MergeEdgeAssertions(ValueNum num, ASSERT_VALARG_TP, Range* pRange); + // The maximum possible value of the given "limit." If such a value could not be determined // return "false." For example: ARRLEN_MAX for array length. bool GetLimitMax(Limit& limit, int* pMax); @@ -519,6 +537,13 @@ class RangeCheck GenTreeBoundsChk* m_pCurBndsChk; + // Is the given assertion a constant assertion with the given vn + bool IsConstantAssertion(Compiler::AssertionDsc* dsc, ValueNum vn) + { + return (dsc->assertionKind == Compiler::OAK_EQUAL) && (dsc->op1.vn == vn) + && (m_pCompiler->vnStore->IsVNInt32Constant(dsc->op2.vn)); + } + // Get the cached overflow values. OverflowMap* GetOverflowMap(); OverflowMap* m_pOverflowMap; diff --git a/src/libraries/System.Private.CoreLib/src/System/String.cs b/src/libraries/System.Private.CoreLib/src/System/String.cs index 54333a3c215d9..510c1fe97f126 100644 --- a/src/libraries/System.Private.CoreLib/src/System/String.cs +++ b/src/libraries/System.Private.CoreLib/src/System/String.cs @@ -487,13 +487,9 @@ public char[] ToCharArray(int startIndex, int length) [NonVersionable] public static bool IsNullOrEmpty([NotNullWhen(false)] string? value) { - // Using 0u >= (uint)value.Length rather than - // value.Length == 0 as it will elide the bounds check to - // the first char: value[0] if that is performed following the test - // for the same test cost. // Ternary operator returning true/false prevents redundant asm generation: // https://github.com/dotnet/runtime/issues/4207 - return (value == null || 0u >= (uint)value.Length) ? true : false; + return (value == null || 0 == value.Length) ? true : false; } public static bool IsNullOrWhiteSpace([NotNullWhen(false)] string? value) diff --git a/src/tests/JIT/opt/AssertionPropagation/ArrBoundMinLength.cs b/src/tests/JIT/opt/AssertionPropagation/ArrBoundMinLength.cs new file mode 100644 index 0000000000000..758a935c06a63 --- /dev/null +++ b/src/tests/JIT/opt/AssertionPropagation/ArrBoundMinLength.cs @@ -0,0 +1,288 @@ + +using System; + +class Program +{ + private static int returnCode = 100; + + private readonly static int[] arr = new int[6]; + + public static int Main(string[] args) + { + RunTestThrows(Tests.GreaterOutOfBound); + RunTestThrows(Tests.GreaterEqualOutOfBound); + RunTestThrows(Tests.LessOutOfBound); + RunTestThrows(Tests.LessEqualsOutOfBound); + RunTestThrows(Tests.EqualsOutOfBound); + RunTestThrows(Tests.EqualsReversedOutOfBound); + RunTestThrows(Tests.NotEqualsOutOfBound); + RunTestThrows(TestsEarlyReturn.GreaterOutOfBound); + RunTestThrows(TestsEarlyReturn.GreaterEqualOutOfBound); + RunTestThrows(TestsEarlyReturn.LessOutOfBound); + RunTestThrows(TestsEarlyReturn.LessEqualsOutOfBound); + RunTestThrows(TestsEarlyReturn.NotEqualsOutOfBound); + RunTestThrows(TestsEarlyReturn.EqualsOutOfBound); + + RunTestNoThrow(Tests.GreaterInBound); + RunTestNoThrow(Tests.GreaterEqualInBound); + RunTestNoThrow(Tests.LessInBound); + RunTestNoThrow(Tests.EqualsInBound); + RunTestNoThrow(Tests.LessEqualsInBound); + RunTestNoThrow(Tests.EqualsInBound); + RunTestNoThrow(Tests.EqualsReversedInBound); + RunTestNoThrow(TestsEarlyReturn.GreaterInBound); + RunTestNoThrow(TestsEarlyReturn.GreaterEqualInBound); + RunTestNoThrow(TestsEarlyReturn.LessInBound); + RunTestNoThrow(TestsEarlyReturn.LessEqualsInBound); + RunTestNoThrow(TestsEarlyReturn.NotEqualsInBound); + + return returnCode; + } + + private static void RunTestThrows(Action action) + { + try + { + action(arr); + Console.WriteLine("failed " + action.Method.Name); + returnCode--; + } + catch (Exception) + { + + } + } + + private static void RunTestNoThrow(Action action) + { + try + { + action(arr); + } + catch (Exception) + { + Console.WriteLine("failed " + action.Method.Name); + returnCode--; + } + } +} + +public static class Tests +{ + public static void GreaterInBound(int[] arr) + { + if (arr.Length > 5) + { + arr[5] = 1; + } + } + + public static void GreaterOutOfBound(int[] arr) + { + if (arr.Length > 5) + { + arr[6] = 1; + } + } + + public static void GreaterEqualInBound(int[] arr) + { + if (arr.Length >= 6) + { + arr[5] = 1; + } + } + + public static void GreaterEqualOutOfBound(int[] arr) + { + if (arr.Length >= 5) + { + arr[6] = 1; + } + } + + public static void LessInBound(int[] arr) + { + if (5 < arr.Length) + { + arr[5] = 1; + } + } + + public static void LessOutOfBound(int[] arr) + { + if (5 < arr.Length) + { + arr[6] = 1; + } + } + + public static void LessEqualsInBound(int[] arr) + { + if (5 <= arr.Length) + { + arr[4] = 1; + } + } + + public static void LessEqualsOutOfBound(int[] arr) + { + if (5 <= arr.Length) + { + arr[6] = 1; + } + } + + public static void EqualsInBound(int[] arr) + { + if (arr.Length == 6) + { + arr[5] = 1; + } + } + + public static void EqualsReversedInBound(int[] arr) + { + if (6 == arr.Length) + { + arr[5] = 1; + } + } + + public static void EqualsOutOfBound(int[] arr) + { + if (arr.Length == 6) + { + arr[6] = 1; + } + } + + public static void EqualsReversedOutOfBound(int[] arr) + { + if (6 == arr.Length) + { + arr[6] = 1; + } + } + + public static void NotEqualsOutOfBound(int[] arr) + { + if (arr.Length != 5) + { + arr[6] = 1; + } + } +} + +public static class TestsEarlyReturn +{ + public static void GreaterInBound(int[] arr) + { + if (arr.Length <= 5) + { + return; + } + + arr[5] = 1; + } + + public static void GreaterOutOfBound(int[] arr) + { + if (arr.Length <= 5) + { + return; + } + + arr[6] = 1; + } + + public static void GreaterEqualInBound(int[] arr) + { + if (arr.Length < 5) + { + return; + } + + arr[4] = 1; + } + + public static void GreaterEqualOutOfBound(int[] arr) + { + if (arr.Length < 5) + { + return; + } + + arr[6] = 1; + } + + public static void LessInBound(int[] arr) + { + if (5 >= arr.Length) + { + return; + } + + arr[5] = 1; + } + + public static void LessOutOfBound(int[] arr) + { + if (5 >= arr.Length) + { + return; + } + + arr[6] = 1; + } + + public static void LessEqualsInBound(int[] arr) + { + if (5 > arr.Length) + { + return; + } + + arr[4] = 1; + } + + public static void LessEqualsOutOfBound(int[] arr) + { + if (5 > arr.Length) + { + return; + } + + arr[6] = 1; + } + + public static void NotEqualsInBound(int[] arr) + { + if (arr.Length != 6) + { + return; + } + + arr[5] = 1; + } + + public static void NotEqualsOutOfBound(int[] arr) + { + if (arr.Length != 6) + { + return; + } + + arr[6] = 1; + } + + public static void EqualsOutOfBound(int[] arr) + { + if (arr.Length == 5) + { + return; + } + + arr[6] = 1; + } +} \ No newline at end of file diff --git a/src/tests/JIT/opt/AssertionPropagation/ArrBoundMinLength.csproj b/src/tests/JIT/opt/AssertionPropagation/ArrBoundMinLength.csproj new file mode 100644 index 0000000000000..5c20dcadd57ca --- /dev/null +++ b/src/tests/JIT/opt/AssertionPropagation/ArrBoundMinLength.csproj @@ -0,0 +1,12 @@ + + + Exe + + + None + True + + + + +