From f5bc205737733f6a2ef4e3f4fb2ebea42169053c Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 2 Dec 2021 11:51:40 +0300 Subject: [PATCH 01/25] tmp --- src/coreclr/jit/assertionprop.cpp | 28 +++++++++++++++++--- src/coreclr/jit/compiler.h | 6 +++++ src/coreclr/jit/rangecheck.cpp | 43 ++++++++++++++++++++++++++++--- src/coreclr/jit/valuenum.cpp | 35 +++++++++++++++++++++++-- src/coreclr/jit/valuenum.h | 3 +++ 5 files changed, 106 insertions(+), 9 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 1ae2176674a7d..2d284facdd939 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -1007,6 +1007,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"); @@ -1086,7 +1091,8 @@ void Compiler::optPrintAssertion(AssertionDsc* curAssertion, AssertionIndex asse assert(!optLocalAssertionProp); vnStore->vnDump(this, curAssertion->op2.vn); } - else if (curAssertion->op1.kind == O1K_CONSTANT_LOOP_BND) + else if ((curAssertion->op1.kind == O1K_CONSTANT_LOOP_BND) || + (curAssertion->op1.kind == O1K_CONSTANT_LOOP_BND_UN)) { assert(!optLocalAssertionProp); vnStore->vnDump(this, curAssertion->op2.vn); @@ -1973,6 +1979,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; @@ -2070,8 +2077,8 @@ 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; @@ -2332,7 +2339,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 258e15c26f305..87d004699a2db 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7540,6 +7540,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, @@ -7615,6 +7616,11 @@ class Compiler return ((assertionKind == OAK_EQUAL || assertionKind == OAK_NOT_EQUAL) && 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() { return ((assertionKind == OAK_NO_THROW) && (op1.kind == O1K_ARR_BND)); diff --git a/src/coreclr/jit/rangecheck.cpp b/src/coreclr/jit/rangecheck.cpp index 4b957a720adc3..c8d4199e88752 100644 --- a/src/coreclr/jit/rangecheck.cpp +++ b/src/coreclr/jit/rangecheck.cpp @@ -273,9 +273,12 @@ void RangeCheck::OptimizeRangeCheck(BasicBlock* block, Statement* stmt, GenTree* // Get the range for this index. Range range = GetRange(block, treeIndex, false DEBUGARG(0)); + Limit lowerLimit = range.LowerLimit(); + Limit upperLimit = range.UpperLimit(); + // If upper or lower limit is found to be unknown (top), or it was found to // be unknown because of over budget or a deep search, then return early. - if (range.UpperLimit().IsUnknown() || range.LowerLimit().IsUnknown()) + if (lowerLimit.IsUnknown() || upperLimit.IsUnknown()) { // Note: If we had stack depth too deep in the GetRange call, we'd be // too deep even in the DoesOverflow call. So return early. @@ -284,8 +287,18 @@ void RangeCheck::OptimizeRangeCheck(BasicBlock* block, Statement* stmt, GenTree* if (DoesOverflow(block, treeIndex)) { - JITDUMP("Method determined to overflow.\n"); - return; + // Actually, we don't overflow if both upper and lower limits are known + // to be constants + if (lowerLimit.IsConstant() && upperLimit.IsConstant() && (lowerLimit.cns <= upperLimit.cns) && + (lowerLimit.cns > 0)) + { + JITDUMP("Upper and lower limits are constants => we don't overflow.\n"); + } + else + { + JITDUMP("Method determined to overflow.\n"); + return; + } } JITDUMP("Range value %s\n", range.ToString(m_pCompiler->getAllocatorDebugOnly())); @@ -602,6 +615,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()) @@ -673,6 +687,25 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse limit = Limit(Limit::keConstant, info.constVal); cmpOper = (genTreeOps)info.cmpOper; } + // Current assertion is of the form (i < 100) != 0 + else if (curAssertion->IsConstantBoundUnsigned()) + { + ValueNumStore::ConstantBoundInfo info; + + //// Get the info as "i", "<" and "100" + m_pCompiler->vnStore->GetConstantBoundInfo(curAssertion->op1.vn, &info); + + // If we don't have the same variable we are comparing against, bail. + if (normalLclVN != info.cmpOpVN) + { + continue; + } + + limit = Limit(Limit::keConstant, info.constVal); + assert(info.cmpOper == VNF_GE_UN); + cmpOper = GT_GE; + isUnsigned = true; + } // Current assertion is of the form i == 100 else if (curAssertion->IsConstantInt32Assertion()) { @@ -827,6 +860,10 @@ 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, 1); + } break; case GT_GT: diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 51927d5256c8e..1ac7ec2194f52 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -4793,9 +4793,31 @@ bool ValueNumStore::IsVNConstantBound(ValueNum vn) return IsVNInt32Constant(funcAttr.m_args[0]) != IsVNInt32Constant(funcAttr.m_args[1]); } +bool ValueNumStore::IsVNConstantBoundUnsigned(ValueNum vn) +{ + // Do we have "var < 100"? + if (vn == NoVN) + { + return false; + } + + VNFuncApp funcAttr; + if (!GetVNFunc(vn, &funcAttr)) + { + return false; + } + if (funcAttr.m_func != VNF_GE_UN) + { + return false; + } + + // check IsVNPositiveInt32Constant ? + return IsVNInt32Constant(funcAttr.m_args[0]) != IsVNInt32Constant(funcAttr.m_args[1]); +} + void ValueNumStore::GetConstantBoundInfo(ValueNum vn, ConstantBoundInfo* info) { - assert(IsVNConstantBound(vn)); + //assert(IsVNConstantBound(vn)); assert(info); // Do we have var < 100? @@ -4812,7 +4834,16 @@ void ValueNumStore::GetConstantBoundInfo(ValueNum vn, ConstantBoundInfo* info) } else { - info->cmpOper = GenTree::SwapRelop((genTreeOps)funcAttr.m_func); + genTreeOps op = (genTreeOps)funcAttr.m_func; + if (funcAttr.m_func == VNF_GT_UN) + { + op = GT_GT; + } + else if (funcAttr.m_func == VNF_GE_UN) + { + op = GT_GE; + } + info->cmpOper = GenTree::SwapRelop(op); info->cmpOpVN = funcAttr.m_args[1]; info->constVal = GetConstantInt32(funcAttr.m_args[0]); } diff --git a/src/coreclr/jit/valuenum.h b/src/coreclr/jit/valuenum.h index 2d7559dd75fc3..6f14bb279ee8f 100644 --- a/src/coreclr/jit/valuenum.h +++ b/src/coreclr/jit/valuenum.h @@ -801,6 +801,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); + // Return true with any Relop except for == and != and one operand has to be a 32-bit integer constant. + bool IsVNConstantBoundUnsigned(ValueNum vn); + // If "vn" is constant bound, then populate the "info" fields for constVal, cmpOp, cmpOper. void GetConstantBoundInfo(ValueNum vn, ConstantBoundInfo* info); From 1556eb8faaa29ab202884535837a67a5b2751d8e Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 16 Dec 2021 01:15:03 +0300 Subject: [PATCH 02/25] Handle bound checks for ((uint)index cmp CNS) pattern --- src/coreclr/jit/assertionprop.cpp | 14 ++------- src/coreclr/jit/morph.cpp | 32 +++++++++++++++++++-- src/coreclr/jit/rangecheck.cpp | 48 ++++++------------------------- src/coreclr/jit/valuenum.cpp | 36 ++++++++++++++--------- src/coreclr/jit/valuenum.h | 6 ++-- 5 files changed, 68 insertions(+), 68 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 2d284facdd939..d0e02cd9a15a6 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -1081,17 +1081,9 @@ 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); diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index f3aceac0dd14c..784e29146ca91 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -12093,9 +12093,8 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) { tree = fgOptimizeRelationalComparisonWithConst(tree->AsOp()); oper = tree->OperGet(); - - assert(op1 == tree->AsOp()->gtGetOp1()); - assert(op2 == tree->AsOp()->gtGetOp2()); + op1 = tree->AsOp()->gtGetOp1(); + op2 = tree->AsOp()->gtGetOp2(); } COMPARE: @@ -13710,6 +13709,33 @@ GenTree* Compiler::fgOptimizeRelationalComparisonWithConst(GenTreeOp* cmp) } } } + else if (fgGlobalMorph && !cmp->IsUnsigned() && (op2Value >= 0) && cmp->OperIs(GT_LE, GT_LT, GT_GE, GT_GE) && + op1->OperIs(GT_CAST) && varTypeIsLong(op1->CastToType()) && op1->gtGetOp1()->TypeIs(TYP_INT) && + op1->IsUnsigned() && ((size_t)op2Value <= UINT_MAX)) + { + // Transform + // + // * GE int + // +--* CAST long <- ulong <- uint + // | \--* X int + // \--* CNS_INT long + // + // to + // + // * GE_un int + // +--* X int + // \--* CNS_INT int + // + // TODO: handle small type casts. Also we can fold the whole condition + // if op2Value doesn't fit into CastToType. + // + cmp->gtOp1 = op1->gtGetOp1(); + cmp->gtFlags |= GTF_UNSIGNED; + assert(op1->TypeIs(TYP_LONG) && op2->TypeIs(TYP_LONG)); + op2->ChangeType(TYP_INT); + DEBUG_DESTROY_NODE(op1); + op1 = cmp->gtGetOp1(); + } if (!cmp->OperIs(oper)) { diff --git a/src/coreclr/jit/rangecheck.cpp b/src/coreclr/jit/rangecheck.cpp index 056b7ad733408..ed7645c1ef3b9 100644 --- a/src/coreclr/jit/rangecheck.cpp +++ b/src/coreclr/jit/rangecheck.cpp @@ -273,12 +273,9 @@ void RangeCheck::OptimizeRangeCheck(BasicBlock* block, Statement* stmt, GenTree* // Get the range for this index. Range range = GetRange(block, treeIndex, false DEBUGARG(0)); - Limit lowerLimit = range.LowerLimit(); - Limit upperLimit = range.UpperLimit(); - // If upper or lower limit is found to be unknown (top), or it was found to // be unknown because of over budget or a deep search, then return early. - if (lowerLimit.IsUnknown() || upperLimit.IsUnknown()) + if (range.LowerLimit().IsUnknown() || range.UpperLimit().IsUnknown()) { // Note: If we had stack depth too deep in the GetRange call, we'd be // too deep even in the DoesOverflow call. So return early. @@ -287,18 +284,8 @@ void RangeCheck::OptimizeRangeCheck(BasicBlock* block, Statement* stmt, GenTree* if (DoesOverflow(block, treeIndex)) { - // Actually, we don't overflow if both upper and lower limits are known - // to be constants - if (lowerLimit.IsConstant() && upperLimit.IsConstant() && (lowerLimit.cns <= upperLimit.cns) && - (lowerLimit.cns > 0)) - { - JITDUMP("Upper and lower limits are constants => we don't overflow.\n"); - } - else - { - JITDUMP("Method determined to overflow.\n"); - return; - } + JITDUMP("Method determined to overflow.\n"); + return; } JITDUMP("Range value %s\n", range.ToString(m_pCompiler->getAllocatorDebugOnly())); @@ -679,7 +666,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; @@ -692,27 +679,9 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse continue; } - limit = Limit(Limit::keConstant, info.constVal); - cmpOper = (genTreeOps)info.cmpOper; - } - // Current assertion is of the form (i < 100) != 0 - else if (curAssertion->IsConstantBoundUnsigned()) - { - ValueNumStore::ConstantBoundInfo info; - - //// Get the info as "i", "<" and "100" - m_pCompiler->vnStore->GetConstantBoundInfo(curAssertion->op1.vn, &info); - - // If we don't have the same variable we are comparing against, bail. - if (normalLclVN != info.cmpOpVN) - { - continue; - } - - limit = Limit(Limit::keConstant, info.constVal); - assert(info.cmpOper == VNF_GE_UN); - cmpOper = GT_GE; - isUnsigned = true; + 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()) @@ -870,13 +839,14 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse pRange->uLimit = limit; if (isUnsigned) { - pRange->lLimit = Limit(Limit::keConstant, 1); + pRange->lLimit = Limit(Limit::keConstant, 0); } break; case GT_GT: case GT_GE: pRange->lLimit = limit; + assert(!isUnsigned); break; case GT_EQ: diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index bb7162d6f8a5e..b8261dc10df26 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -4795,7 +4795,7 @@ bool ValueNumStore::IsVNConstantBound(ValueNum vn) bool ValueNumStore::IsVNConstantBoundUnsigned(ValueNum vn) { - // Do we have "var < 100"? + // Do we have "(unsigned type)var < 100"? which is an equivalent for "var >= 0 && var < 100" if (vn == NoVN) { return false; @@ -4806,7 +4806,7 @@ bool ValueNumStore::IsVNConstantBoundUnsigned(ValueNum vn) { return false; } - if (funcAttr.m_func != VNF_GE_UN) + if ((funcAttr.m_func != VNF_GE_UN) && (funcAttr.m_func != VNF_GT_UN)) { return false; } @@ -4817,7 +4817,8 @@ bool ValueNumStore::IsVNConstantBoundUnsigned(ValueNum vn) void ValueNumStore::GetConstantBoundInfo(ValueNum vn, ConstantBoundInfo* info) { - //assert(IsVNConstantBound(vn)); + bool isUnsignedCnsBnd = IsVNConstantBoundUnsigned(vn); + assert(IsVNConstantBound(vn) || isUnsignedCnsBnd); assert(info); // Do we have var < 100? @@ -4826,27 +4827,36 @@ void ValueNumStore::GetConstantBoundInfo(ValueNum vn, ConstantBoundInfo* info) bool isOp1Const = IsVNInt32Constant(funcAttr.m_args[1]); + genTreeOps op; + if (funcAttr.m_func == VNF_GT_UN) + { + assert(isUnsignedCnsBnd); + op = GT_GT; + } + else if (funcAttr.m_func == VNF_GE_UN) + { + assert(isUnsignedCnsBnd); + op = GT_GE; + } + else + { + assert(!isUnsignedCnsBnd); + op = (genTreeOps)funcAttr.m_func; + } + if (isOp1Const) { - info->cmpOper = funcAttr.m_func; + info->cmpOper = op; info->cmpOpVN = funcAttr.m_args[0]; info->constVal = GetConstantInt32(funcAttr.m_args[1]); } else { - genTreeOps op = (genTreeOps)funcAttr.m_func; - if (funcAttr.m_func == VNF_GT_UN) - { - op = GT_GT; - } - else if (funcAttr.m_func == VNF_GE_UN) - { - op = GT_GE; - } info->cmpOper = GenTree::SwapRelop(op); info->cmpOpVN = funcAttr.m_args[1]; info->constVal = GetConstantInt32(funcAttr.m_args[0]); } + info->isUnsigned = isUnsignedCnsBnd; } //------------------------------------------------------------------------ diff --git a/src/coreclr/jit/valuenum.h b/src/coreclr/jit/valuenum.h index 3d43e5173e274..4392ccc10a8e2 100644 --- a/src/coreclr/jit/valuenum.h +++ b/src/coreclr/jit/valuenum.h @@ -767,8 +767,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) { } @@ -799,7 +800,8 @@ 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); - // Return true with any Relop except for == and != and one operand has to be a 32-bit integer constant. + // Return true with any Relop except for == and != and one operand has to be a 32-bit integer constant + // with (uint) cast for the other operand bool IsVNConstantBoundUnsigned(ValueNum vn); // If "vn" is constant bound, then populate the "info" fields for constVal, cmpOp, cmpOper. From be074e42c1dd99b04c075436dfd0de476b00c356 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 16 Dec 2021 01:29:23 +0300 Subject: [PATCH 03/25] Fix typo --- src/coreclr/jit/morph.cpp | 2 +- src/coreclr/jit/rangecheck.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 784e29146ca91..c5748823f1690 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -13709,7 +13709,7 @@ GenTree* Compiler::fgOptimizeRelationalComparisonWithConst(GenTreeOp* cmp) } } } - else if (fgGlobalMorph && !cmp->IsUnsigned() && (op2Value >= 0) && cmp->OperIs(GT_LE, GT_LT, GT_GE, GT_GE) && + else if (fgGlobalMorph && !cmp->IsUnsigned() && (op2Value >= 0) && cmp->OperIs(GT_LE, GT_LT, GT_GE, GT_GT) && op1->OperIs(GT_CAST) && varTypeIsLong(op1->CastToType()) && op1->gtGetOp1()->TypeIs(TYP_INT) && op1->IsUnsigned() && ((size_t)op2Value <= UINT_MAX)) { diff --git a/src/coreclr/jit/rangecheck.cpp b/src/coreclr/jit/rangecheck.cpp index ed7645c1ef3b9..6804249b7d522 100644 --- a/src/coreclr/jit/rangecheck.cpp +++ b/src/coreclr/jit/rangecheck.cpp @@ -275,7 +275,7 @@ void RangeCheck::OptimizeRangeCheck(BasicBlock* block, Statement* stmt, GenTree* // If upper or lower limit is found to be unknown (top), or it was found to // be unknown because of over budget or a deep search, then return early. - if (range.LowerLimit().IsUnknown() || range.UpperLimit().IsUnknown()) + if (range.UpperLimit().IsUnknown() || range.LowerLimit().IsUnknown()) { // Note: If we had stack depth too deep in the GetRange call, we'd be // too deep even in the DoesOverflow call. So return early. From 1b1474672dc4f24c50218b6cb384481c1694709c Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 16 Dec 2021 02:43:25 +0300 Subject: [PATCH 04/25] Fix build error --- src/coreclr/jit/assertionprop.cpp | 3 +- src/coreclr/jit/morph.cpp | 97 ++++++++++++++++++++++--------- src/coreclr/jit/rangecheck.cpp | 1 - src/coreclr/jit/valuenum.cpp | 14 ++++- 4 files changed, 82 insertions(+), 33 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index d0e02cd9a15a6..6d4b2b27bff25 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -2070,7 +2070,8 @@ 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) || (candidateAssertion.op1.kind == O1K_CONSTANT_LOOP_BND_UN)) + (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; diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index c5748823f1690..5339572ba55ec 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -12088,13 +12088,79 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) case GT_GE: case GT_GT: + if (opts.OptimizationEnabled() && fgGlobalMorph && !tree->IsUnsigned() && + tree->OperIs(GT_LE, GT_LT, GT_GE, GT_GT) && op1->OperIs(GT_CAST) && varTypeIsLong(op1->CastToType()) && + op1->gtGetOp1()->TypeIs(TYP_INT) && op1->IsUnsigned()) + { + bool op2IsCast = false; + bool op2IsNotNegativeU32 = false; + if (op2->IsIntegralConst() && ((size_t)op2->AsIntCon()->IntegralValue() <= UINT_MAX)) + { + op2IsNotNegativeU32 = true; + } + else if (op2->OperIs(GT_CAST) && varTypeIsLong(op2->CastToType()) && + op2->gtGetOp1()->OperIs(GT_ARR_LENGTH)) + { + op2IsNotNegativeU32 = true; + op2IsCast = true; + // TODO: we need to recognize Span._length here as well + } + + // Transform + // + // * GE int + // +--* CAST long <- ulong <- uint + // | \--* X int + // \--* CNS_INT long + // + // to + // + // * GE_un int + // +--* X int + // \--* CNS_INT int + // + // TODO: handle small type casts. Also we can fold the whole condition + // if op2Value doesn't fit into CastToType. + // + // Same for: + // + // * GE int + // +--* CAST long <- ulong <- uint + // | \--* X int + // \--* CAST long <- [u]long <- int + // \--* ARR_LEN int + // + + if (op2IsNotNegativeU32) + { + assert(op1->TypeIs(TYP_LONG) && op2->TypeIs(TYP_LONG)); + + tree->AsOp()->gtOp1 = op1->gtGetOp1(); + tree->gtFlags |= GTF_UNSIGNED; + if (op2IsCast) + { + tree->AsOp()->gtOp2 = op2->gtGetOp1(); + DEBUG_DESTROY_NODE(op2); + op2 = tree->gtGetOp2(); + assert(op2->TypeIs(TYP_INT)); + } + else + { + op2->ChangeType(TYP_INT); + } + DEBUG_DESTROY_NODE(op1); + op1 = tree->gtGetOp1(); + } + } + // op2's value may be changed, so it cannot be a CSE candidate. if (op2->IsIntegralConst() && !gtIsActiveCSE_Candidate(op2)) { tree = fgOptimizeRelationalComparisonWithConst(tree->AsOp()); oper = tree->OperGet(); - op1 = tree->AsOp()->gtGetOp1(); - op2 = tree->AsOp()->gtGetOp2(); + + assert(op1 == tree->AsOp()->gtGetOp1()); + assert(op2 == tree->AsOp()->gtGetOp2()); } COMPARE: @@ -13709,33 +13775,6 @@ GenTree* Compiler::fgOptimizeRelationalComparisonWithConst(GenTreeOp* cmp) } } } - else if (fgGlobalMorph && !cmp->IsUnsigned() && (op2Value >= 0) && cmp->OperIs(GT_LE, GT_LT, GT_GE, GT_GT) && - op1->OperIs(GT_CAST) && varTypeIsLong(op1->CastToType()) && op1->gtGetOp1()->TypeIs(TYP_INT) && - op1->IsUnsigned() && ((size_t)op2Value <= UINT_MAX)) - { - // Transform - // - // * GE int - // +--* CAST long <- ulong <- uint - // | \--* X int - // \--* CNS_INT long - // - // to - // - // * GE_un int - // +--* X int - // \--* CNS_INT int - // - // TODO: handle small type casts. Also we can fold the whole condition - // if op2Value doesn't fit into CastToType. - // - cmp->gtOp1 = op1->gtGetOp1(); - cmp->gtFlags |= GTF_UNSIGNED; - assert(op1->TypeIs(TYP_LONG) && op2->TypeIs(TYP_LONG)); - op2->ChangeType(TYP_INT); - DEBUG_DESTROY_NODE(op1); - op1 = cmp->gtGetOp1(); - } if (!cmp->OperIs(oper)) { diff --git a/src/coreclr/jit/rangecheck.cpp b/src/coreclr/jit/rangecheck.cpp index 6804249b7d522..b441a32d47884 100644 --- a/src/coreclr/jit/rangecheck.cpp +++ b/src/coreclr/jit/rangecheck.cpp @@ -846,7 +846,6 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse case GT_GT: case GT_GE: pRange->lLimit = limit; - assert(!isUnsigned); break; case GT_EQ: diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index b8261dc10df26..2edfb89ff0917 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -4806,7 +4806,8 @@ bool ValueNumStore::IsVNConstantBoundUnsigned(ValueNum vn) { return false; } - if ((funcAttr.m_func != VNF_GE_UN) && (funcAttr.m_func != VNF_GT_UN)) + if ((funcAttr.m_func != VNF_GE_UN) && (funcAttr.m_func != VNF_GT_UN) && (funcAttr.m_func != VNF_LE_UN) && + (funcAttr.m_func != VNF_LT_UN)) { return false; } @@ -4821,7 +4822,6 @@ void ValueNumStore::GetConstantBoundInfo(ValueNum vn, ConstantBoundInfo* info) assert(IsVNConstantBound(vn) || isUnsignedCnsBnd); assert(info); - // Do we have var < 100? VNFuncApp funcAttr; GetVNFunc(vn, &funcAttr); @@ -4838,6 +4838,16 @@ void ValueNumStore::GetConstantBoundInfo(ValueNum vn, ConstantBoundInfo* info) assert(isUnsignedCnsBnd); op = GT_GE; } + else if (funcAttr.m_func == VNF_LT_UN) + { + assert(isUnsignedCnsBnd); + op = GT_LT; + } + else if (funcAttr.m_func == VNF_LE_UN) + { + assert(isUnsignedCnsBnd); + op = GT_LE; + } else { assert(!isUnsignedCnsBnd); From 31ab6d5c944be51218c5def9f197582244860053 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 16 Dec 2021 03:33:08 +0300 Subject: [PATCH 05/25] Fix 32bit platforms --- src/coreclr/jit/morph.cpp | 39 ++++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 5339572ba55ec..68033a2ed75e4 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -12088,24 +12088,11 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) case GT_GE: case GT_GT: - if (opts.OptimizationEnabled() && fgGlobalMorph && !tree->IsUnsigned() && - tree->OperIs(GT_LE, GT_LT, GT_GE, GT_GT) && op1->OperIs(GT_CAST) && varTypeIsLong(op1->CastToType()) && - op1->gtGetOp1()->TypeIs(TYP_INT) && op1->IsUnsigned()) + if (opts.OptimizationEnabled() && !gtIsActiveCSE_Candidate(tree) && !gtIsActiveCSE_Candidate(op1) && + !gtIsActiveCSE_Candidate(op2) && !tree->IsUnsigned() && tree->OperIs(GT_LE, GT_LT, GT_GE, GT_GT) && + op1->OperIs(GT_CAST) && varTypeIsLong(op1->CastToType()) && op1->gtGetOp1()->TypeIs(TYP_INT) && + op1->IsUnsigned()) { - bool op2IsCast = false; - bool op2IsNotNegativeU32 = false; - if (op2->IsIntegralConst() && ((size_t)op2->AsIntCon()->IntegralValue() <= UINT_MAX)) - { - op2IsNotNegativeU32 = true; - } - else if (op2->OperIs(GT_CAST) && varTypeIsLong(op2->CastToType()) && - op2->gtGetOp1()->OperIs(GT_ARR_LENGTH)) - { - op2IsNotNegativeU32 = true; - op2IsCast = true; - // TODO: we need to recognize Span._length here as well - } - // Transform // // * GE int @@ -12119,8 +12106,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) // +--* X int // \--* CNS_INT int // - // TODO: handle small type casts. Also we can fold the whole condition - // if op2Value doesn't fit into CastToType. + // TODO: handle small type casts // // Same for: // @@ -12130,6 +12116,21 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) // \--* CAST long <- [u]long <- int // \--* ARR_LEN int // + bool op2IsCast = false; + bool op2IsNotNegativeU32 = false; + if (op2->IsIntegralConst() && ((size_t)op2->AsIntConCommon()->IntegralValue() <= UINT_MAX)) + { + // We can fold the whole condition if op2 doesn't fit into UINT_MAX. + op2IsNotNegativeU32 = true; + } + else if (op2->OperIs(GT_CAST) && varTypeIsLong(op2->CastToType()) && + op2->gtGetOp1()->OperIs(GT_ARR_LENGTH)) + { + // ARR_LEN is known to be in [0..UINT_MAX] range. + op2IsNotNegativeU32 = true; + op2IsCast = true; + // TODO: we need to recognize Span._length here as well + } if (op2IsNotNegativeU32) { From 1b0619666d145c3318334811a01fdd8413e47f8d Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 16 Dec 2021 04:45:48 +0300 Subject: [PATCH 06/25] Fix 32bit again --- src/coreclr/jit/morph.cpp | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 68033a2ed75e4..3e65d5af0d96c 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -12116,29 +12116,27 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) // \--* CAST long <- [u]long <- int // \--* ARR_LEN int // - bool op2IsCast = false; - bool op2IsNotNegativeU32 = false; - if (op2->IsIntegralConst() && ((size_t)op2->AsIntConCommon()->IntegralValue() <= UINT_MAX)) + bool op2FitsIntoU32 = false; + if (op2->IsIntegralConst() && ((UINT64)op2->AsIntConCommon()->IntegralValue() <= UINT_MAX)) { // We can fold the whole condition if op2 doesn't fit into UINT_MAX. - op2IsNotNegativeU32 = true; + op2FitsIntoU32 = true; } else if (op2->OperIs(GT_CAST) && varTypeIsLong(op2->CastToType()) && op2->gtGetOp1()->OperIs(GT_ARR_LENGTH)) { // ARR_LEN is known to be in [0..UINT_MAX] range. - op2IsNotNegativeU32 = true; - op2IsCast = true; + op2FitsIntoU32 = true; // TODO: we need to recognize Span._length here as well } - if (op2IsNotNegativeU32) + if (op2FitsIntoU32) { assert(op1->TypeIs(TYP_LONG) && op2->TypeIs(TYP_LONG)); tree->AsOp()->gtOp1 = op1->gtGetOp1(); tree->gtFlags |= GTF_UNSIGNED; - if (op2IsCast) + if (op2->OperIs(GT_CAST)) { tree->AsOp()->gtOp2 = op2->gtGetOp1(); DEBUG_DESTROY_NODE(op2); @@ -12148,6 +12146,10 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) else { op2->ChangeType(TYP_INT); +#ifndef TARGET_64BIT + assert(op2->TypeIs(GT_CNS_LNG)); + op2->ChangeOper(GT_CNS_INT, GenTree::PRESERVE_VN); +#endif } DEBUG_DESTROY_NODE(op1); op1 = tree->gtGetOp1(); From 81b52322475ca3d038e628cd81803589798db9d5 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Thu, 16 Dec 2021 05:06:22 +0300 Subject: [PATCH 07/25] Update morph.cpp --- src/coreclr/jit/morph.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 3e65d5af0d96c..ffe64f8b1b901 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -12147,7 +12147,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) { op2->ChangeType(TYP_INT); #ifndef TARGET_64BIT - assert(op2->TypeIs(GT_CNS_LNG)); + assert(op2->OperIs(GT_CNS_LNG)); op2->ChangeOper(GT_CNS_INT, GenTree::PRESERVE_VN); #endif } From b13b852af3fecda725f2542c2aafef5b2c2a03f7 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Thu, 16 Dec 2021 05:51:53 +0300 Subject: [PATCH 08/25] Update morph.cpp --- src/coreclr/jit/morph.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index ffe64f8b1b901..bdf7769c63b59 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -12148,7 +12148,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) op2->ChangeType(TYP_INT); #ifndef TARGET_64BIT assert(op2->OperIs(GT_CNS_LNG)); - op2->ChangeOper(GT_CNS_INT, GenTree::PRESERVE_VN); + op2->ChangeOperUnchecked(GT_CNS_INT); #endif } DEBUG_DESTROY_NODE(op1); From fc5ceb23fa3141af3670aa4d77891a8fd7799e11 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 17 Dec 2021 16:46:00 +0300 Subject: [PATCH 09/25] Clean up --- src/coreclr/jit/assertionprop.cpp | 2 +- src/coreclr/jit/compiler.h | 7 +--- src/coreclr/jit/rangecheck.cpp | 2 +- src/coreclr/jit/valuenum.cpp | 59 +++++++++++++++---------------- 4 files changed, 31 insertions(+), 39 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 6d4b2b27bff25..0dad6512a0cd6 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -2332,7 +2332,7 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) optCreateComplementaryAssertion(index, nullptr, nullptr); return index; } - else if (vnStore->IsVNConstantBoundUnsigned(relopVN)) + else if (!hasTestAgainstZero && vnStore->IsVNConstantBoundUnsigned(relopVN)) { AssertionDsc dsc; dsc.assertionKind = OAK_NOT_EQUAL; diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index c2d2d5cbc728e..be3b74e286110 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7684,12 +7684,7 @@ class Compiler bool IsConstantBound() { return ((assertionKind == OAK_EQUAL || assertionKind == OAK_NOT_EQUAL) && - op1.kind == O1K_CONSTANT_LOOP_BND); - } - bool IsConstantBoundUnsigned() - { - return ((assertionKind == OAK_EQUAL || assertionKind == OAK_NOT_EQUAL) && - op1.kind == O1K_CONSTANT_LOOP_BND_UN); + (op1.kind == O1K_CONSTANT_LOOP_BND) || (op1.kind == O1K_CONSTANT_LOOP_BND_UN)); } bool IsBoundsCheckNoThrow() { diff --git a/src/coreclr/jit/rangecheck.cpp b/src/coreclr/jit/rangecheck.cpp index b441a32d47884..a902082f3d133 100644 --- a/src/coreclr/jit/rangecheck.cpp +++ b/src/coreclr/jit/rangecheck.cpp @@ -666,7 +666,7 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse } } // Current assertion is of the form (i < 100) != 0 - else if (curAssertion->IsConstantBound() || curAssertion->IsConstantBoundUnsigned()) + else if (curAssertion->IsConstantBound()) { ValueNumStore::ConstantBoundInfo info; diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 2edfb89ff0917..98822b18ec1b0 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -4793,33 +4793,10 @@ bool ValueNumStore::IsVNConstantBound(ValueNum vn) return IsVNInt32Constant(funcAttr.m_args[0]) != IsVNInt32Constant(funcAttr.m_args[1]); } -bool ValueNumStore::IsVNConstantBoundUnsigned(ValueNum vn) -{ - // Do we have "(unsigned type)var < 100"? which is an equivalent for "var >= 0 && var < 100" - if (vn == NoVN) - { - return false; - } - - VNFuncApp funcAttr; - if (!GetVNFunc(vn, &funcAttr)) - { - return false; - } - if ((funcAttr.m_func != VNF_GE_UN) && (funcAttr.m_func != VNF_GT_UN) && (funcAttr.m_func != VNF_LE_UN) && - (funcAttr.m_func != VNF_LT_UN)) - { - return false; - } - - // check IsVNPositiveInt32Constant ? - return IsVNInt32Constant(funcAttr.m_args[0]) != IsVNInt32Constant(funcAttr.m_args[1]); -} - void ValueNumStore::GetConstantBoundInfo(ValueNum vn, ConstantBoundInfo* info) { - bool isUnsignedCnsBnd = IsVNConstantBoundUnsigned(vn); - assert(IsVNConstantBound(vn) || isUnsignedCnsBnd); + bool isUnsignedBound = IsVNConstantBoundUnsigned(vn); + assert(IsVNConstantBound(vn) || isUnsignedBound); assert(info); VNFuncApp funcAttr; @@ -4830,27 +4807,22 @@ void ValueNumStore::GetConstantBoundInfo(ValueNum vn, ConstantBoundInfo* info) genTreeOps op; if (funcAttr.m_func == VNF_GT_UN) { - assert(isUnsignedCnsBnd); op = GT_GT; } else if (funcAttr.m_func == VNF_GE_UN) { - assert(isUnsignedCnsBnd); op = GT_GE; } else if (funcAttr.m_func == VNF_LT_UN) { - assert(isUnsignedCnsBnd); op = GT_LT; } else if (funcAttr.m_func == VNF_LE_UN) { - assert(isUnsignedCnsBnd); op = GT_LE; } else { - assert(!isUnsignedCnsBnd); op = (genTreeOps)funcAttr.m_func; } @@ -4866,7 +4838,7 @@ void ValueNumStore::GetConstantBoundInfo(ValueNum vn, ConstantBoundInfo* info) info->cmpOpVN = funcAttr.m_args[1]; info->constVal = GetConstantInt32(funcAttr.m_args[0]); } - info->isUnsigned = isUnsignedCnsBnd; + info->isUnsigned = isUnsignedBound; } //------------------------------------------------------------------------ @@ -4879,6 +4851,31 @@ bool ValueNumStore::IsVNPositiveInt32Constant(ValueNum vn) return IsVNInt32Constant(vn) && (ConstantValue(vn) > 0); } +bool ValueNumStore::IsVNConstantBoundUnsigned(ValueNum vn) +{ + VNFuncApp funcApp; + if ((vn != NoVN) && GetVNFunc(vn, &funcApp)) + { + const bool op1IsConst = IsVNPositiveInt32Constant(funcApp.m_args[0]); + const bool op2IsConst = IsVNPositiveInt32Constant(funcApp.m_args[1]); + + if (!op1IsConst && op2IsConst) + { + // (uint)index < CNS + // (uint)index >= CNS + return (funcApp.m_func == VNF_LT_UN) || (funcApp.m_func == VNF_GE_UN); + } + else if (op1IsConst && !op2IsConst) + { + // CNS > (uint)index + // CNS <= (uint)index + assert(op1IsConst); + return (funcApp.m_func == VNF_GT_UN) || (funcApp.m_func == VNF_LE_UN); + } + } + return false; +} + //------------------------------------------------------------------------ // IsVNArrLenUnsignedBound: Checks if the specified vn represents an expression // of one of the following forms: From 1e14f8ecdb4cbf459f02681af06ab8cf67f92612 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 17 Dec 2021 17:01:57 +0300 Subject: [PATCH 10/25] Clean up --- src/coreclr/jit/assertionprop.cpp | 26 +--------- src/coreclr/jit/compiler.h | 3 +- src/coreclr/jit/valuenum.cpp | 83 +++++++++++++------------------ src/coreclr/jit/valuenum.h | 4 -- 4 files changed, 38 insertions(+), 78 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 0dad6512a0cd6..9650850d333eb 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -1007,11 +1007,6 @@ 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"); @@ -1083,8 +1078,7 @@ void Compiler::optPrintAssertion(AssertionDsc* curAssertion, AssertionIndex asse } 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)) + (curAssertion->op1.kind == O1K_CONSTANT_LOOP_BND)) { assert(!optLocalAssertionProp); vnStore->vnDump(this, curAssertion->op2.vn); @@ -1971,7 +1965,6 @@ 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; @@ -2070,8 +2063,7 @@ 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) || - (candidateAssertion.op1.kind == O1K_CONSTANT_LOOP_BND_UN)) + (candidateAssertion.op1.kind == O1K_CONSTANT_LOOP_BND)) { AssertionDsc dsc = candidateAssertion; dsc.assertionKind = dsc.assertionKind == OAK_EQUAL ? OAK_NOT_EQUAL : OAK_EQUAL; @@ -2332,20 +2324,6 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) optCreateComplementaryAssertion(index, nullptr, nullptr); return index; } - else if (!hasTestAgainstZero && 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 be3b74e286110..5e10bfccfd1c4 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7610,7 +7610,6 @@ 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, @@ -7684,7 +7683,7 @@ class Compiler bool IsConstantBound() { return ((assertionKind == OAK_EQUAL || assertionKind == OAK_NOT_EQUAL) && - (op1.kind == O1K_CONSTANT_LOOP_BND) || (op1.kind == O1K_CONSTANT_LOOP_BND_UN)); + (op1.kind == O1K_CONSTANT_LOOP_BND)); } bool IsBoundsCheckNoThrow() { diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 98822b18ec1b0..9a8a9c2b3c84f 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -4773,30 +4773,38 @@ 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; + } - 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) - { - 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) { - bool isUnsignedBound = IsVNConstantBoundUnsigned(vn); - assert(IsVNConstantBound(vn) || isUnsignedBound); + assert(IsVNConstantBound(vn)); assert(info); VNFuncApp funcAttr; @@ -4807,23 +4815,28 @@ void ValueNumStore::GetConstantBoundInfo(ValueNum vn, ConstantBoundInfo* info) genTreeOps op; if (funcAttr.m_func == VNF_GT_UN) { - op = GT_GT; + op = GT_GT; + info->isUnsigned = true; } else if (funcAttr.m_func == VNF_GE_UN) { - op = GT_GE; + op = GT_GE; + info->isUnsigned = true; } else if (funcAttr.m_func == VNF_LT_UN) { - op = GT_LT; + op = GT_LT; + info->isUnsigned = true; } else if (funcAttr.m_func == VNF_LE_UN) { - op = GT_LE; + op = GT_LE; + info->isUnsigned = true; } else { - op = (genTreeOps)funcAttr.m_func; + op = (genTreeOps)funcAttr.m_func; + info->isUnsigned = false; } if (isOp1Const) @@ -4838,7 +4851,6 @@ void ValueNumStore::GetConstantBoundInfo(ValueNum vn, ConstantBoundInfo* info) info->cmpOpVN = funcAttr.m_args[1]; info->constVal = GetConstantInt32(funcAttr.m_args[0]); } - info->isUnsigned = isUnsignedBound; } //------------------------------------------------------------------------ @@ -4851,31 +4863,6 @@ bool ValueNumStore::IsVNPositiveInt32Constant(ValueNum vn) return IsVNInt32Constant(vn) && (ConstantValue(vn) > 0); } -bool ValueNumStore::IsVNConstantBoundUnsigned(ValueNum vn) -{ - VNFuncApp funcApp; - if ((vn != NoVN) && GetVNFunc(vn, &funcApp)) - { - const bool op1IsConst = IsVNPositiveInt32Constant(funcApp.m_args[0]); - const bool op2IsConst = IsVNPositiveInt32Constant(funcApp.m_args[1]); - - if (!op1IsConst && op2IsConst) - { - // (uint)index < CNS - // (uint)index >= CNS - return (funcApp.m_func == VNF_LT_UN) || (funcApp.m_func == VNF_GE_UN); - } - else if (op1IsConst && !op2IsConst) - { - // CNS > (uint)index - // CNS <= (uint)index - assert(op1IsConst); - return (funcApp.m_func == VNF_GT_UN) || (funcApp.m_func == VNF_LE_UN); - } - } - return false; -} - //------------------------------------------------------------------------ // IsVNArrLenUnsignedBound: Checks if the specified vn represents an expression // of one of the following forms: diff --git a/src/coreclr/jit/valuenum.h b/src/coreclr/jit/valuenum.h index 4392ccc10a8e2..57aeffa05d736 100644 --- a/src/coreclr/jit/valuenum.h +++ b/src/coreclr/jit/valuenum.h @@ -800,10 +800,6 @@ 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); - // Return true with any Relop except for == and != and one operand has to be a 32-bit integer constant - // with (uint) cast for the other operand - bool IsVNConstantBoundUnsigned(ValueNum vn); - // If "vn" is constant bound, then populate the "info" fields for constVal, cmpOp, cmpOper. void GetConstantBoundInfo(ValueNum vn, ConstantBoundInfo* info); From 53a64056af0e0d64882aa5d6955b026c7d37c321 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 17 Dec 2021 21:01:12 +0300 Subject: [PATCH 11/25] Move to fgOptimizeRelationalComparisonWithCasts --- src/coreclr/jit/compiler.h | 1 + src/coreclr/jit/morph.cpp | 208 +++++++++++++++++++++++++------------ 2 files changed, 144 insertions(+), 65 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 5e10bfccfd1c4..ea7a316c25e7b 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -6412,6 +6412,7 @@ class Compiler GenTree* fgOptimizeCast(GenTreeCast* cast); GenTree* fgOptimizeEqualityComparisonWithConst(GenTreeOp* cmp); GenTree* fgOptimizeRelationalComparisonWithConst(GenTreeOp* cmp); + GenTree* fgOptimizeRelationalComparisonWithCasts(GenTreeOp* cmp); GenTree* fgPropagateCommaThrow(GenTree* parent, GenTreeOp* commaThrow, GenTreeFlags precedingSideEffects); GenTree* fgMorphRetInd(GenTreeUnOp* tree); GenTree* fgMorphModToSubMulDiv(GenTreeOp* tree); diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index bdf7769c63b59..ef7020e38cfb4 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -12088,72 +12088,12 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) case GT_GE: case GT_GT: - if (opts.OptimizationEnabled() && !gtIsActiveCSE_Candidate(tree) && !gtIsActiveCSE_Candidate(op1) && - !gtIsActiveCSE_Candidate(op2) && !tree->IsUnsigned() && tree->OperIs(GT_LE, GT_LT, GT_GE, GT_GT) && - op1->OperIs(GT_CAST) && varTypeIsLong(op1->CastToType()) && op1->gtGetOp1()->TypeIs(TYP_INT) && - op1->IsUnsigned()) + if (opts.OptimizationEnabled() && (op1->OperIs(GT_CAST) || op2->OperIs(GT_CAST))) { - // Transform - // - // * GE int - // +--* CAST long <- ulong <- uint - // | \--* X int - // \--* CNS_INT long - // - // to - // - // * GE_un int - // +--* X int - // \--* CNS_INT int - // - // TODO: handle small type casts - // - // Same for: - // - // * GE int - // +--* CAST long <- ulong <- uint - // | \--* X int - // \--* CAST long <- [u]long <- int - // \--* ARR_LEN int - // - bool op2FitsIntoU32 = false; - if (op2->IsIntegralConst() && ((UINT64)op2->AsIntConCommon()->IntegralValue() <= UINT_MAX)) - { - // We can fold the whole condition if op2 doesn't fit into UINT_MAX. - op2FitsIntoU32 = true; - } - else if (op2->OperIs(GT_CAST) && varTypeIsLong(op2->CastToType()) && - op2->gtGetOp1()->OperIs(GT_ARR_LENGTH)) - { - // ARR_LEN is known to be in [0..UINT_MAX] range. - op2FitsIntoU32 = true; - // TODO: we need to recognize Span._length here as well - } - - if (op2FitsIntoU32) - { - assert(op1->TypeIs(TYP_LONG) && op2->TypeIs(TYP_LONG)); - - tree->AsOp()->gtOp1 = op1->gtGetOp1(); - tree->gtFlags |= GTF_UNSIGNED; - if (op2->OperIs(GT_CAST)) - { - tree->AsOp()->gtOp2 = op2->gtGetOp1(); - DEBUG_DESTROY_NODE(op2); - op2 = tree->gtGetOp2(); - assert(op2->TypeIs(TYP_INT)); - } - else - { - op2->ChangeType(TYP_INT); -#ifndef TARGET_64BIT - assert(op2->OperIs(GT_CNS_LNG)); - op2->ChangeOperUnchecked(GT_CNS_INT); -#endif - } - DEBUG_DESTROY_NODE(op1); - op1 = tree->gtGetOp1(); - } + 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. @@ -13791,6 +13731,144 @@ GenTree* Compiler::fgOptimizeRelationalComparisonWithConst(GenTreeOp* cmp) return cmp; } +//------------------------------------------------------------------------ +// fgOptimizeRelationalComparisonWithCasts: optimizes a comparison operation. +// 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 +// +// Arguments: +// tree - the GT_LE/GT_LT/GT_GE/GT_GT tree to morph. +// +// Return Value: +// Returns the same tree where operands might have narrower types +// +GenTree* Compiler::fgOptimizeRelationalComparisonWithCasts(GenTreeOp* tree) +{ + + assert(tree->OperIs(GT_LE, GT_LT, GT_GE, GT_GT)); + + GenTree* castedOp = tree->gtGetOp1(); + GenTree* knownPositiveOp = tree->gtGetOp2(); + + // Caller is expected to call this function only if we have CAST nodes + assert(castedOp->OperIs(GT_CAST) || knownPositiveOp->OperIs(GT_CAST)); + + if (gtIsActiveCSE_Candidate(tree) || gtIsActiveCSE_Candidate(castedOp) || gtIsActiveCSE_Candidate(knownPositiveOp)) + { + // We're going to modify all of them + return tree; + } + + if (!castedOp->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 tree; + } + + bool knownPositiveIsOp2; + if (knownPositiveOp->IsIntegralConst() || + ((knownPositiveOp->OperIs(GT_CAST) && knownPositiveOp->gtGetOp1()->OperIs(GT_ARR_LENGTH)))) + { + // op2 is either a LONG constant or (T)ARR_LENGTH + knownPositiveIsOp2 = true; + } + else + { + // op1 is either a LONG constant (yes, it's pretty normal for relops) + // or (T)ARR_LENGTH + castedOp = tree->gtGetOp2(); + knownPositiveOp = tree->gtGetOp1(); + knownPositiveIsOp2 = false; + } + + if (castedOp->OperIs(GT_CAST) && varTypeIsLong(castedOp->CastToType()) && castedOp->gtGetOp1()->TypeIs(TYP_INT) && + !castedOp->gtGetOp1()->OperIs(GT_ARR_LENGTH) && castedOp->IsUnsigned()) + { + bool knownPositiveFitsIntoU32 = false; + if (knownPositiveOp->IsIntegralConst() && + ((UINT64)knownPositiveOp->AsIntConCommon()->IntegralValue() <= UINT_MAX)) + { + // We can fold the whole condition if op2 doesn't fit into UINT_MAX. + // but let's not bother. + knownPositiveFitsIntoU32 = true; + } + else if (knownPositiveOp->OperIs(GT_CAST) && varTypeIsLong(knownPositiveOp->CastToType()) && + knownPositiveOp->gtGetOp1()->OperIs(GT_ARR_LENGTH)) + { + knownPositiveFitsIntoU32 = true; + // TODO: we need to recognize Span._length here as well + } + + if (!knownPositiveFitsIntoU32) + { + return tree; + } + + JITDUMP("Removing redundant cast(s) for:\n") + DISPTREE(tree) + JITDUMP("\n\nto:\n\n") + + tree->SetUnsigned(); + + // Drop cast from castedOp + if (knownPositiveIsOp2) + { + tree->gtOp1 = castedOp->gtGetOp1(); + } + else + { + tree->gtOp2 = castedOp->gtGetOp1(); + } + DEBUG_DESTROY_NODE(castedOp); + + if (knownPositiveOp->OperIs(GT_CAST)) + { + // Drop cast from knownPositiveOp too + if (knownPositiveIsOp2) + { + tree->gtOp2 = knownPositiveOp->gtGetOp1(); + } + else + { + tree->gtOp1 = knownPositiveOp->gtGetOp1(); + } + DEBUG_DESTROY_NODE(knownPositiveOp); + } + else + { + // Change type for constant from LONG to INT + knownPositiveOp->ChangeType(TYP_INT); +#ifndef TARGET_64BIT + assert(op2->OperIs(GT_CNS_LNG)); + op2->ChangeOperUnchecked(GT_CNS_INT); +#endif + } + DISPTREE(tree) + JITDUMP("\n") + } + return tree; +} + //------------------------------------------------------------------------ // fgPropagateCommaThrow: propagate a "comma throw" up the tree. // From 747339cef814f41decb43c837371fa37e9b06e30 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 17 Dec 2021 21:25:55 +0300 Subject: [PATCH 12/25] fix 32bit again --- src/coreclr/jit/morph.cpp | 10 ++++------ src/coreclr/jit/rangecheck.cpp | 1 + 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index ef7020e38cfb4..1684035efb4f0 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -13763,7 +13763,6 @@ GenTree* Compiler::fgOptimizeRelationalComparisonWithConst(GenTreeOp* cmp) // GenTree* Compiler::fgOptimizeRelationalComparisonWithCasts(GenTreeOp* tree) { - assert(tree->OperIs(GT_LE, GT_LT, GT_GE, GT_GT)); GenTree* castedOp = tree->gtGetOp1(); @@ -13802,14 +13801,13 @@ GenTree* Compiler::fgOptimizeRelationalComparisonWithCasts(GenTreeOp* tree) } if (castedOp->OperIs(GT_CAST) && varTypeIsLong(castedOp->CastToType()) && castedOp->gtGetOp1()->TypeIs(TYP_INT) && - !castedOp->gtGetOp1()->OperIs(GT_ARR_LENGTH) && castedOp->IsUnsigned()) + castedOp->IsUnsigned()) { bool knownPositiveFitsIntoU32 = false; if (knownPositiveOp->IsIntegralConst() && ((UINT64)knownPositiveOp->AsIntConCommon()->IntegralValue() <= UINT_MAX)) { - // We can fold the whole condition if op2 doesn't fit into UINT_MAX. - // but let's not bother. + // 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()) && @@ -13859,8 +13857,8 @@ GenTree* Compiler::fgOptimizeRelationalComparisonWithCasts(GenTreeOp* tree) // Change type for constant from LONG to INT knownPositiveOp->ChangeType(TYP_INT); #ifndef TARGET_64BIT - assert(op2->OperIs(GT_CNS_LNG)); - op2->ChangeOperUnchecked(GT_CNS_INT); + assert(knownPositiveOp->OperIs(GT_CNS_LNG)); + knownPositiveOp->ChangeOperUnchecked(GT_CNS_INT); #endif } DISPTREE(tree) diff --git a/src/coreclr/jit/rangecheck.cpp b/src/coreclr/jit/rangecheck.cpp index a902082f3d133..c46787dc57ae7 100644 --- a/src/coreclr/jit/rangecheck.cpp +++ b/src/coreclr/jit/rangecheck.cpp @@ -846,6 +846,7 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse 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: From f4b1ffac64f00a716b0da281bb9b6c78c71a7442 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 17 Dec 2021 21:32:27 +0300 Subject: [PATCH 13/25] fix SetUnsigned --- 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 3fd3792eb8b1f..dc531eed1572a 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -2097,7 +2097,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; } From 58f4f68a008c17d5a6d152792d3cfcf04ab97688 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 18 Dec 2021 03:30:54 +0300 Subject: [PATCH 14/25] Clean up --- src/coreclr/jit/morph.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 1684035efb4f0..8c5999db88811 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -13771,7 +13771,8 @@ GenTree* Compiler::fgOptimizeRelationalComparisonWithCasts(GenTreeOp* tree) // Caller is expected to call this function only if we have CAST nodes assert(castedOp->OperIs(GT_CAST) || knownPositiveOp->OperIs(GT_CAST)); - if (gtIsActiveCSE_Candidate(tree) || gtIsActiveCSE_Candidate(castedOp) || gtIsActiveCSE_Candidate(knownPositiveOp)) + if (optValnumCSE_phase || gtIsActiveCSE_Candidate(tree) || gtIsActiveCSE_Candidate(castedOp) || + gtIsActiveCSE_Candidate(knownPositiveOp)) { // We're going to modify all of them return tree; @@ -13801,7 +13802,7 @@ GenTree* Compiler::fgOptimizeRelationalComparisonWithCasts(GenTreeOp* tree) } if (castedOp->OperIs(GT_CAST) && varTypeIsLong(castedOp->CastToType()) && castedOp->gtGetOp1()->TypeIs(TYP_INT) && - castedOp->IsUnsigned()) + castedOp->IsUnsigned() && !castedOp->gtOverflow()) { bool knownPositiveFitsIntoU32 = false; if (knownPositiveOp->IsIntegralConst() && @@ -13860,7 +13861,12 @@ GenTree* Compiler::fgOptimizeRelationalComparisonWithCasts(GenTreeOp* tree) assert(knownPositiveOp->OperIs(GT_CNS_LNG)); knownPositiveOp->ChangeOperUnchecked(GT_CNS_INT); #endif + fgUpdateConstTreeValueNumber(knownPositiveOp); } + + tree->gtGetOp1()->SetAllEffectsFlags(tree); + tree->gtGetOp2()->SetAllEffectsFlags(tree); + DISPTREE(tree) JITDUMP("\n") } From b9e68b00ebcc8247bde343b09312b8c6b8fda76c Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Sat, 18 Dec 2021 03:48:49 +0300 Subject: [PATCH 15/25] Update morph.cpp --- src/coreclr/jit/morph.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 8c5999db88811..77915ce97706f 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -13863,10 +13863,6 @@ GenTree* Compiler::fgOptimizeRelationalComparisonWithCasts(GenTreeOp* tree) #endif fgUpdateConstTreeValueNumber(knownPositiveOp); } - - tree->gtGetOp1()->SetAllEffectsFlags(tree); - tree->gtGetOp2()->SetAllEffectsFlags(tree); - DISPTREE(tree) JITDUMP("\n") } From f4e82cee81ae8851353a17baeb34d1a87e850630 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Mon, 20 Dec 2021 00:18:32 +0300 Subject: [PATCH 16/25] test --- src/coreclr/jit/assertionprop.cpp | 6 ++-- src/coreclr/jit/valuenum.cpp | 56 +++++++++++++++---------------- src/coreclr/jit/valuenum.h | 2 +- 3 files changed, 33 insertions(+), 31 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index b1fd90a86b7a6..e86c672875721 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -2195,6 +2195,8 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) bool hasTestAgainstZero = (relop->gtOper == GT_EQ || relop->gtOper == GT_NE) && (op2VN == vnStore->VNZeroForType(op2->TypeGet())); + bool isUnsignedRelop = false; + ValueNumStore::UnsignedCompareCheckedBoundInfo unsignedCompareBnd; // Cases where op1 holds the upper bound arithmetic and op2 is 0. // Loop condition like: "i < bnd +/-k == 0" @@ -2293,7 +2295,7 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) // Cases where op1 holds the condition bound check and op2 is 0. // Loop condition like: "i < 100 == 0" // Assertion: "i < 100 == false" - else if (hasTestAgainstZero && vnStore->IsVNConstantBound(op1VN)) + else if (hasTestAgainstZero && vnStore->IsVNConstantBound(op1VN, &isUnsignedRelop) && !isUnsignedRelop) { AssertionDsc dsc; dsc.assertionKind = relop->gtOper == GT_EQ ? OAK_EQUAL : OAK_NOT_EQUAL; @@ -2310,7 +2312,7 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) // Cases where op1 holds the lhs of the condition op2 holds rhs. // Loop condition like "i < 100" // Assertion: "i < 100 != 0" - else if (vnStore->IsVNConstantBound(relopVN)) + else if (vnStore->IsVNConstantBound(relopVN, &isUnsignedRelop)) { AssertionDsc dsc; dsc.assertionKind = OAK_NOT_EQUAL; diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 322375946ae93..085e985b693ac 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -4771,14 +4771,17 @@ bool ValueNumStore::IsVNRelop(ValueNum vn) } } -bool ValueNumStore::IsVNConstantBound(ValueNum vn) +bool ValueNumStore::IsVNConstantBound(ValueNum vn, bool* viaUnsigned) { + assert(viaUnsigned); + VNFuncApp funcApp; if ((vn != NoVN) && GetVNFunc(vn, &funcApp)) { 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)) { + *viaUnsigned = false; const bool op1IsConst = IsVNInt32Constant(funcApp.m_args[0]); const bool op2IsConst = IsVNInt32Constant(funcApp.m_args[1]); return op1IsConst != op2IsConst; @@ -4790,12 +4793,14 @@ bool ValueNumStore::IsVNConstantBound(ValueNum vn) { // (uint)index < CNS // (uint)index >= CNS + *viaUnsigned = true; return (funcApp.m_func == VNF_LT_UN) || (funcApp.m_func == VNF_GE_UN); } else if (op1IsPositiveConst && !op2IsPositiveConst) { // CNS > (uint)index // CNS <= (uint)index + *viaUnsigned = true; return (funcApp.m_func == VNF_GT_UN) || (funcApp.m_func == VNF_LE_UN); } } @@ -4804,42 +4809,36 @@ bool ValueNumStore::IsVNConstantBound(ValueNum vn) void ValueNumStore::GetConstantBoundInfo(ValueNum vn, ConstantBoundInfo* info) { - assert(IsVNConstantBound(vn)); + bool viaUnsigned = false; + assert(IsVNConstantBound(vn, &viaUnsigned)); assert(info); VNFuncApp funcAttr; GetVNFunc(vn, &funcAttr); - bool isOp1Const = IsVNInt32Constant(funcAttr.m_args[1]); - + bool isUnsigned = true; genTreeOps op; - if (funcAttr.m_func == VNF_GT_UN) + switch (funcAttr.m_func) { - op = GT_GT; - info->isUnsigned = true; - } - else if (funcAttr.m_func == VNF_GE_UN) - { - op = GT_GE; - info->isUnsigned = true; - } - else if (funcAttr.m_func == VNF_LT_UN) - { - op = GT_LT; - info->isUnsigned = true; - } - else if (funcAttr.m_func == VNF_LE_UN) - { - op = GT_LE; - info->isUnsigned = true; - } - else - { - op = (genTreeOps)funcAttr.m_func; - info->isUnsigned = false; + 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 = op; info->cmpOpVN = funcAttr.m_args[0]; @@ -4851,6 +4850,7 @@ void ValueNumStore::GetConstantBoundInfo(ValueNum vn, ConstantBoundInfo* info) 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 5435649e9ee7a..dea307c31d951 100644 --- a/src/coreclr/jit/valuenum.h +++ b/src/coreclr/jit/valuenum.h @@ -798,7 +798,7 @@ class ValueNumStore ValueNum GetArrForLenVn(ValueNum vn); // Return true with any Relop except for == and != and one operand has to be a 32-bit integer constant. - bool IsVNConstantBound(ValueNum vn); + bool IsVNConstantBound(ValueNum vn, bool* viaUnsigned); // If "vn" is constant bound, then populate the "info" fields for constVal, cmpOp, cmpOper. void GetConstantBoundInfo(ValueNum vn, ConstantBoundInfo* info); From abc7d543d5ae872daa705edd07556dd7a7ca869a Mon Sep 17 00:00:00 2001 From: EgorBo Date: Mon, 20 Dec 2021 00:37:25 +0300 Subject: [PATCH 17/25] Introduce O1K_CONSTANT_LOOP_BND_UN --- src/coreclr/jit/assertionprop.cpp | 30 ++++++++++++++++++++++++++---- src/coreclr/jit/compiler.h | 6 ++++++ src/coreclr/jit/rangecheck.cpp | 2 +- src/coreclr/jit/valuenum.cpp | 18 ++++++++++-------- src/coreclr/jit/valuenum.h | 6 +++++- 5 files changed, 48 insertions(+), 14 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index e86c672875721..ba7e7efe10da2 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -1007,6 +1007,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"); @@ -1078,7 +1083,8 @@ void Compiler::optPrintAssertion(AssertionDsc* curAssertion, AssertionIndex asse } 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) || + (curAssertion->op1.kind == O1K_CONSTANT_LOOP_BND_UN)) { assert(!optLocalAssertionProp); vnStore->vnDump(this, curAssertion->op2.vn); @@ -1965,6 +1971,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; @@ -2063,7 +2070,8 @@ 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)) + (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; @@ -2295,7 +2303,7 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) // Cases where op1 holds the condition bound check and op2 is 0. // Loop condition like: "i < 100 == 0" // Assertion: "i < 100 == false" - else if (hasTestAgainstZero && vnStore->IsVNConstantBound(op1VN, &isUnsignedRelop) && !isUnsignedRelop) + else if (hasTestAgainstZero && vnStore->IsVNConstantBound(op1VN)) { AssertionDsc dsc; dsc.assertionKind = relop->gtOper == GT_EQ ? OAK_EQUAL : OAK_NOT_EQUAL; @@ -2312,7 +2320,7 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) // Cases where op1 holds the lhs of the condition op2 holds rhs. // Loop condition like "i < 100" // Assertion: "i < 100 != 0" - else if (vnStore->IsVNConstantBound(relopVN, &isUnsignedRelop)) + else if (vnStore->IsVNConstantBound(relopVN)) { AssertionDsc dsc; dsc.assertionKind = OAK_NOT_EQUAL; @@ -2326,6 +2334,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 ea7a316c25e7b..f014f1d2d5387 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7611,6 +7611,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, @@ -7686,6 +7687,11 @@ class Compiler return ((assertionKind == OAK_EQUAL || assertionKind == OAK_NOT_EQUAL) && (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() { return ((assertionKind == OAK_NO_THROW) && (op1.kind == O1K_ARR_BND)); diff --git a/src/coreclr/jit/rangecheck.cpp b/src/coreclr/jit/rangecheck.cpp index 71875993b5113..6bce75a328586 100644 --- a/src/coreclr/jit/rangecheck.cpp +++ b/src/coreclr/jit/rangecheck.cpp @@ -659,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; diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 085e985b693ac..f417cf86c83db 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -4771,36 +4771,39 @@ bool ValueNumStore::IsVNRelop(ValueNum vn) } } -bool ValueNumStore::IsVNConstantBound(ValueNum vn, bool* viaUnsigned) +bool ValueNumStore::IsVNConstantBound(ValueNum vn) { - assert(viaUnsigned); - VNFuncApp funcApp; if ((vn != NoVN) && GetVNFunc(vn, &funcApp)) { 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)) { - *viaUnsigned = false; const bool op1IsConst = IsVNInt32Constant(funcApp.m_args[0]); const bool op2IsConst = IsVNInt32Constant(funcApp.m_args[1]); return op1IsConst != op2IsConst; } + } + return false; +} +bool ValueNumStore::IsVNConstantBoundUnsigned(ValueNum vn) +{ + VNFuncApp funcApp; + if ((vn != NoVN) && GetVNFunc(vn, &funcApp)) + { 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 - *viaUnsigned = true; return (funcApp.m_func == VNF_LT_UN) || (funcApp.m_func == VNF_GE_UN); } else if (op1IsPositiveConst && !op2IsPositiveConst) { // CNS > (uint)index // CNS <= (uint)index - *viaUnsigned = true; return (funcApp.m_func == VNF_GT_UN) || (funcApp.m_func == VNF_LE_UN); } } @@ -4809,8 +4812,7 @@ bool ValueNumStore::IsVNConstantBound(ValueNum vn, bool* viaUnsigned) void ValueNumStore::GetConstantBoundInfo(ValueNum vn, ConstantBoundInfo* info) { - bool viaUnsigned = false; - assert(IsVNConstantBound(vn, &viaUnsigned)); + assert(IsVNConstantBound(vn) || IsVNConstantBoundUnsigned(vn)); assert(info); VNFuncApp funcAttr; diff --git a/src/coreclr/jit/valuenum.h b/src/coreclr/jit/valuenum.h index dea307c31d951..a0dc4e7d9e360 100644 --- a/src/coreclr/jit/valuenum.h +++ b/src/coreclr/jit/valuenum.h @@ -798,7 +798,11 @@ class ValueNumStore ValueNum GetArrForLenVn(ValueNum vn); // Return true with any Relop except for == and != and one operand has to be a 32-bit integer constant. - bool IsVNConstantBound(ValueNum vn, bool* viaUnsigned); + bool IsVNConstantBound(ValueNum vn); + + // Return true with any Relop except for == and != and one operand has to be a 32-bit integer constant. + // while the other one is under (uint) cast. E.g. (uint)x < 10 + bool IsVNConstantBoundUnsigned(ValueNum vn); // If "vn" is constant bound, then populate the "info" fields for constVal, cmpOp, cmpOper. void GetConstantBoundInfo(ValueNum vn, ConstantBoundInfo* info); From aecf6a43df381112d051f23b4a64a13a26bd68f0 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Mon, 20 Dec 2021 23:02:22 +0300 Subject: [PATCH 18/25] Update assertionprop.cpp --- src/coreclr/jit/assertionprop.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index ba7e7efe10da2..8c3a4a214dbda 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -2203,8 +2203,6 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) bool hasTestAgainstZero = (relop->gtOper == GT_EQ || relop->gtOper == GT_NE) && (op2VN == vnStore->VNZeroForType(op2->TypeGet())); - bool isUnsignedRelop = false; - ValueNumStore::UnsignedCompareCheckedBoundInfo unsignedCompareBnd; // Cases where op1 holds the upper bound arithmetic and op2 is 0. // Loop condition like: "i < bnd +/-k == 0" From ff582124454530d4a20764275bb9eda6b0db2373 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 27 Jan 2022 13:42:22 +0300 Subject: [PATCH 19/25] resolve conflicts --- src/coreclr/jit/morph.cpp | 509 +++++++++++--------------------------- 1 file changed, 139 insertions(+), 370 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 2246dd0ed1328..78aebd41637d0 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -12065,14 +12065,6 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) case GT_GE: case GT_GT: - if (opts.OptimizationEnabled() && (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)) { @@ -13421,144 +13413,6 @@ GenTree* Compiler::fgOptimizeRelationalComparisonWithConst(GenTreeOp* cmp) return cmp; } -//------------------------------------------------------------------------ -// fgOptimizeRelationalComparisonWithCasts: optimizes a comparison operation. -// 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 -// -// Arguments: -// tree - the GT_LE/GT_LT/GT_GE/GT_GT tree to morph. -// -// Return Value: -// Returns the same tree where operands might have narrower types -// -GenTree* Compiler::fgOptimizeRelationalComparisonWithCasts(GenTreeOp* tree) -{ - assert(tree->OperIs(GT_LE, GT_LT, GT_GE, GT_GT)); - - GenTree* castedOp = tree->gtGetOp1(); - GenTree* knownPositiveOp = tree->gtGetOp2(); - - // Caller is expected to call this function only if we have CAST nodes - assert(castedOp->OperIs(GT_CAST) || knownPositiveOp->OperIs(GT_CAST)); - - if (optValnumCSE_phase || gtIsActiveCSE_Candidate(tree) || gtIsActiveCSE_Candidate(castedOp) || - gtIsActiveCSE_Candidate(knownPositiveOp)) - { - // We're going to modify all of them - return tree; - } - - if (!castedOp->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 tree; - } - - bool knownPositiveIsOp2; - if (knownPositiveOp->IsIntegralConst() || - ((knownPositiveOp->OperIs(GT_CAST) && knownPositiveOp->gtGetOp1()->OperIs(GT_ARR_LENGTH)))) - { - // op2 is either a LONG constant or (T)ARR_LENGTH - knownPositiveIsOp2 = true; - } - else - { - // op1 is either a LONG constant (yes, it's pretty normal for relops) - // or (T)ARR_LENGTH - castedOp = tree->gtGetOp2(); - knownPositiveOp = tree->gtGetOp1(); - knownPositiveIsOp2 = false; - } - - if (castedOp->OperIs(GT_CAST) && varTypeIsLong(castedOp->CastToType()) && castedOp->gtGetOp1()->TypeIs(TYP_INT) && - castedOp->IsUnsigned() && !castedOp->gtOverflow()) - { - bool knownPositiveFitsIntoU32 = false; - if (knownPositiveOp->IsIntegralConst() && - ((UINT64)knownPositiveOp->AsIntConCommon()->IntegralValue() <= UINT_MAX)) - { - // 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->gtGetOp1()->OperIs(GT_ARR_LENGTH)) - { - knownPositiveFitsIntoU32 = true; - // TODO: we need to recognize Span._length here as well - } - - if (!knownPositiveFitsIntoU32) - { - return tree; - } - - JITDUMP("Removing redundant cast(s) for:\n") - DISPTREE(tree) - JITDUMP("\n\nto:\n\n") - - tree->SetUnsigned(); - - // Drop cast from castedOp - if (knownPositiveIsOp2) - { - tree->gtOp1 = castedOp->gtGetOp1(); - } - else - { - tree->gtOp2 = castedOp->gtGetOp1(); - } - DEBUG_DESTROY_NODE(castedOp); - - if (knownPositiveOp->OperIs(GT_CAST)) - { - // Drop cast from knownPositiveOp too - if (knownPositiveIsOp2) - { - tree->gtOp2 = knownPositiveOp->gtGetOp1(); - } - else - { - tree->gtOp1 = knownPositiveOp->gtGetOp1(); - } - 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->ChangeOperUnchecked(GT_CNS_INT); -#endif - fgUpdateConstTreeValueNumber(knownPositiveOp); - } - DISPTREE(tree) - JITDUMP("\n") - } - return tree; -} - //------------------------------------------------------------------------ // fgOptimizeCommutativeArithmetic: Optimizes commutative operations. // @@ -13698,228 +13552,6 @@ GenTree* Compiler::fgOptimizeAddition(GenTreeOp* add) // ADD((NEG(a), b) => SUB(b, a) // Do not do this if "op2" is constant for canonicalization purposes. -<<<<<<< .mine - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -======= if (op1->OperIs(GT_NEG) && !op2->OperIs(GT_NEG) && !op2->IsIntegralConst() && gtCanSwapOrder(op1, op2)) { add->SetOper(GT_SUB); @@ -14140,7 +13772,144 @@ GenTree* Compiler::fgOptimizeBitwiseAnd(GenTreeOp* andOp) } //------------------------------------------------------------------------ ->>>>>>> .theirs +// fgOptimizeRelationalComparisonWithCasts: optimizes a comparison operation. +// 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 +// +// Arguments: +// tree - the GT_LE/GT_LT/GT_GE/GT_GT tree to morph. +// +// Return Value: +// Returns the same tree where operands might have narrower types +// +GenTree* Compiler::fgOptimizeRelationalComparisonWithCasts(GenTreeOp* tree) +{ + assert(tree->OperIs(GT_LE, GT_LT, GT_GE, GT_GT)); + + GenTree* castedOp = tree->gtGetOp1(); + GenTree* knownPositiveOp = tree->gtGetOp2(); + + // Caller is expected to call this function only if we have CAST nodes + assert(castedOp->OperIs(GT_CAST) || knownPositiveOp->OperIs(GT_CAST)); + + if (optValnumCSE_phase || gtIsActiveCSE_Candidate(tree) || gtIsActiveCSE_Candidate(castedOp) || + gtIsActiveCSE_Candidate(knownPositiveOp)) + { + // We're going to modify all of them + return tree; + } + + if (!castedOp->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 tree; + } + + bool knownPositiveIsOp2; + if (knownPositiveOp->IsIntegralConst() || + ((knownPositiveOp->OperIs(GT_CAST) && knownPositiveOp->gtGetOp1()->OperIs(GT_ARR_LENGTH)))) + { + // op2 is either a LONG constant or (T)ARR_LENGTH + knownPositiveIsOp2 = true; + } + else + { + // op1 is either a LONG constant (yes, it's pretty normal for relops) + // or (T)ARR_LENGTH + castedOp = tree->gtGetOp2(); + knownPositiveOp = tree->gtGetOp1(); + knownPositiveIsOp2 = false; + } + + if (castedOp->OperIs(GT_CAST) && varTypeIsLong(castedOp->CastToType()) && castedOp->gtGetOp1()->TypeIs(TYP_INT) && + castedOp->IsUnsigned() && !castedOp->gtOverflow()) + { + bool knownPositiveFitsIntoU32 = false; + if (knownPositiveOp->IsIntegralConst() && + ((UINT64)knownPositiveOp->AsIntConCommon()->IntegralValue() <= UINT_MAX)) + { + // 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->gtGetOp1()->OperIs(GT_ARR_LENGTH)) + { + knownPositiveFitsIntoU32 = true; + // TODO: we need to recognize Span._length here as well + } + + if (!knownPositiveFitsIntoU32) + { + return tree; + } + + JITDUMP("Removing redundant cast(s) for:\n") + DISPTREE(tree) + JITDUMP("\n\nto:\n\n") + + tree->SetUnsigned(); + + // Drop cast from castedOp + if (knownPositiveIsOp2) + { + tree->gtOp1 = castedOp->gtGetOp1(); + } + else + { + tree->gtOp2 = castedOp->gtGetOp1(); + } + DEBUG_DESTROY_NODE(castedOp); + + if (knownPositiveOp->OperIs(GT_CAST)) + { + // Drop cast from knownPositiveOp too + if (knownPositiveIsOp2) + { + tree->gtOp2 = knownPositiveOp->gtGetOp1(); + } + else + { + tree->gtOp1 = knownPositiveOp->gtGetOp1(); + } + 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->ChangeOperUnchecked(GT_CNS_INT); +#endif + fgUpdateConstTreeValueNumber(knownPositiveOp); + } + DISPTREE(tree) + JITDUMP("\n") + } + return tree; +} + +//------------------------------------------------------------------------ // fgPropagateCommaThrow: propagate a "comma throw" up the tree. // // "Comma throws" in the compiler represent the canonical form of an always @@ -18568,4 +18337,4 @@ GenTree* Compiler::fgMorphReduceAddOps(GenTree* tree) DEBUG_DESTROY_NODE(tree); return morphed; -} +} \ No newline at end of file From 61b1fa114bf6ccafe5eb92e2a13a2dc117f7d239 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 27 Jan 2022 13:43:06 +0300 Subject: [PATCH 20/25] fix newline --- src/coreclr/jit/morph.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 78aebd41637d0..38667815a4062 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -18337,4 +18337,4 @@ GenTree* Compiler::fgMorphReduceAddOps(GenTree* tree) DEBUG_DESTROY_NODE(tree); return morphed; -} \ No newline at end of file +} From 58cec6401fe59278f41d00f4f6747bb2f51d4dc8 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 18 Feb 2022 20:04:41 +0300 Subject: [PATCH 21/25] Address feedback --- src/coreclr/jit/morph.cpp | 17 ++++++++++++----- src/coreclr/jit/valuenum.h | 3 +-- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 545679d1bea11..d41122b3d3bab 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 (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)) { @@ -13970,9 +13978,8 @@ GenTree* Compiler::fgOptimizeBitwiseAnd(GenTreeOp* andOp) } //------------------------------------------------------------------------ -// fgOptimizeRelationalComparisonWithCasts: optimizes a comparison operation. -// Recognizes comparisons against various cast operands and tries to remove -// them. E.g.: +// fgOptimizeRelationalComparisonWithCasts: Recognizes comparisons against +// various cast operands and tries to remove them. E.g.: // // * GE int // +--* CAST long <- ulong <- uint @@ -14009,8 +14016,7 @@ GenTree* Compiler::fgOptimizeRelationalComparisonWithCasts(GenTreeOp* tree) // Caller is expected to call this function only if we have CAST nodes assert(castedOp->OperIs(GT_CAST) || knownPositiveOp->OperIs(GT_CAST)); - if (optValnumCSE_phase || gtIsActiveCSE_Candidate(tree) || gtIsActiveCSE_Candidate(castedOp) || - gtIsActiveCSE_Candidate(knownPositiveOp)) + if (optValnumCSE_phase) { // We're going to modify all of them return tree; @@ -14054,6 +14060,7 @@ GenTree* Compiler::fgOptimizeRelationalComparisonWithCasts(GenTreeOp* tree) { knownPositiveFitsIntoU32 = true; // TODO: we need to recognize Span._length here as well + // https://github.com/dotnet/runtime/issues/59922#issuecomment-933495507 } if (!knownPositiveFitsIntoU32) diff --git a/src/coreclr/jit/valuenum.h b/src/coreclr/jit/valuenum.h index 2ac34b1f06680..b64f30108baf0 100644 --- a/src/coreclr/jit/valuenum.h +++ b/src/coreclr/jit/valuenum.h @@ -808,8 +808,7 @@ 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); - // Return true with any Relop except for == and != and one operand has to be a 32-bit integer constant. - // while the other one is under (uint) cast. E.g. (uint)x < 10 + // 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. From 10d9a0d2e4711ff1ffc2ceed9d6c27646318ea2b Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Thu, 24 Feb 2022 23:46:13 +0300 Subject: [PATCH 22/25] Apply suggestions from code review Co-authored-by: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com> --- src/coreclr/jit/morph.cpp | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index d41122b3d3bab..7259091e0ff54 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -14031,7 +14031,7 @@ GenTree* Compiler::fgOptimizeRelationalComparisonWithCasts(GenTreeOp* tree) bool knownPositiveIsOp2; if (knownPositiveOp->IsIntegralConst() || - ((knownPositiveOp->OperIs(GT_CAST) && knownPositiveOp->gtGetOp1()->OperIs(GT_ARR_LENGTH)))) + ((knownPositiveOp->OperIs(GT_CAST) && knownPositiveOp->AsCast()->CastOp()->OperIs(GT_ARR_LENGTH)))) { // op2 is either a LONG constant or (T)ARR_LENGTH knownPositiveIsOp2 = true; @@ -14045,7 +14045,7 @@ GenTree* Compiler::fgOptimizeRelationalComparisonWithCasts(GenTreeOp* tree) knownPositiveIsOp2 = false; } - if (castedOp->OperIs(GT_CAST) && varTypeIsLong(castedOp->CastToType()) && castedOp->gtGetOp1()->TypeIs(TYP_INT) && + if (castedOp->OperIs(GT_CAST) && varTypeIsLong(castedOp->CastToType()) && castedOp->AsCast()->CastOp()->TypeIs(TYP_INT) && castedOp->IsUnsigned() && !castedOp->gtOverflow()) { bool knownPositiveFitsIntoU32 = false; @@ -14056,11 +14056,10 @@ GenTree* Compiler::fgOptimizeRelationalComparisonWithCasts(GenTreeOp* tree) knownPositiveFitsIntoU32 = true; } else if (knownPositiveOp->OperIs(GT_CAST) && varTypeIsLong(knownPositiveOp->CastToType()) && - knownPositiveOp->gtGetOp1()->OperIs(GT_ARR_LENGTH)) + knownPositiveOp->AsCast()->CastOp()->OperIs(GT_ARR_LENGTH)) { knownPositiveFitsIntoU32 = true; - // TODO: we need to recognize Span._length here as well - // https://github.com/dotnet/runtime/issues/59922#issuecomment-933495507 + // TODO-Casts: recognize Span.Length here as well. } if (!knownPositiveFitsIntoU32) @@ -14077,11 +14076,11 @@ GenTree* Compiler::fgOptimizeRelationalComparisonWithCasts(GenTreeOp* tree) // Drop cast from castedOp if (knownPositiveIsOp2) { - tree->gtOp1 = castedOp->gtGetOp1(); + tree->gtOp1 = castedOp->AsCast()->CastOp(); } else { - tree->gtOp2 = castedOp->gtGetOp1(); + tree->gtOp2 = castedOp->AsCast()->CastOp(); } DEBUG_DESTROY_NODE(castedOp); @@ -14090,11 +14089,11 @@ GenTree* Compiler::fgOptimizeRelationalComparisonWithCasts(GenTreeOp* tree) // Drop cast from knownPositiveOp too if (knownPositiveIsOp2) { - tree->gtOp2 = knownPositiveOp->gtGetOp1(); + tree->gtOp2 = knownPositiveOp->AsCast()->CastOp(); } else { - tree->gtOp1 = knownPositiveOp->gtGetOp1(); + tree->gtOp1 = knownPositiveOp->AsCast()->CastOp(); } DEBUG_DESTROY_NODE(knownPositiveOp); } From a643eb65bd536c8c9c31105dfb6b0be854ba9705 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 25 Feb 2022 00:07:15 +0300 Subject: [PATCH 23/25] Apply suggestions --- src/coreclr/jit/morph.cpp | 71 ++++++++++++++++++++------------------- 1 file changed, 37 insertions(+), 34 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index b7a2bbd20a72f..e1c11815ab7ac 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -12176,7 +12176,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac) case GT_GE: case GT_GT: - if (op1->OperIs(GT_CAST) || op2->OperIs(GT_CAST)) + if (!optValnumCSE_phase && (op1->OperIs(GT_CAST) || op2->OperIs(GT_CAST))) { tree = fgOptimizeRelationalComparisonWithCasts(tree->AsOp()); oper = tree->OperGet(); @@ -13999,57 +13999,60 @@ GenTree* Compiler::fgOptimizeBitwiseAnd(GenTreeOp* andOp) // \--* CAST long <- [u]long <- int // \--* ARR_LEN int // +// These patterns quite often show up along with index checks +// // Arguments: -// tree - the GT_LE/GT_LT/GT_GE/GT_GT tree to morph. +// 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 // -GenTree* Compiler::fgOptimizeRelationalComparisonWithCasts(GenTreeOp* tree) +// Notes: +// TODO-Casts: consider unifying this function with "optNarrowTree" +// +GenTree* Compiler::fgOptimizeRelationalComparisonWithCasts(GenTreeOp* cmp) { - assert(tree->OperIs(GT_LE, GT_LT, GT_GE, GT_GT)); + assert(cmp->OperIs(GT_LE, GT_LT, GT_GE, GT_GT)); + assert(!optValnumCSE_phase); - GenTree* castedOp = tree->gtGetOp1(); - GenTree* knownPositiveOp = tree->gtGetOp2(); + GenTree* op1 = cmp->gtGetOp1(); + GenTree* op2 = cmp->gtGetOp2(); // Caller is expected to call this function only if we have CAST nodes - assert(castedOp->OperIs(GT_CAST) || knownPositiveOp->OperIs(GT_CAST)); + assert(op1->OperIs(GT_CAST) || op2->OperIs(GT_CAST)); - if (optValnumCSE_phase) - { - // We're going to modify all of them - return tree; - } - - if (!castedOp->TypeIs(TYP_LONG)) + 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 tree; + return cmp; } + GenTree* castOp; + GenTree* knownPositiveOp; + bool knownPositiveIsOp2; - if (knownPositiveOp->IsIntegralConst() || - ((knownPositiveOp->OperIs(GT_CAST) && knownPositiveOp->AsCast()->CastOp()->OperIs(GT_ARR_LENGTH)))) + 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 - castedOp = tree->gtGetOp2(); - knownPositiveOp = tree->gtGetOp1(); + castOp = cmp->gtGetOp2(); + knownPositiveOp = cmp->gtGetOp1(); knownPositiveIsOp2 = false; } - if (castedOp->OperIs(GT_CAST) && varTypeIsLong(castedOp->CastToType()) && castedOp->AsCast()->CastOp()->TypeIs(TYP_INT) && - castedOp->IsUnsigned() && !castedOp->gtOverflow()) + if (castOp->OperIs(GT_CAST) && varTypeIsLong(castOp->CastToType()) && castOp->AsCast()->CastOp()->TypeIs(TYP_INT) && + castOp->IsUnsigned() && !castOp->gtOverflow()) { bool knownPositiveFitsIntoU32 = false; - if (knownPositiveOp->IsIntegralConst() && - ((UINT64)knownPositiveOp->AsIntConCommon()->IntegralValue() <= UINT_MAX)) + if (knownPositiveOp->IsIntegralConst() && FitsIn(knownPositiveOp->AsIntConCommon()->IntegralValue())) { // BTW, we can fold the whole condition if op2 doesn't fit into UINT_MAX. knownPositiveFitsIntoU32 = true; @@ -14063,36 +14066,36 @@ GenTree* Compiler::fgOptimizeRelationalComparisonWithCasts(GenTreeOp* tree) if (!knownPositiveFitsIntoU32) { - return tree; + return cmp; } JITDUMP("Removing redundant cast(s) for:\n") - DISPTREE(tree) + DISPTREE(cmp) JITDUMP("\n\nto:\n\n") - tree->SetUnsigned(); + cmp->SetUnsigned(); - // Drop cast from castedOp + // Drop cast from castOp if (knownPositiveIsOp2) { - tree->gtOp1 = castedOp->AsCast()->CastOp(); + cmp->gtOp1 = castOp->AsCast()->CastOp(); } else { - tree->gtOp2 = castedOp->AsCast()->CastOp(); + cmp->gtOp2 = castOp->AsCast()->CastOp(); } - DEBUG_DESTROY_NODE(castedOp); + DEBUG_DESTROY_NODE(castOp); if (knownPositiveOp->OperIs(GT_CAST)) { // Drop cast from knownPositiveOp too if (knownPositiveIsOp2) { - tree->gtOp2 = knownPositiveOp->AsCast()->CastOp(); + cmp->gtOp2 = knownPositiveOp->AsCast()->CastOp(); } else { - tree->gtOp1 = knownPositiveOp->AsCast()->CastOp(); + cmp->gtOp1 = knownPositiveOp->AsCast()->CastOp(); } DEBUG_DESTROY_NODE(knownPositiveOp); } @@ -14106,10 +14109,10 @@ GenTree* Compiler::fgOptimizeRelationalComparisonWithCasts(GenTreeOp* tree) #endif fgUpdateConstTreeValueNumber(knownPositiveOp); } - DISPTREE(tree) + DISPTREE(cmp) JITDUMP("\n") } - return tree; + return cmp; } //------------------------------------------------------------------------ From ffadcd794ebe9a0ae749d85fef07fc3ce1e5d853 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 25 Feb 2022 02:32:17 +0300 Subject: [PATCH 24/25] use bashtoconst --- src/coreclr/jit/morph.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index e1c11815ab7ac..1f37adfcaace0 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -14105,7 +14105,7 @@ GenTree* Compiler::fgOptimizeRelationalComparisonWithCasts(GenTreeOp* cmp) knownPositiveOp->ChangeType(TYP_INT); #ifndef TARGET_64BIT assert(knownPositiveOp->OperIs(GT_CNS_LNG)); - knownPositiveOp->ChangeOperUnchecked(GT_CNS_INT); + knownPositiveOp->BashToConst(static_cast(knownPositiveOp->AsIntCommon()->IntegralValue())); #endif fgUpdateConstTreeValueNumber(knownPositiveOp); } From d2518c2b44370cf688544ed1aff60e46c7ff9d95 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 25 Feb 2022 12:12:52 +0300 Subject: [PATCH 25/25] use bashtoconst --- src/coreclr/jit/morph.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 1f37adfcaace0..01eae70ebe151 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -14105,7 +14105,7 @@ GenTree* Compiler::fgOptimizeRelationalComparisonWithCasts(GenTreeOp* cmp) knownPositiveOp->ChangeType(TYP_INT); #ifndef TARGET_64BIT assert(knownPositiveOp->OperIs(GT_CNS_LNG)); - knownPositiveOp->BashToConst(static_cast(knownPositiveOp->AsIntCommon()->IntegralValue())); + knownPositiveOp->BashToConst(static_cast(knownPositiveOp->AsIntConCommon()->IntegralValue())); #endif fgUpdateConstTreeValueNumber(knownPositiveOp); }