diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 964d999bc76cbb..7e1f5fd1dafe1a 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -1036,6 +1036,11 @@ void Compiler::optPrintAssertion(AssertionDsc* curAssertion, AssertionIndex asse printf("Const_Loop_Bnd"); vnStore->vnDump(this, curAssertion->op1.vn); } + else if (curAssertion->op1.kind == O1K_CONSTANT_LOOP_BND_UN) + { + printf("Const_Loop_Bnd_Un"); + vnStore->vnDump(this, curAssertion->op1.vn); + } else if (curAssertion->op1.kind == O1K_VALUE_NUMBER) { printf("Value_Number"); @@ -1110,17 +1115,10 @@ void Compiler::optPrintAssertion(AssertionDsc* curAssertion, AssertionIndex asse printf("MT(%08X)", dspPtr(curAssertion->op2.u1.iconVal)); assert(curAssertion->op2.u1.iconFlags != GTF_EMPTY); } - else if (curAssertion->op1.kind == O1K_BOUND_OPER_BND) - { - assert(!optLocalAssertionProp); - vnStore->vnDump(this, curAssertion->op2.vn); - } - else if (curAssertion->op1.kind == O1K_BOUND_LOOP_BND) - { - assert(!optLocalAssertionProp); - vnStore->vnDump(this, curAssertion->op2.vn); - } - else if (curAssertion->op1.kind == O1K_CONSTANT_LOOP_BND) + else if ((curAssertion->op1.kind == O1K_BOUND_OPER_BND) || + (curAssertion->op1.kind == O1K_BOUND_LOOP_BND) || + (curAssertion->op1.kind == O1K_CONSTANT_LOOP_BND) || + (curAssertion->op1.kind == O1K_CONSTANT_LOOP_BND_UN)) { assert(!optLocalAssertionProp); vnStore->vnDump(this, curAssertion->op2.vn); @@ -2011,6 +2009,7 @@ void Compiler::optDebugCheckAssertion(AssertionDsc* assertion) case O1K_BOUND_OPER_BND: case O1K_BOUND_LOOP_BND: case O1K_CONSTANT_LOOP_BND: + case O1K_CONSTANT_LOOP_BND_UN: case O1K_VALUE_NUMBER: assert(!optLocalAssertionProp); break; @@ -2108,8 +2107,9 @@ void Compiler::optCreateComplementaryAssertion(AssertionIndex assertionIndex, } AssertionDsc& candidateAssertion = *optGetAssertion(assertionIndex); - if (candidateAssertion.op1.kind == O1K_BOUND_OPER_BND || candidateAssertion.op1.kind == O1K_BOUND_LOOP_BND || - candidateAssertion.op1.kind == O1K_CONSTANT_LOOP_BND) + if ((candidateAssertion.op1.kind == O1K_BOUND_OPER_BND) || (candidateAssertion.op1.kind == O1K_BOUND_LOOP_BND) || + (candidateAssertion.op1.kind == O1K_CONSTANT_LOOP_BND) || + (candidateAssertion.op1.kind == O1K_CONSTANT_LOOP_BND_UN)) { AssertionDsc dsc = candidateAssertion; dsc.assertionKind = dsc.assertionKind == OAK_EQUAL ? OAK_NOT_EQUAL : OAK_EQUAL; @@ -2370,7 +2370,20 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) optCreateComplementaryAssertion(index, nullptr, nullptr); return index; } - + else if (vnStore->IsVNConstantBoundUnsigned(relopVN)) + { + AssertionDsc dsc; + dsc.assertionKind = OAK_NOT_EQUAL; + dsc.op1.kind = O1K_CONSTANT_LOOP_BND_UN; + dsc.op1.vn = relopVN; + dsc.op2.kind = O2K_CONST_INT; + dsc.op2.vn = vnStore->VNZeroForType(TYP_INT); + dsc.op2.u1.iconVal = 0; + dsc.op2.u1.iconFlags = GTF_EMPTY; + AssertionIndex index = optAddAssertion(&dsc); + optCreateComplementaryAssertion(index, nullptr, nullptr); + return index; + } return NO_ASSERTION_INDEX; } diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 7d63e1e66bac12..72f7256c7fbcbc 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -6429,6 +6429,7 @@ class Compiler GenTree* fgOptimizeHWIntrinsic(GenTreeHWIntrinsic* node); #endif GenTree* fgOptimizeCommutativeArithmetic(GenTreeOp* tree); + GenTree* fgOptimizeRelationalComparisonWithCasts(GenTreeOp* cmp); GenTree* fgOptimizeAddition(GenTreeOp* add); GenTree* fgOptimizeMultiply(GenTreeOp* mul); GenTree* fgOptimizeBitwiseAnd(GenTreeOp* andOp); @@ -7629,6 +7630,7 @@ class Compiler O1K_BOUND_OPER_BND, O1K_BOUND_LOOP_BND, O1K_CONSTANT_LOOP_BND, + O1K_CONSTANT_LOOP_BND_UN, O1K_EXACT_TYPE, O1K_SUBTYPE, O1K_VALUE_NUMBER, @@ -7705,7 +7707,12 @@ class Compiler bool IsConstantBound() { return ((assertionKind == OAK_EQUAL || assertionKind == OAK_NOT_EQUAL) && - op1.kind == O1K_CONSTANT_LOOP_BND); + (op1.kind == O1K_CONSTANT_LOOP_BND)); + } + bool IsConstantBoundUnsigned() + { + return ((assertionKind == OAK_EQUAL || assertionKind == OAK_NOT_EQUAL) && + (op1.kind == O1K_CONSTANT_LOOP_BND_UN)); } bool IsBoundsCheckNoThrow() { diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index ed75194f3a373b..50f355a4176749 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -2045,7 +2045,7 @@ struct GenTree void SetUnsigned() { - assert(OperIs(GT_ADD, GT_SUB, GT_CAST) || OperIsMul()); + assert(OperIs(GT_ADD, GT_SUB, GT_CAST, GT_LE, GT_LT, GT_GT, GT_GE) || OperIsMul()); gtFlags |= GTF_UNSIGNED; } diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index c364925e2ac657..01eae70ebe151e 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -12176,6 +12176,14 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) case GT_GE: case GT_GT: + if (!optValnumCSE_phase && (op1->OperIs(GT_CAST) || op2->OperIs(GT_CAST))) + { + tree = fgOptimizeRelationalComparisonWithCasts(tree->AsOp()); + oper = tree->OperGet(); + op1 = tree->gtGetOp1(); + op2 = tree->gtGetOp2(); + } + // op2's value may be changed, so it cannot be a CSE candidate. if (op2->IsIntegralConst() && !gtIsActiveCSE_Candidate(op2)) { @@ -13968,6 +13976,145 @@ GenTree* Compiler::fgOptimizeBitwiseAnd(GenTreeOp* andOp) return nullptr; } +//------------------------------------------------------------------------ +// fgOptimizeRelationalComparisonWithCasts: Recognizes comparisons against +// various cast operands and tries to remove them. E.g.: +// +// * GE int +// +--* CAST long <- ulong <- uint +// | \--* X int +// \--* CNS_INT long +// +// to: +// +// * GE_un int +// +--* X int +// \--* CNS_INT int +// +// same for: +// +// * GE int +// +--* CAST long <- ulong <- uint +// | \--* X int +// \--* CAST long <- [u]long <- int +// \--* ARR_LEN int +// +// These patterns quite often show up along with index checks +// +// Arguments: +// cmp - the GT_LE/GT_LT/GT_GE/GT_GT tree to morph. +// +// Return Value: +// Returns the same tree where operands might have narrower types +// +// Notes: +// TODO-Casts: consider unifying this function with "optNarrowTree" +// +GenTree* Compiler::fgOptimizeRelationalComparisonWithCasts(GenTreeOp* cmp) +{ + assert(cmp->OperIs(GT_LE, GT_LT, GT_GE, GT_GT)); + assert(!optValnumCSE_phase); + + GenTree* op1 = cmp->gtGetOp1(); + GenTree* op2 = cmp->gtGetOp2(); + + // Caller is expected to call this function only if we have CAST nodes + assert(op1->OperIs(GT_CAST) || op2->OperIs(GT_CAST)); + + if (!op1->TypeIs(TYP_LONG)) + { + // We can extend this logic to handle small types as well, but currently it's done mostly to + // assist range check elimination + return cmp; + } + + GenTree* castOp; + GenTree* knownPositiveOp; + + bool knownPositiveIsOp2; + if (op2->IsIntegralConst() || ((op2->OperIs(GT_CAST) && op2->AsCast()->CastOp()->OperIs(GT_ARR_LENGTH)))) + { + // op2 is either a LONG constant or (T)ARR_LENGTH + knownPositiveIsOp2 = true; + castOp = cmp->gtGetOp1(); + knownPositiveOp = cmp->gtGetOp2(); + } + else + { + // op1 is either a LONG constant (yes, it's pretty normal for relops) + // or (T)ARR_LENGTH + castOp = cmp->gtGetOp2(); + knownPositiveOp = cmp->gtGetOp1(); + knownPositiveIsOp2 = false; + } + + if (castOp->OperIs(GT_CAST) && varTypeIsLong(castOp->CastToType()) && castOp->AsCast()->CastOp()->TypeIs(TYP_INT) && + castOp->IsUnsigned() && !castOp->gtOverflow()) + { + bool knownPositiveFitsIntoU32 = false; + if (knownPositiveOp->IsIntegralConst() && FitsIn(knownPositiveOp->AsIntConCommon()->IntegralValue())) + { + // BTW, we can fold the whole condition if op2 doesn't fit into UINT_MAX. + knownPositiveFitsIntoU32 = true; + } + else if (knownPositiveOp->OperIs(GT_CAST) && varTypeIsLong(knownPositiveOp->CastToType()) && + knownPositiveOp->AsCast()->CastOp()->OperIs(GT_ARR_LENGTH)) + { + knownPositiveFitsIntoU32 = true; + // TODO-Casts: recognize Span.Length here as well. + } + + if (!knownPositiveFitsIntoU32) + { + return cmp; + } + + JITDUMP("Removing redundant cast(s) for:\n") + DISPTREE(cmp) + JITDUMP("\n\nto:\n\n") + + cmp->SetUnsigned(); + + // Drop cast from castOp + if (knownPositiveIsOp2) + { + cmp->gtOp1 = castOp->AsCast()->CastOp(); + } + else + { + cmp->gtOp2 = castOp->AsCast()->CastOp(); + } + DEBUG_DESTROY_NODE(castOp); + + if (knownPositiveOp->OperIs(GT_CAST)) + { + // Drop cast from knownPositiveOp too + if (knownPositiveIsOp2) + { + cmp->gtOp2 = knownPositiveOp->AsCast()->CastOp(); + } + else + { + cmp->gtOp1 = knownPositiveOp->AsCast()->CastOp(); + } + DEBUG_DESTROY_NODE(knownPositiveOp); + } + else + { + // Change type for constant from LONG to INT + knownPositiveOp->ChangeType(TYP_INT); +#ifndef TARGET_64BIT + assert(knownPositiveOp->OperIs(GT_CNS_LNG)); + knownPositiveOp->BashToConst(static_cast(knownPositiveOp->AsIntConCommon()->IntegralValue())); +#endif + fgUpdateConstTreeValueNumber(knownPositiveOp); + } + DISPTREE(cmp) + JITDUMP("\n") + } + return cmp; +} + //------------------------------------------------------------------------ // fgPropagateCommaThrow: propagate a "comma throw" up the tree. // diff --git a/src/coreclr/jit/rangecheck.cpp b/src/coreclr/jit/rangecheck.cpp index 56ad0323fd6de2..c4e7b8adc559f0 100644 --- a/src/coreclr/jit/rangecheck.cpp +++ b/src/coreclr/jit/rangecheck.cpp @@ -603,6 +603,7 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse Limit limit(Limit::keUndef); genTreeOps cmpOper = GT_NONE; bool isConstantAssertion = false; + bool isUnsigned = false; // Current assertion is of the form (i < len - cns) != 0 if (curAssertion->IsCheckedBoundArithBound()) @@ -658,7 +659,7 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse } } // Current assertion is of the form (i < 100) != 0 - else if (curAssertion->IsConstantBound()) + else if (curAssertion->IsConstantBound() || curAssertion->IsConstantBoundUnsigned()) { ValueNumStore::ConstantBoundInfo info; @@ -671,8 +672,9 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse continue; } - limit = Limit(Limit::keConstant, info.constVal); - cmpOper = (genTreeOps)info.cmpOper; + limit = Limit(Limit::keConstant, info.constVal); + cmpOper = (genTreeOps)info.cmpOper; + isUnsigned = info.isUnsigned; } // Current assertion is of the form i == 100 else if (curAssertion->IsConstantInt32Assertion()) @@ -828,11 +830,16 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse case GT_LT: case GT_LE: pRange->uLimit = limit; + if (isUnsigned) + { + pRange->lLimit = Limit(Limit::keConstant, 0); + } break; case GT_GT: case GT_GE: pRange->lLimit = limit; + // it doesn't matter if it's isUnsigned or not here - it's not negative anyway. break; case GT_EQ: diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 3f6828df8dde89..68be59d032db61 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -4936,49 +4936,86 @@ bool ValueNumStore::IsVNRelop(ValueNum vn) bool ValueNumStore::IsVNConstantBound(ValueNum vn) { - // Do we have "var < 100"? - if (vn == NoVN) + VNFuncApp funcApp; + if ((vn != NoVN) && GetVNFunc(vn, &funcApp)) { - return false; + if ((funcApp.m_func == (VNFunc)GT_LE) || (funcApp.m_func == (VNFunc)GT_GE) || + (funcApp.m_func == (VNFunc)GT_LT) || (funcApp.m_func == (VNFunc)GT_GT)) + { + const bool op1IsConst = IsVNInt32Constant(funcApp.m_args[0]); + const bool op2IsConst = IsVNInt32Constant(funcApp.m_args[1]); + return op1IsConst != op2IsConst; + } } + return false; +} - VNFuncApp funcAttr; - if (!GetVNFunc(vn, &funcAttr)) - { - return false; - } - if (funcAttr.m_func != (VNFunc)GT_LE && funcAttr.m_func != (VNFunc)GT_GE && funcAttr.m_func != (VNFunc)GT_LT && - funcAttr.m_func != (VNFunc)GT_GT) +bool ValueNumStore::IsVNConstantBoundUnsigned(ValueNum vn) +{ + VNFuncApp funcApp; + if ((vn != NoVN) && GetVNFunc(vn, &funcApp)) { - return false; + const bool op1IsPositiveConst = IsVNPositiveInt32Constant(funcApp.m_args[0]); + const bool op2IsPositiveConst = IsVNPositiveInt32Constant(funcApp.m_args[1]); + if (!op1IsPositiveConst && op2IsPositiveConst) + { + // (uint)index < CNS + // (uint)index >= CNS + return (funcApp.m_func == VNF_LT_UN) || (funcApp.m_func == VNF_GE_UN); + } + else if (op1IsPositiveConst && !op2IsPositiveConst) + { + // CNS > (uint)index + // CNS <= (uint)index + return (funcApp.m_func == VNF_GT_UN) || (funcApp.m_func == VNF_LE_UN); + } } - - return IsVNInt32Constant(funcAttr.m_args[0]) != IsVNInt32Constant(funcAttr.m_args[1]); + return false; } void ValueNumStore::GetConstantBoundInfo(ValueNum vn, ConstantBoundInfo* info) { - assert(IsVNConstantBound(vn)); + assert(IsVNConstantBound(vn) || IsVNConstantBoundUnsigned(vn)); assert(info); - // Do we have var < 100? VNFuncApp funcAttr; GetVNFunc(vn, &funcAttr); - bool isOp1Const = IsVNInt32Constant(funcAttr.m_args[1]); + bool isUnsigned = true; + genTreeOps op; + switch (funcAttr.m_func) + { + case VNF_GT_UN: + op = GT_GT; + break; + case VNF_GE_UN: + op = GT_GE; + break; + case VNF_LT_UN: + op = GT_LT; + break; + case VNF_LE_UN: + op = GT_LE; + break; + default: + op = (genTreeOps)funcAttr.m_func; + isUnsigned = false; + break; + } - if (isOp1Const) + if (IsVNInt32Constant(funcAttr.m_args[1])) { - info->cmpOper = funcAttr.m_func; + info->cmpOper = op; info->cmpOpVN = funcAttr.m_args[0]; info->constVal = GetConstantInt32(funcAttr.m_args[1]); } else { - info->cmpOper = GenTree::SwapRelop((genTreeOps)funcAttr.m_func); + info->cmpOper = GenTree::SwapRelop(op); info->cmpOpVN = funcAttr.m_args[1]; info->constVal = GetConstantInt32(funcAttr.m_args[0]); } + info->isUnsigned = isUnsigned; } //------------------------------------------------------------------------ diff --git a/src/coreclr/jit/valuenum.h b/src/coreclr/jit/valuenum.h index 235017a78cbec7..f15dd5cc876249 100644 --- a/src/coreclr/jit/valuenum.h +++ b/src/coreclr/jit/valuenum.h @@ -790,8 +790,9 @@ class ValueNumStore int constVal; unsigned cmpOper; ValueNum cmpOpVN; + bool isUnsigned; - ConstantBoundInfo() : constVal(0), cmpOper(GT_NONE), cmpOpVN(NoVN) + ConstantBoundInfo() : constVal(0), cmpOper(GT_NONE), cmpOpVN(NoVN), isUnsigned(false) { } @@ -822,6 +823,9 @@ class ValueNumStore // Return true with any Relop except for == and != and one operand has to be a 32-bit integer constant. bool IsVNConstantBound(ValueNum vn); + // If "vn" is of the form "(uint)var relop cns" for any relop except for == and != + bool IsVNConstantBoundUnsigned(ValueNum vn); + // If "vn" is constant bound, then populate the "info" fields for constVal, cmpOp, cmpOper. void GetConstantBoundInfo(ValueNum vn, ConstantBoundInfo* info);