From ebc22889f6d35cba53c1e4598aba56f869cdf041 Mon Sep 17 00:00:00 2001 From: Anthony Canino Date: Tue, 9 Nov 2021 11:20:51 -0500 Subject: [PATCH] Add additional binary operations into the RangeCheck analysis. GT_LSH and GT_MUL are now covered in the range analysis check. This allows to catch and eliminate the range check for cases like ``` ReadOnlySpan readOnlySpan => new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 }; byte byt = 0; for (int i = 0; i < 5; i++) { byt = readOnlySpan[i * 3]; // ... } ``` or ``` bool[] flags = new bool[Size + 1]; for (int i = 2; i <= Size; i++) { for (int k = i * 2; k <= Size; k += i) { flags[k] = false; } } ``` Note that without this change, the previous snippet would not eliminate the range check on `flags[k]`, but the equivalent snippet would ``` for (int i = 2; i <= Size; i++) { for (int k = i + i; k <= Size; k += i) { flags[k] = false; } } ``` as additional was implemented in the range check analysis, but multiply was not. --- src/coreclr/jit/rangecheck.cpp | 85 +++++++++++++++++++----- src/coreclr/jit/rangecheck.h | 115 +++++++++++++++++++++++++++++++++ 2 files changed, 183 insertions(+), 17 deletions(-) diff --git a/src/coreclr/jit/rangecheck.cpp b/src/coreclr/jit/rangecheck.cpp index 4b957a720adc3..269cdf1f65386 100644 --- a/src/coreclr/jit/rangecheck.cpp +++ b/src/coreclr/jit/rangecheck.cpp @@ -336,18 +336,20 @@ void RangeCheck::Widen(BasicBlock* block, GenTree* tree, Range* pRange) bool RangeCheck::IsBinOpMonotonicallyIncreasing(GenTreeOp* binop) { - assert(binop->OperIs(GT_ADD)); + assert(binop->OperIs(GT_ADD) || binop->OperIs(GT_MUL) || binop->OperIs(GT_LSH)); GenTree* op1 = binop->gtGetOp1(); GenTree* op2 = binop->gtGetOp2(); JITDUMP("[RangeCheck::IsBinOpMonotonicallyIncreasing] [%06d], [%06d]\n", Compiler::dspTreeID(op1), Compiler::dspTreeID(op2)); - // Check if we have a var + const. - if (op2->OperGet() == GT_LCL_VAR) + + // Check if we have a var + const or var * const. + if (binop->OperIs(GT_ADD, GT_MUL) && op2->OperGet() == GT_LCL_VAR) { std::swap(op1, op2); } + if (op1->OperGet() != GT_LCL_VAR) { JITDUMP("Not monotonically increasing because op1 is not lclVar.\n"); @@ -417,7 +419,7 @@ bool RangeCheck::IsMonotonicallyIncreasing(GenTree* expr, bool rejectNegativeCon return (ssaDef != nullptr) && IsMonotonicallyIncreasing(ssaDef->GetAssignment()->gtGetOp2(), rejectNegativeConst); } - else if (expr->OperGet() == GT_ADD) + else if (expr->OperGet() == GT_ADD || expr->OperGet() == GT_MUL || expr->OperGet() == GT_LSH) { return IsBinOpMonotonicallyIncreasing(expr->AsOp()); } @@ -894,11 +896,12 @@ void RangeCheck::MergeAssertion(BasicBlock* block, GenTree* op, Range* pRange DE // Compute the range for a binary operation. Range RangeCheck::ComputeRangeForBinOp(BasicBlock* block, GenTreeOp* binop, bool monIncreasing DEBUGARG(int indent)) { - assert(binop->OperIs(GT_ADD, GT_AND, GT_RSH, GT_LSH, GT_UMOD)); + assert(binop->OperIs(GT_ADD, GT_AND, GT_RSH, GT_LSH, GT_UMOD, GT_MUL)); GenTree* op1 = binop->gtGetOp1(); GenTree* op2 = binop->gtGetOp2(); + // Special cases for binops where op2 is a constant if (binop->OperIs(GT_AND, GT_RSH, GT_LSH, GT_UMOD)) { if (!op2->IsIntCnsFitsInI32()) @@ -929,17 +932,21 @@ Range RangeCheck::ComputeRangeForBinOp(BasicBlock* block, GenTreeOp* binop, bool } } - if (icon < 0) + if (icon >= 0) + { + Range range(Limit(Limit::keConstant, 0), Limit(Limit::keConstant, icon)); + JITDUMP("Limit range to %s\n", range.ToString(m_pCompiler->getAllocatorDebugOnly())); + return range; + } + // Generalized range computation not implemented for these operators + else if (binop->OperIs(GT_AND, GT_UMOD, GT_RSH)) { return Range(Limit::keUnknown); } - Range range(Limit(Limit::keConstant, 0), Limit(Limit::keConstant, icon)); - JITDUMP("Limit range to %s\n", range.ToString(m_pCompiler->getAllocatorDebugOnly())); - return range; } // other operators are expected to be handled above. - assert(binop->OperIs(GT_ADD)); + assert(binop->OperIs(GT_ADD, GT_MUL, GT_LSH)); Range* op1RangeCached = nullptr; Range op1Range = Limit(Limit::keUndef); @@ -985,9 +992,30 @@ Range RangeCheck::ComputeRangeForBinOp(BasicBlock* block, GenTreeOp* binop, bool op2Range = *op2RangeCached; } - Range r = RangeOps::Add(op1Range, op2Range); - JITDUMP("BinOp add ranges %s %s = %s\n", op1Range.ToString(m_pCompiler->getAllocatorDebugOnly()), - op2Range.ToString(m_pCompiler->getAllocatorDebugOnly()), r.ToString(m_pCompiler->getAllocatorDebugOnly())); + Range r = Range(Limit::keUnknown); + if (binop->OperIs(GT_ADD)) + { + r = RangeOps::Add(op1Range, op2Range); + JITDUMP("BinOp add ranges %s %s = %s\n", op1Range.ToString(m_pCompiler->getAllocatorDebugOnly()), + op2Range.ToString(m_pCompiler->getAllocatorDebugOnly()), + r.ToString(m_pCompiler->getAllocatorDebugOnly())); + } + else if (binop->OperIs(GT_MUL)) + { + r = RangeOps::Multiply(op1Range, op2Range); + JITDUMP("BinOp multiply ranges %s %s = %s\n", op1Range.ToString(m_pCompiler->getAllocatorDebugOnly()), + op2Range.ToString(m_pCompiler->getAllocatorDebugOnly()), + r.ToString(m_pCompiler->getAllocatorDebugOnly())); + } + else if (binop->OperIs(GT_LSH)) + { + // help the next step a bit, convert the LSH rhs to a multiply + Range convertedOp2Range = RangeOps::ConvertShiftToMultiply(op2Range); + r = RangeOps::Multiply(op1Range, convertedOp2Range); + JITDUMP("BinOp multiply ranges %s %s = %s\n", op1Range.ToString(m_pCompiler->getAllocatorDebugOnly()), + convertedOp2Range.ToString(m_pCompiler->getAllocatorDebugOnly()), + r.ToString(m_pCompiler->getAllocatorDebugOnly())); + } return r; } @@ -1077,6 +1105,24 @@ bool RangeCheck::AddOverflows(Limit& limit1, Limit& limit2) return IntAddOverflows(max1, max2); } +// Check if the arithmetic overflows. +bool RangeCheck::MultiplyOverflows(Limit& limit1, Limit& limit2) +{ + int max1; + if (!GetLimitMax(limit1, &max1)) + { + return true; + } + + int max2; + if (!GetLimitMax(limit2, &max2)) + { + return true; + } + + return IntMultiplyOverflows(max1, max2); +} + // Does the bin operation overflow. bool RangeCheck::DoesBinOpOverflow(BasicBlock* block, GenTreeOp* binop) { @@ -1109,10 +1155,15 @@ bool RangeCheck::DoesBinOpOverflow(BasicBlock* block, GenTreeOp* binop) JITDUMP("Checking bin op overflow %s %s\n", op1Range->ToString(m_pCompiler->getAllocatorDebugOnly()), op2Range->ToString(m_pCompiler->getAllocatorDebugOnly())); - if (!AddOverflows(op1Range->UpperLimit(), op2Range->UpperLimit())) + if (binop->OperIs(GT_ADD)) { - return false; + return AddOverflows(op1Range->UpperLimit(), op2Range->UpperLimit()); } + else if (binop->OperIs(GT_MUL)) + { + return MultiplyOverflows(op1Range->UpperLimit(), op2Range->UpperLimit()); + } + return true; } @@ -1180,7 +1231,7 @@ bool RangeCheck::ComputeDoesOverflow(BasicBlock* block, GenTree* expr) overflows = DoesVarDefOverflow(expr->AsLclVarCommon()); } // Check if add overflows. - else if (expr->OperGet() == GT_ADD) + else if (expr->OperGet() == GT_ADD || expr->OperGet() == GT_MUL) { overflows = DoesBinOpOverflow(block, expr->AsOp()); } @@ -1276,7 +1327,7 @@ Range RangeCheck::ComputeRange(BasicBlock* block, GenTree* expr, bool monIncreas MergeAssertion(block, expr, &range DEBUGARG(indent + 1)); } // compute the range for binary operation - else if (expr->OperIs(GT_ADD, GT_AND, GT_RSH, GT_LSH, GT_UMOD)) + else if (expr->OperIs(GT_ADD, GT_AND, GT_RSH, GT_LSH, GT_UMOD, GT_MUL)) { range = ComputeRangeForBinOp(block, expr->AsOp(), monIncreasing DEBUGARG(indent + 1)); } diff --git a/src/coreclr/jit/rangecheck.h b/src/coreclr/jit/rangecheck.h index 751b243740c40..61f6c7657d1e7 100644 --- a/src/coreclr/jit/rangecheck.h +++ b/src/coreclr/jit/rangecheck.h @@ -72,6 +72,14 @@ static bool IntAddOverflows(int max1, int max2) return false; } +static bool IntMultiplyOverflows(int max1, int max2) +{ + if (max1 == 0 || max2 == 0) + return false; + int res = max1 * max2; + return !(max1 == res / max2); +} + struct Limit { enum LimitType @@ -147,6 +155,28 @@ struct Limit return false; } + bool MultiplyConstant(int i) + { + switch (type) + { + case keDependent: + return true; + case keBinOpArray: + case keConstant: + if (IntMultiplyOverflows(cns, i)) + { + return false; + } + cns *= i; + return true; + case keUndef: + case keUnknown: + // For these values of 'type', conservatively return false + break; + } + + return false; + } bool Equals(Limit& l) { @@ -250,6 +280,21 @@ struct RangeOps } } + // Given a constant limit in "l1", multiply it to l2 and mutate "l2". + static Limit MultiplyConstantLimit(Limit& l1, Limit& l2) + { + assert(l1.IsConstant()); + Limit l = l2; + if (l.MultiplyConstant(l1.GetConstant())) + { + return l; + } + else + { + return Limit(Limit::keUnknown); + } + } + // Given two ranges "r1" and "r2", perform an add operation on the // ranges. static Range Add(Range& r1, Range& r2) @@ -291,6 +336,47 @@ struct RangeOps return result; } + // Given two ranges "r1" and "r2", perform an multiply operation on the + // ranges. + static Range Multiply(Range& r1, Range& r2) + { + Limit& r1lo = r1.LowerLimit(); + Limit& r1hi = r1.UpperLimit(); + Limit& r2lo = r2.LowerLimit(); + Limit& r2hi = r2.UpperLimit(); + + Range result = Limit(Limit::keUnknown); + + // Check lo ranges if they are dependent and not unknown. + if ((r1lo.IsDependent() && !r1lo.IsUnknown()) || (r2lo.IsDependent() && !r2lo.IsUnknown())) + { + result.lLimit = Limit(Limit::keDependent); + } + // Check hi ranges if they are dependent and not unknown. + if ((r1hi.IsDependent() && !r1hi.IsUnknown()) || (r2hi.IsDependent() && !r2hi.IsUnknown())) + { + result.uLimit = Limit(Limit::keDependent); + } + + if (r1lo.IsConstant()) + { + result.lLimit = MultiplyConstantLimit(r1lo, r2lo); + } + if (r2lo.IsConstant()) + { + result.lLimit = MultiplyConstantLimit(r2lo, r1lo); + } + if (r1hi.IsConstant()) + { + result.uLimit = MultiplyConstantLimit(r1hi, r2hi); + } + if (r2hi.IsConstant()) + { + result.uLimit = MultiplyConstantLimit(r2hi, r1hi); + } + return result; + } + // Given two ranges "r1" and "r2", do a Phi merge. If "monIncreasing" is true, // then ignore the dependent variables for the lower bound but not for the upper bound. static Range Merge(Range& r1, Range& r2, bool monIncreasing) @@ -378,6 +464,32 @@ struct RangeOps } return result; } + + // Given a Range C from an op (x << C), convert it to be used as + // (x * C'), where C' is a power of 2. + static Range ConvertShiftToMultiply(Range& r1) + { + Limit& r1lo = r1.LowerLimit(); + Limit& r1hi = r1.UpperLimit(); + + if (!r1lo.IsConstant() || !r1hi.IsConstant()) + { + return Limit(Limit::keUnknown); + } + + // Keep it simple for now, check if 0 <= C < 31 + int r1loConstant = r1lo.GetConstant(); + int r1hiConstant = r1hi.GetConstant(); + if (r1loConstant <= 0 || r1loConstant > 31 || r1hiConstant <= 0 || r1hiConstant > 31) + { + return Limit(Limit::keUnknown); + } + + Range result = Limit(Limit::keConstant); + result.lLimit = Limit(Limit::keConstant, 1 << r1loConstant); + result.uLimit = Limit(Limit::keConstant, 1 << r1hiConstant); + return result; + } }; class RangeCheck @@ -484,6 +596,9 @@ class RangeCheck // Does the addition of the two limits overflow? bool AddOverflows(Limit& limit1, Limit& limit2); + // Does the multiplication of the two limits overflow? + bool MultiplyOverflows(Limit& limit1, Limit& limit2); + // Does the binary operation between the operands overflow? Check recursively. bool DoesBinOpOverflow(BasicBlock* block, GenTreeOp* binop);