From 500c199cb6ef2c5642e16884e47b1237d9f7d85c Mon Sep 17 00:00:00 2001 From: Nathan Moore Date: Fri, 8 May 2020 18:07:53 -0400 Subject: [PATCH 1/6] Fix case of subtracted length --- src/coreclr/src/jit/assertionprop.cpp | 19 ++++++++++++++++++- src/coreclr/src/jit/rangecheck.cpp | 2 +- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/coreclr/src/jit/assertionprop.cpp b/src/coreclr/src/jit/assertionprop.cpp index 031d4867d87cf5..2adc80f10dba80 100644 --- a/src/coreclr/src/jit/assertionprop.cpp +++ b/src/coreclr/src/jit/assertionprop.cpp @@ -854,7 +854,7 @@ AssertionIndex Compiler::optCreateAssertion(GenTree* op1, if (op2 == nullptr) { // - // Must an OAK_NOT_EQUAL assertion + // Must be an OAK_NOT_EQUAL assertion // noway_assert(assertionKind == OAK_NOT_EQUAL); @@ -1796,6 +1796,23 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) optCreateComplementaryAssertion(index, nullptr, nullptr); return index; } + // Cases where op1 holds the ulhs of the condition and op2 holds the bound arithmatic. + // Loop condition like: "i < bnd +/-k" + // Assertion: "i < bnd +/- k != 0" + else if (vnStore->IsVNCompareCheckedBoundArith(relopVN)) + { + AssertionDsc dsc; + dsc.assertionKind = OAK_NOT_EQUAL; + dsc.op1.kind = O1K_BOUND_OPER_BND; + dsc.op1.vn = relopVN; + dsc.op2.kind = O2K_CONST_INT; + dsc.op2.vn = vnStore->VNZeroForType(op2->TypeGet()); + dsc.op2.u1.iconVal = 0; + dsc.op2.u1.iconFlags = 0; + AssertionIndex index = optAddAssertion(&dsc); + optCreateComplementaryAssertion(index, nullptr, nullptr); + return index; + } // Cases where op1 holds the upper bound and op2 is 0. // Loop condition like: "i < bnd == 0" // Assertion: "i < bnd == false" diff --git a/src/coreclr/src/jit/rangecheck.cpp b/src/coreclr/src/jit/rangecheck.cpp index 3019824427f266..141e33cfd657e4 100644 --- a/src/coreclr/src/jit/rangecheck.cpp +++ b/src/coreclr/src/jit/rangecheck.cpp @@ -729,7 +729,7 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP } // Merge assertions from the pred edges of the block, i.e., check for any assertions about "op's" value numbers for phi -// arguments. If not a phi argument, check if we assertions about local variables. +// arguments. If not a phi argument, check if we have assertions about local variables. void RangeCheck::MergeAssertion(BasicBlock* block, GenTree* op, Range* pRange DEBUGARG(int indent)) { JITDUMP("Merging assertions from pred edges of " FMT_BB " for op [%06d] " FMT_VN "\n", block->bbNum, From 52318e9684005e069e714884f3c373094df41fc6 Mon Sep 17 00:00:00 2001 From: Nathan Moore Date: Fri, 8 May 2020 22:10:18 -0400 Subject: [PATCH 2/6] Handle GT_COMMA --- src/coreclr/src/jit/rangecheck.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/coreclr/src/jit/rangecheck.cpp b/src/coreclr/src/jit/rangecheck.cpp index 141e33cfd657e4..0146c9b0292a9a 100644 --- a/src/coreclr/src/jit/rangecheck.cpp +++ b/src/coreclr/src/jit/rangecheck.cpp @@ -420,6 +420,10 @@ bool RangeCheck::IsMonotonicallyIncreasing(GenTree* expr, bool rejectNegativeCon } return true; } + else if (expr->OperGet() == GT_COMMA) + { + return IsMonotonicallyIncreasing(expr->gtEffectiveVal(), rejectNegativeConst); + } JITDUMP("Unknown tree type\n"); return false; } @@ -1217,6 +1221,10 @@ Range RangeCheck::ComputeRange(BasicBlock* block, GenTree* expr, bool monIncreas JITDUMP("%s\n", range.ToString(m_pCompiler->getAllocatorDebugOnly())); } + else if(expr->OperGet() == GT_COMMA) + { + range = GetRange(block, expr->gtEffectiveVal(), monIncreasing DEBUGARG(indent + 1)); + } else { // The expression is not recognized, so the result is unknown. From 417792c28be8ddae9398be6233c7e8da76c77479 Mon Sep 17 00:00:00 2001 From: Nathan Moore Date: Sat, 9 May 2020 12:49:35 -0400 Subject: [PATCH 3/6] Added addional logging around pessimistic check --- src/coreclr/src/jit/rangecheck.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/coreclr/src/jit/rangecheck.cpp b/src/coreclr/src/jit/rangecheck.cpp index 0146c9b0292a9a..be350d7cd25875 100644 --- a/src/coreclr/src/jit/rangecheck.cpp +++ b/src/coreclr/src/jit/rangecheck.cpp @@ -348,7 +348,13 @@ bool RangeCheck::IsBinOpMonotonicallyIncreasing(GenTreeOp* binop) return IsMonotonicallyIncreasing(op1, true) && IsMonotonicallyIncreasing(op2, true); case GT_CNS_INT: - return (op2->AsIntConCommon()->IconValue() >= 0) && IsMonotonicallyIncreasing(op1, false); + if (op2->AsIntConCommon()->IconValue() < 0) + { + JITDUMP("Not monotonically increasing because of encountered negative constant"); + return false; + } + + return IsMonotonicallyIncreasing(op1, false); default: JITDUMP("Not monotonically increasing because expression is not recognized.\n"); From 85c7cf588c3cc95950de3819bbb450b6236b6456 Mon Sep 17 00:00:00 2001 From: Nathan Moore Date: Mon, 11 May 2020 22:08:38 -0400 Subject: [PATCH 4/6] Fix up comments --- src/coreclr/src/jit/assertionprop.cpp | 2 +- src/coreclr/src/jit/rangecheck.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/src/jit/assertionprop.cpp b/src/coreclr/src/jit/assertionprop.cpp index 2adc80f10dba80..c6fb628664cb19 100644 --- a/src/coreclr/src/jit/assertionprop.cpp +++ b/src/coreclr/src/jit/assertionprop.cpp @@ -1796,7 +1796,7 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) optCreateComplementaryAssertion(index, nullptr, nullptr); return index; } - // Cases where op1 holds the ulhs of the condition and op2 holds the bound arithmatic. + // Cases where op1 holds the lhs of the condition and op2 holds the bound arithmatic. // Loop condition like: "i < bnd +/-k" // Assertion: "i < bnd +/- k != 0" else if (vnStore->IsVNCompareCheckedBoundArith(relopVN)) diff --git a/src/coreclr/src/jit/rangecheck.cpp b/src/coreclr/src/jit/rangecheck.cpp index be350d7cd25875..8a68bf705c93a5 100644 --- a/src/coreclr/src/jit/rangecheck.cpp +++ b/src/coreclr/src/jit/rangecheck.cpp @@ -350,7 +350,7 @@ bool RangeCheck::IsBinOpMonotonicallyIncreasing(GenTreeOp* binop) case GT_CNS_INT: if (op2->AsIntConCommon()->IconValue() < 0) { - JITDUMP("Not monotonically increasing because of encountered negative constant"); + JITDUMP("Not monotonically increasing because of encountered negative constant\n"); return false; } From 939aa31f9477329b72332d224cdf7b13c49437c4 Mon Sep 17 00:00:00 2001 From: Nathan Moore Date: Wed, 13 May 2020 10:29:18 -0400 Subject: [PATCH 5/6] formating --- src/coreclr/src/jit/rangecheck.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/src/jit/rangecheck.cpp b/src/coreclr/src/jit/rangecheck.cpp index 8a68bf705c93a5..c4fcc85b07b476 100644 --- a/src/coreclr/src/jit/rangecheck.cpp +++ b/src/coreclr/src/jit/rangecheck.cpp @@ -1227,7 +1227,7 @@ Range RangeCheck::ComputeRange(BasicBlock* block, GenTree* expr, bool monIncreas JITDUMP("%s\n", range.ToString(m_pCompiler->getAllocatorDebugOnly())); } - else if(expr->OperGet() == GT_COMMA) + else if (expr->OperGet() == GT_COMMA) { range = GetRange(block, expr->gtEffectiveVal(), monIncreasing DEBUGARG(indent + 1)); } From 57c3bace08815848448058d2912a2af9eb808157 Mon Sep 17 00:00:00 2001 From: Nathan Moore Date: Wed, 3 Jun 2020 12:06:27 -0400 Subject: [PATCH 6/6] Typo --- src/coreclr/src/jit/assertionprop.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/src/jit/assertionprop.cpp b/src/coreclr/src/jit/assertionprop.cpp index c6fb628664cb19..450f98018dbbef 100644 --- a/src/coreclr/src/jit/assertionprop.cpp +++ b/src/coreclr/src/jit/assertionprop.cpp @@ -1796,7 +1796,7 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) optCreateComplementaryAssertion(index, nullptr, nullptr); return index; } - // Cases where op1 holds the lhs of the condition and op2 holds the bound arithmatic. + // Cases where op1 holds the lhs of the condition and op2 holds the bound arithmetic. // Loop condition like: "i < bnd +/-k" // Assertion: "i < bnd +/- k != 0" else if (vnStore->IsVNCompareCheckedBoundArith(relopVN))