From bd86ddf1fa91a50bf3d05ebd418dd56dab34d516 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Tue, 25 Feb 2025 02:06:49 +0100 Subject: [PATCH 1/3] Introduce O1K_VN --- src/coreclr/jit/assertionprop.cpp | 81 +++++++++++++++---------------- src/coreclr/jit/compiler.h | 11 ++++- 2 files changed, 48 insertions(+), 44 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index f35612a8943150..d8c82ab873bf2b 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -742,6 +742,10 @@ void Compiler::optPrintAssertion(AssertionDsc* curAssertion, AssertionIndex asse { printf("ArrBnds "); } + else if (curAssertion->op1.kind == O1K_VN) + { + printf("Vn "); + } else if (curAssertion->op1.kind == O1K_SUBTYPE) { printf("Subtype "); @@ -787,6 +791,10 @@ void Compiler::optPrintAssertion(AssertionDsc* curAssertion, AssertionIndex asse vnStore->vnDump(this, curAssertion->op1.bnd.vnLen); printf("]"); } + else if (curAssertion->op1.kind == O1K_VN) + { + printf("[" FMT_VN "]", curAssertion->op1.vn); + } else if (curAssertion->op1.kind == O1K_BOUND_OPER_BND) { printf("Oper_Bnd"); @@ -1534,6 +1542,30 @@ AssertionIndex Compiler::optCreateAssertion(GenTree* op1, } } } + else + { + // Currently, O1K_VN serves as a backup for O1K_LCLVAR (where it's not a local), + // but long term we should keep O1K_LCLVAR for local assertions only. + if (!optLocalAssertionProp) + { + ValueNum op1VN = optConservativeNormalVN(op1); + ValueNum op2VN = optConservativeNormalVN(op2); + + if (op1VN != ValueNumStore::NoVN && op2VN != ValueNumStore::NoVN && vnStore->IsVNInt32Constant(op2VN) && + !vnStore->IsVNHandle(op2VN)) + { + assert(assertionKind == OAK_EQUAL || assertionKind == OAK_NOT_EQUAL); + assertion.assertionKind = assertionKind; + assertion.op1.vn = op1VN; + assertion.op1.kind = O1K_VN; + assertion.op2.vn = op2VN; + assertion.op2.kind = O2K_CONST_INT; + assertion.op2.u1.iconVal = vnStore->ConstantValue(op2VN); + assertion.op2.SetIconFlag(GTF_EMPTY); + return optAddAssertion(&assertion); + } + } + } DONE_ASSERTION: return optFinalizeCreatingAssertion(&assertion); @@ -1830,8 +1862,12 @@ void Compiler::optDebugCheckAssertion(AssertionDsc* assertion) lvaGetDesc(assertion->op1.lcl.lclNum)->lvPerSsaData.IsValidSsaNum(assertion->op1.lcl.ssaNum)); break; case O1K_ARR_BND: - // It would be good to check that bnd.vnIdx and bnd.vnLen are valid value numbers. + assert(assertion->op1.bnd.vnIdx != ValueNumStore::NoVN); + assert(assertion->op1.bnd.vnLen != ValueNumStore::NoVN); + assert(!optLocalAssertionProp); + assert(assertion->assertionKind == OAK_NO_THROW); break; + case O1K_VN: case O1K_BOUND_OPER_BND: case O1K_BOUND_LOOP_BND: case O1K_CONSTANT_LOOP_BND: @@ -2249,45 +2285,7 @@ AssertionInfo Compiler::optAssertionGenJtrue(GenTree* tree) if (vnStore->IsVNCheckedBound(op1VN) && vnStore->IsVNInt32Constant(op2VN)) { assert(relop->OperIs(GT_EQ, GT_NE)); - - int con = vnStore->ConstantValue(op2VN); - if (con >= 0) - { - AssertionDsc dsc; - - // For arr.Length != 0, we know that 0 is a valid index - // For arr.Length == con, we know that con - 1 is the greatest valid index - if (con == 0) - { - dsc.assertionKind = OAK_NOT_EQUAL; - dsc.op1.bnd.vnIdx = vnStore->VNForIntCon(0); - } - else - { - dsc.assertionKind = OAK_EQUAL; - dsc.op1.bnd.vnIdx = vnStore->VNForIntCon(con - 1); - } - - dsc.op1.vn = op1VN; - dsc.op1.kind = O1K_ARR_BND; - dsc.op1.bnd.vnLen = op1VN; - dsc.op2.vn = vnStore->VNConservativeNormalValue(op2->gtVNPair); - dsc.op2.kind = O2K_CONST_INT; - dsc.op2.u1.iconVal = 0; - dsc.op2.SetIconFlag(GTF_EMPTY); - - // when con is not zero, create an assertion on the arr.Length == con edge - // when con is zero, create an assertion on the arr.Length != 0 edge - AssertionIndex index = optAddAssertion(&dsc); - if (relop->OperIs(GT_NE) != (con == 0)) - { - return AssertionInfo::ForNextEdge(index); - } - else - { - return index; - } - } + return optCreateJtrueAssertions(op1, op2, assertionKind); } } @@ -4061,8 +4059,7 @@ void Compiler::optAssertionProp_RangeProperties(ASSERT_VALARG_TP assertions, } // First, analyze possible X ==/!= CNS assertions. - if (curAssertion->IsConstantInt32Assertion() && (curAssertion->op1.kind == O1K_LCLVAR) && - (curAssertion->op1.vn == treeVN)) + if (curAssertion->IsConstantInt32Assertion() && (curAssertion->op1.vn == treeVN)) { if ((curAssertion->assertionKind == OAK_NOT_EQUAL) && (curAssertion->op2.u1.iconVal == 0)) { diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index e7b60869d6fb07..76e7d3496598f8 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7884,6 +7884,7 @@ class Compiler { O1K_INVALID, O1K_LCLVAR, + O1K_VN, O1K_ARR_BND, O1K_BOUND_OPER_BND, O1K_BOUND_LOOP_BND, @@ -8008,7 +8009,8 @@ class Compiler bool IsConstantInt32Assertion() { - return ((assertionKind == OAK_EQUAL) || (assertionKind == OAK_NOT_EQUAL)) && (op2.kind == O2K_CONST_INT); + return ((assertionKind == OAK_EQUAL) || (assertionKind == OAK_NOT_EQUAL)) && (op2.kind == O2K_CONST_INT) && + ((op1.kind == O1K_LCLVAR) || (op1.kind == O1K_VN)); } bool CanPropLclVar() @@ -8028,7 +8030,7 @@ class Compiler bool CanPropBndsCheck() { - return op1.kind == O1K_ARR_BND; + return (op1.kind == O1K_ARR_BND) || (op1.kind == O1K_VN); } bool CanPropSubRange() @@ -8066,6 +8068,11 @@ class Compiler assert(vnBased); return (op1.bnd.vnIdx == that->op1.bnd.vnIdx) && (op1.bnd.vnLen == that->op1.bnd.vnLen); } + else if (op1.kind == O1K_VN) + { + assert(vnBased); + return (op1.vn == that->op1.vn); + } else { return ((vnBased && (op1.vn == that->op1.vn)) || From cb0a7116ffab733a211f373108d4bceda52a076e Mon Sep 17 00:00:00 2001 From: EgorBo Date: Tue, 25 Feb 2025 02:22:06 +0100 Subject: [PATCH 2/3] cleanup --- src/coreclr/jit/assertionprop.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index d8c82ab873bf2b..28d572891c6ffc 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -1862,8 +1862,7 @@ void Compiler::optDebugCheckAssertion(AssertionDsc* assertion) lvaGetDesc(assertion->op1.lcl.lclNum)->lvPerSsaData.IsValidSsaNum(assertion->op1.lcl.ssaNum)); break; case O1K_ARR_BND: - assert(assertion->op1.bnd.vnIdx != ValueNumStore::NoVN); - assert(assertion->op1.bnd.vnLen != ValueNumStore::NoVN); + // It would be good to check that bnd.vnIdx and bnd.vnLen are valid value numbers. assert(!optLocalAssertionProp); assert(assertion->assertionKind == OAK_NO_THROW); break; From 82de83c966edf56cd135863685e042a6a1240174 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Tue, 25 Feb 2025 02:27:12 +0100 Subject: [PATCH 3/3] add a comment --- src/coreclr/jit/assertionprop.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 28d572891c6ffc..91aa15bf1af5ef 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -1551,7 +1551,8 @@ AssertionIndex Compiler::optCreateAssertion(GenTree* op1, ValueNum op1VN = optConservativeNormalVN(op1); ValueNum op2VN = optConservativeNormalVN(op2); - if (op1VN != ValueNumStore::NoVN && op2VN != ValueNumStore::NoVN && vnStore->IsVNInt32Constant(op2VN) && + // For TP reasons, limited to 32-bit constants on the op2 side. + if ((op1VN != ValueNumStore::NoVN) && (op2VN != ValueNumStore::NoVN) && vnStore->IsVNInt32Constant(op2VN) && !vnStore->IsVNHandle(op2VN)) { assert(assertionKind == OAK_EQUAL || assertionKind == OAK_NOT_EQUAL);