From e955452941b6e3e0c804af4c15320ef837746e66 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 9 Feb 2025 06:39:11 +0100 Subject: [PATCH 1/8] clean up --- src/coreclr/jit/assertionprop.cpp | 154 ++++++------------------------ src/coreclr/jit/compiler.h | 4 +- 2 files changed, 31 insertions(+), 127 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index ef91f916fb9563..a6c8d667500179 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -4962,22 +4962,11 @@ GenTree* Compiler::optAssertionProp_Ind(ASSERT_VALARG_TP assertions, GenTree* tr // Arguments: // op - tree to check // assertions - set of live assertions -// pVnBased - [out] set to true if value numbers were used -// pIndex - [out] the assertion used in the proof // // Return Value: // true if the tree's value will be non-null // -// Notes: -// Sets "pVnBased" if the assertion is value number based. If no matching -// assertions are found from the table, then returns "NO_ASSERTION_INDEX." -// -// If both VN and assertion table yield a matching assertion, "pVnBased" -// is only set and the return value is "NO_ASSERTION_INDEX." -// -bool Compiler::optAssertionIsNonNull(GenTree* op, - ASSERT_VALARG_TP assertions DEBUGARG(bool* pVnBased) - DEBUGARG(AssertionIndex* pIndex)) +bool Compiler::optAssertionIsNonNull(GenTree* op, ASSERT_VALARG_TP assertions) { if (op->OperIs(GT_ADD) && op->AsOp()->gtGetOp2()->IsCnsIntOrI() && !fgIsBigOffset(op->AsOp()->gtGetOp2()->AsIntCon()->IconValue())) @@ -4985,88 +4974,42 @@ bool Compiler::optAssertionIsNonNull(GenTree* op, op = op->AsOp()->gtGetOp1(); } - bool vnBased = (!optLocalAssertionProp && vnStore->IsKnownNonNull(op->gtVNPair.GetConservative())); -#ifdef DEBUG - *pIndex = NO_ASSERTION_INDEX; - *pVnBased = vnBased; -#endif - - if (vnBased) + // Fast path when we have a VN + if (!optLocalAssertionProp && vnStore->IsKnownNonNull(vnStore->VNConservativeNormalValue(op->gtVNPair))) { return true; } - op = op->gtEffectiveVal(); - - if (!op->OperIs(GT_LCL_VAR)) + if (!optCanPropNonNull || BitVecOps::MayBeUninit(assertions)) { return false; } - AssertionIndex index = optAssertionIsNonNullInternal(op, assertions DEBUGARG(pVnBased)); -#ifdef DEBUG - *pIndex = index; -#endif - return index != NO_ASSERTION_INDEX; -} - -//------------------------------------------------------------------------ -// optAssertionIsNonNullInternal: see if we can prove a tree's value will -// be non-null based on assertions -// -// Arguments: -// op - tree to check -// assertions - set of live assertions -// pVnBased - [out] set to true if value numbers were used -// -// Return Value: -// index of assertion, or NO_ASSERTION_INDEX -// -AssertionIndex Compiler::optAssertionIsNonNullInternal(GenTree* op, - ASSERT_VALARG_TP assertions DEBUGARG(bool* pVnBased)) -{ - -#ifdef DEBUG - // Initialize the out param - // - *pVnBased = false; -#endif - - if (!optCanPropNonNull) + op = op->gtEffectiveVal(); + if (!op->OperIs(GT_LCL_VAR)) { - return NO_ASSERTION_INDEX; + return false; } // If local assertion prop use lcl comparison, else use VN comparison. if (!optLocalAssertionProp) { - if (BitVecOps::MayBeUninit(assertions) || BitVecOps::IsEmpty(apTraits, assertions)) - { - return NO_ASSERTION_INDEX; - } - // Look at both the top-level vn, and // the vn we get by stripping off any constant adds. // - ValueNum vn = vnStore->VNConservativeNormalValue(op->gtVNPair); - ValueNum vnBase = vn; - VNFuncApp funcAttr; + ValueNum vn = vnStore->VNConservativeNormalValue(op->gtVNPair); + if (vn == ValueNumStore::NoVN) + { + return false; + } - while (vnStore->GetVNFunc(vnBase, &funcAttr) && (funcAttr.m_func == (VNFunc)GT_ADD)) + ValueNum vnBase = vn; + target_ssize_t offset = 0; + vnStore->PeelOffsets(&vnBase, &offset); + if (fgIsBigOffset((size_t)offset)) { - if (vnStore->IsVNConstant(funcAttr.m_args[1]) && varTypeIsIntegral(vnStore->TypeOfVN(funcAttr.m_args[1]))) - { - vnBase = funcAttr.m_args[0]; - } - else if (vnStore->IsVNConstant(funcAttr.m_args[0]) && - varTypeIsIntegral(vnStore->TypeOfVN(funcAttr.m_args[0]))) - { - vnBase = funcAttr.m_args[1]; - } - else - { - break; - } + // Give up on big offsets + vnBase = vn; } // Check each assertion to find if we have a vn != null assertion. @@ -5076,26 +5019,11 @@ AssertionIndex Compiler::optAssertionIsNonNullInternal(GenTree* while (iter.NextElem(&index)) { AssertionIndex assertionIndex = GetAssertionIndex(index); - if (assertionIndex > optAssertionCount) - { - break; - } - AssertionDsc* curAssertion = optGetAssertion(assertionIndex); - if (!curAssertion->CanPropNonNull()) - { - continue; - } - - if ((curAssertion->op1.vn != vn) && (curAssertion->op1.vn != vnBase)) + AssertionDsc* curAssertion = optGetAssertion(assertionIndex); + if (curAssertion->CanPropNonNull() && ((curAssertion->op1.vn == vn) || (curAssertion->op1.vn == vnBase))) { - continue; + return true; } - -#ifdef DEBUG - *pVnBased = true; -#endif - - return assertionIndex; } } else @@ -5119,11 +5047,11 @@ AssertionIndex Compiler::optAssertionIsNonNullInternal(GenTree* (curAssertion->op2.kind == O2K_CONST_INT) && // op2 (curAssertion->op1.lcl.lclNum == lclNum) && (curAssertion->op2.u1.iconVal == 0)) { - return assertionIndex; + return true; } } } - return NO_ASSERTION_INDEX; + return false; } //------------------------------------------------------------------------ @@ -5177,20 +5105,10 @@ GenTree* Compiler::optNonNullAssertionProp_Call(ASSERT_VALARG_TP assertions, Gen GenTree* op1 = call->gtArgs.GetThisArg()->GetNode(); noway_assert(op1 != nullptr); -#ifdef DEBUG - bool vnBased = false; - AssertionIndex index = NO_ASSERTION_INDEX; -#endif - if (optAssertionIsNonNull(op1, assertions DEBUGARG(&vnBased) DEBUGARG(&index))) + if (optAssertionIsNonNull(op1, assertions)) { -#ifdef DEBUG - if (verbose) - { - (vnBased) ? printf("\nVN based non-null prop in " FMT_BB ":\n", compCurBB->bbNum) - : printf("\nNon-null prop for index #%02u in " FMT_BB ":\n", index, compCurBB->bbNum); - gtDispTree(call, nullptr, nullptr, true); - } -#endif + JITDUMP("Non-null assertion prop for tree [%06d] in " FMT_BB ":\n", dspTreeID(op1), compCurBB->bbNum); + call->gtFlags &= ~GTF_CALL_NULLCHECK; call->gtFlags &= ~GTF_EXCEPT; noway_assert(call->gtFlags & GTF_SIDE_EFFECT); @@ -5219,20 +5137,10 @@ bool Compiler::optNonNullAssertionProp_Ind(ASSERT_VALARG_TP assertions, GenTree* return false; } -#ifdef DEBUG - bool vnBased = false; - AssertionIndex index = NO_ASSERTION_INDEX; -#endif - if (optAssertionIsNonNull(indir->AsIndir()->Addr(), assertions DEBUGARG(&vnBased) DEBUGARG(&index))) + if (optAssertionIsNonNull(indir->AsIndir()->Addr(), assertions)) { -#ifdef DEBUG - if (verbose) - { - (vnBased) ? printf("\nVN based non-null prop in " FMT_BB ":\n", compCurBB->bbNum) - : printf("\nNon-null prop for index #%02u in " FMT_BB ":\n", index, compCurBB->bbNum); - gtDispTree(indir, nullptr, nullptr, true); - } -#endif + JITDUMP("Non-null assertion prop for indirection [%06d] in " FMT_BB ":\n", dspTreeID(indir), compCurBB->bbNum); + indir->gtFlags &= ~GTF_EXCEPT; indir->gtFlags |= GTF_IND_NONFAULTING; @@ -5429,11 +5337,9 @@ GenTree* Compiler::optAssertionProp_Call(ASSERT_VALARG_TP assertions, GenTreeCal } // Leave a hint for fgLateCastExpansion that obj is never null. - INDEBUG(AssertionIndex nonNullIdx = NO_ASSERTION_INDEX); - INDEBUG(bool vnBased = false); // GTF_CALL_M_CAST_CAN_BE_EXPANDED check is to improve TP if (((call->gtCallMoreFlags & GTF_CALL_M_CAST_CAN_BE_EXPANDED) != 0) && - optAssertionIsNonNull(objArg, assertions DEBUGARG(&vnBased) DEBUGARG(&nonNullIdx))) + optAssertionIsNonNull(objArg, assertions)) { call->gtCallMoreFlags |= GTF_CALL_M_CAST_OBJ_NONNULL; return optAssertionProp_Update(call, call, stmt); diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index dd967f33cbed9e..2f5d219ed94d43 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -8247,10 +8247,8 @@ class Compiler // Used for respective assertion propagations. AssertionIndex optAssertionIsSubrange(GenTree* tree, IntegralRange range, ASSERT_VALARG_TP assertions); AssertionIndex optAssertionIsSubtype(GenTree* tree, GenTree* methodTableArg, ASSERT_VALARG_TP assertions); - AssertionIndex optAssertionIsNonNullInternal(GenTree* op, ASSERT_VALARG_TP assertions DEBUGARG(bool* pVnBased)); bool optAssertionVNIsNonNull(ValueNum vn, ASSERT_VALARG_TP assertions); - bool optAssertionIsNonNull(GenTree* op, - ASSERT_VALARG_TP assertions DEBUGARG(bool* pVnBased) DEBUGARG(AssertionIndex* pIndex)); + bool optAssertionIsNonNull(GenTree* op, ASSERT_VALARG_TP assertions); AssertionIndex optGlobalAssertionIsEqualOrNotEqual(ASSERT_VALARG_TP assertions, GenTree* op1, GenTree* op2); AssertionIndex optGlobalAssertionIsEqualOrNotEqualZero(ASSERT_VALARG_TP assertions, GenTree* op1); From 4158fe9b7fbad0df48965985524b80fd4500ebb9 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 9 Feb 2025 08:01:22 +0100 Subject: [PATCH 2/8] more clean up --- src/coreclr/jit/valuenum.cpp | 29 ++++++++--------------------- 1 file changed, 8 insertions(+), 21 deletions(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index ecd7f4fbdf475c..75ffb9d7ae5c99 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -12123,28 +12123,15 @@ bool Compiler::GetObjectHandleAndOffset(GenTree* tree, ssize_t* byteOffset, CORI return false; } - ValueNum treeVN = tree->gtVNPair.GetLiberal(); - VNFuncApp funcApp; - target_ssize_t offset = 0; - while (vnStore->GetVNFunc(treeVN, &funcApp) && (funcApp.m_func == (VNFunc)GT_ADD)) + ValueNum treeVN = tree->gtVNPair.GetLiberal(); + if (treeVN == ValueNumStore::NoVN) { - if (vnStore->IsVNConstantNonHandle(funcApp.m_args[0]) && (vnStore->TypeOfVN(funcApp.m_args[0]) == TYP_I_IMPL)) - { - offset += vnStore->ConstantValue(funcApp.m_args[0]); - treeVN = funcApp.m_args[1]; - } - else if (vnStore->IsVNConstantNonHandle(funcApp.m_args[1]) && - (vnStore->TypeOfVN(funcApp.m_args[1]) == TYP_I_IMPL)) - { - offset += vnStore->ConstantValue(funcApp.m_args[1]); - treeVN = funcApp.m_args[0]; - } - else - { - return false; - } + return false; } + target_ssize_t offset = 0; + vnStore->PeelOffsets(&treeVN, &offset); + if (vnStore->IsVNObjHandle(treeVN)) { *pObj = vnStore->ConstantObjHandle(treeVN); @@ -15247,12 +15234,12 @@ void ValueNumStore::PeelOffsets(ValueNum* vn, target_ssize_t* offset) VNFuncApp app; while (GetVNFunc(*vn, &app) && (app.m_func == VNF_ADD)) { - if (IsVNConstantNonHandle(app.m_args[0])) + if (IsVNConstantNonHandle(app.m_args[0]) && (app.m_args[0] != VNForNull())) { *offset += ConstantValue(app.m_args[0]); *vn = app.m_args[1]; } - else if (IsVNConstantNonHandle(app.m_args[1])) + else if (IsVNConstantNonHandle(app.m_args[1]) && (app.m_args[1] != VNForNull())) { *offset += ConstantValue(app.m_args[1]); *vn = app.m_args[0]; From a5d3e3e6b4955294c28279b03ddd6038e02a0542 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 9 Feb 2025 08:16:44 +0100 Subject: [PATCH 3/8] more refactorings --- src/coreclr/jit/valuenum.cpp | 101 ++++++----------------------------- 1 file changed, 15 insertions(+), 86 deletions(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 75ffb9d7ae5c99..61ef4918434efc 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -12256,7 +12256,6 @@ bool Compiler::fgValueNumberConstLoad(GenTreeIndir* tree) size_t index = -1; // First, let see if we have PtrToArrElem - ValueNum addr = funcApp.m_args[0]; if (funcApp.m_func == VNF_PtrToArrElem) { ValueNum arrVN = funcApp.m_args[1]; @@ -12270,34 +12269,9 @@ bool Compiler::fgValueNumberConstLoad(GenTreeIndir* tree) } else if (funcApp.m_func == (VNFunc)GT_ADD) { - ssize_t dataOffset = 0; - ValueNum baseVN = ValueNumStore::NoVN; - - // Loop to accumulate total dataOffset, e.g.: - // ADD(C1, ADD(ObjHandle, C2)) -> C1 + C2 - do - { - ValueNum op1VN = funcApp.m_args[0]; - ValueNum op2VN = funcApp.m_args[1]; - - if (vnStore->IsVNConstant(op1VN) && varTypeIsIntegral(vnStore->TypeOfVN(op1VN)) && - !isCnsObjHandle(vnStore, op1VN, &objHandle)) - { - dataOffset += vnStore->CoercedConstantValue(op1VN); - baseVN = op2VN; - } - else if (vnStore->IsVNConstant(op2VN) && varTypeIsIntegral(vnStore->TypeOfVN(op2VN)) && - !isCnsObjHandle(vnStore, op2VN, &objHandle)) - { - dataOffset += vnStore->CoercedConstantValue(op2VN); - baseVN = op1VN; - } - else - { - // one of the args is expected to be an integer constant - return false; - } - } while (vnStore->GetVNFunc(baseVN, &funcApp) && (funcApp.m_func == (VNFunc)GT_ADD)); + ssize_t dataOffset = 0; + vnStore->PeelOffsets(&addrVN, &dataOffset); + ValueNum baseVN = addrVN; if (isCnsObjHandle(vnStore, baseVN, &objHandle) && (dataOffset >= (ssize_t)OFFSETOF__CORINFO_String__chars) && ((dataOffset % 2) == 0)) @@ -14421,68 +14395,21 @@ void Compiler::fgValueNumberAddExceptionSetForIndirection(GenTree* tree, GenTree ValueNumPair baseVNP = vnStore->VNPNormalPair(baseAddr->gtVNPair); ValueNum baseLVN = baseVNP.GetLiberal(); ValueNum baseCVN = baseVNP.GetConservative(); - ssize_t offsetL = 0; - ssize_t offsetC = 0; - VNFuncApp funcAttr; - while (vnStore->GetVNFunc(baseLVN, &funcAttr) && (funcAttr.m_func == (VNFunc)GT_ADD) && - (vnStore->TypeOfVN(baseLVN) == TYP_BYREF)) + target_ssize_t offsetL; + vnStore->PeelOffsets(&baseLVN, &offsetL); + if (fgIsBigOffset(offsetL)) { - // The arguments in value numbering functions are sorted in increasing order - // Thus either arg could be the constant. - if (vnStore->IsVNConstant(funcAttr.m_args[0]) && varTypeIsIntegral(vnStore->TypeOfVN(funcAttr.m_args[0]))) - { - offsetL += vnStore->CoercedConstantValue(funcAttr.m_args[0]); - baseLVN = funcAttr.m_args[1]; - } - else if (vnStore->IsVNConstant(funcAttr.m_args[1]) && varTypeIsIntegral(vnStore->TypeOfVN(funcAttr.m_args[1]))) - { - offsetL += vnStore->CoercedConstantValue(funcAttr.m_args[1]); - baseLVN = funcAttr.m_args[0]; - } - else // neither argument is a constant - { - break; - } - - if (fgIsBigOffset(offsetL)) - { - // Failure: Exit this loop if we have a "big" offset - - // reset baseLVN back to the full address expression - baseLVN = baseVNP.GetLiberal(); - break; - } + // Reset baseLVN back to the full address expression + baseLVN = baseVNP.GetLiberal(); } - while (vnStore->GetVNFunc(baseCVN, &funcAttr) && (funcAttr.m_func == (VNFunc)GT_ADD) && - (vnStore->TypeOfVN(baseCVN) == TYP_BYREF)) + target_ssize_t offsetC; + vnStore->PeelOffsets(&baseCVN, &offsetC); + if (fgIsBigOffset(offsetC)) { - // The arguments in value numbering functions are sorted in increasing order - // Thus either arg could be the constant. - if (vnStore->IsVNConstant(funcAttr.m_args[0]) && varTypeIsIntegral(vnStore->TypeOfVN(funcAttr.m_args[0]))) - { - offsetL += vnStore->CoercedConstantValue(funcAttr.m_args[0]); - baseCVN = funcAttr.m_args[1]; - } - else if (vnStore->IsVNConstant(funcAttr.m_args[1]) && varTypeIsIntegral(vnStore->TypeOfVN(funcAttr.m_args[1]))) - { - offsetC += vnStore->CoercedConstantValue(funcAttr.m_args[1]); - baseCVN = funcAttr.m_args[0]; - } - else // neither argument is a constant - { - break; - } - - if (fgIsBigOffset(offsetC)) - { - // Failure: Exit this loop if we have a "big" offset - - // reset baseCVN back to the full address expression - baseCVN = baseVNP.GetConservative(); - break; - } + // Reset baseCVN back to the full address expression + baseCVN = baseVNP.GetConservative(); } // The exceptions in "baseVNP" should have been added to the "tree"'s set already. @@ -15234,6 +15161,8 @@ void ValueNumStore::PeelOffsets(ValueNum* vn, target_ssize_t* offset) VNFuncApp app; while (GetVNFunc(*vn, &app) && (app.m_func == VNF_ADD)) { + // We don't treat handles and null as constant offset. + if (IsVNConstantNonHandle(app.m_args[0]) && (app.m_args[0] != VNForNull())) { *offset += ConstantValue(app.m_args[0]); From 4e850b13a8e49abd15f6ec51f687b535d396c9ac Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 9 Feb 2025 08:19:04 +0100 Subject: [PATCH 4/8] fix compilation --- src/coreclr/jit/valuenum.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 61ef4918434efc..1fd56e153f1d64 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -12269,7 +12269,7 @@ bool Compiler::fgValueNumberConstLoad(GenTreeIndir* tree) } else if (funcApp.m_func == (VNFunc)GT_ADD) { - ssize_t dataOffset = 0; + target_ssize_t dataOffset = 0; vnStore->PeelOffsets(&addrVN, &dataOffset); ValueNum baseVN = addrVN; From ea2425873c55adf8775b6922be10c45ccf7f2d63 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 9 Feb 2025 09:04:34 +0100 Subject: [PATCH 5/8] fix tp regressions --- src/coreclr/jit/assertionprop.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index a6c8d667500179..ac111bef9abf14 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -4975,7 +4975,7 @@ bool Compiler::optAssertionIsNonNull(GenTree* op, ASSERT_VALARG_TP assertions) } // Fast path when we have a VN - if (!optLocalAssertionProp && vnStore->IsKnownNonNull(vnStore->VNConservativeNormalValue(op->gtVNPair))) + if (!optLocalAssertionProp && vnStore->IsKnownNonNull(op->gtVNPair.GetConservative())) { return true; } From e9187b44ffd274219d7bc1ecafab713252b328f8 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 9 Feb 2025 15:55:22 +0100 Subject: [PATCH 6/8] ci test --- src/coreclr/jit/valuenum.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 1fd56e153f1d64..71313b00e92d0d 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -14396,6 +14396,8 @@ void Compiler::fgValueNumberAddExceptionSetForIndirection(GenTree* tree, GenTree ValueNum baseLVN = baseVNP.GetLiberal(); ValueNum baseCVN = baseVNP.GetConservative(); + assert(baseVNP.BothDefined()); + target_ssize_t offsetL; vnStore->PeelOffsets(&baseLVN, &offsetL); if (fgIsBigOffset(offsetL)) @@ -15154,7 +15156,11 @@ void ValueNumStore::PeelOffsets(ValueNum* vn, target_ssize_t* offset) { #ifdef DEBUG var_types vnType = TypeOfVN(*vn); - assert((vnType == TYP_I_IMPL) || (vnType == TYP_REF) || (vnType == TYP_BYREF)); + if (!((vnType == TYP_I_IMPL) || (vnType == TYP_REF) || (vnType == TYP_BYREF))) + { + printf("PeelOffsets: unexpected type %s\n", varTypeName(vnType)); + unreached(); + } #endif *offset = 0; From 618dcac640dbc8e9e40ee019cfd210afbaba641b Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 9 Feb 2025 18:53:14 +0100 Subject: [PATCH 7/8] fix ci issues --- src/coreclr/jit/valuenum.cpp | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 71313b00e92d0d..3de41dc1bccf3c 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -14398,20 +14398,25 @@ void Compiler::fgValueNumberAddExceptionSetForIndirection(GenTree* tree, GenTree assert(baseVNP.BothDefined()); - target_ssize_t offsetL; - vnStore->PeelOffsets(&baseLVN, &offsetL); - if (fgIsBigOffset(offsetL)) - { - // Reset baseLVN back to the full address expression - baseLVN = baseVNP.GetLiberal(); - } + target_ssize_t offsetL = 0; + target_ssize_t offsetC = 0; - target_ssize_t offsetC; - vnStore->PeelOffsets(&baseCVN, &offsetC); - if (fgIsBigOffset(offsetC)) + // baseAddr could be a SIMD for certain implicit indirs, e.g. GatherVector API. + if (!varTypeIsSIMD(baseAddr)) { - // Reset baseCVN back to the full address expression - baseCVN = baseVNP.GetConservative(); + vnStore->PeelOffsets(&baseLVN, &offsetL); + if (fgIsBigOffset(offsetL)) + { + // Reset baseLVN back to the full address expression + baseLVN = baseVNP.GetLiberal(); + } + + vnStore->PeelOffsets(&baseCVN, &offsetC); + if (fgIsBigOffset(offsetC)) + { + // Reset baseCVN back to the full address expression + baseCVN = baseVNP.GetConservative(); + } } // The exceptions in "baseVNP" should have been added to the "tree"'s set already. @@ -15156,11 +15161,7 @@ void ValueNumStore::PeelOffsets(ValueNum* vn, target_ssize_t* offset) { #ifdef DEBUG var_types vnType = TypeOfVN(*vn); - if (!((vnType == TYP_I_IMPL) || (vnType == TYP_REF) || (vnType == TYP_BYREF))) - { - printf("PeelOffsets: unexpected type %s\n", varTypeName(vnType)); - unreached(); - } + assert((vnType == TYP_I_IMPL) || (vnType == TYP_REF) || (vnType == TYP_BYREF)); #endif *offset = 0; From 9c02d2dad7030eba33cefe4482fe496432fceb80 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Mon, 10 Feb 2025 17:14:36 +0100 Subject: [PATCH 8/8] Remove check for big offsets in assertionprop.cpp --- src/coreclr/jit/assertionprop.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index ac111bef9abf14..ae8c38daadb92c 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -5006,11 +5006,6 @@ bool Compiler::optAssertionIsNonNull(GenTree* op, ASSERT_VALARG_TP assertions) ValueNum vnBase = vn; target_ssize_t offset = 0; vnStore->PeelOffsets(&vnBase, &offset); - if (fgIsBigOffset((size_t)offset)) - { - // Give up on big offsets - vnBase = vn; - } // Check each assertion to find if we have a vn != null assertion. //