Skip to content

Commit 68d697a

Browse files
[release/11.0-preview2] Revert "Increase number of assertions (GlobalAP) + VN cache (#124132)" (#124955)
Backport of #124928 to release/11.0-preview2 /cc @akoeplinger @AndyAyersMS ## Customer Impact - [ ] Customer reported - [x] Found internally This was causing excessive memory allocation during jitting (see dotnet/dotnet#4933). ## Regression - [X] Yes - [ ] No Caused by: 92741be ## Testing Manual testing. ## Risk Low. Reverts an earlier change. [High/Medium/Low. Justify the indication by mentioning how risks were measured and addressed.] **IMPORTANT**: If this backport is for a servicing release, please verify that: - For .NET 8 and .NET 9: The PR target branch is `release/X.0-staging`, not `release/X.0`. - For .NET 10+: The PR target branch is `release/X.0` (no `-staging` suffix). ## Package authoring no longer needed in .NET 9 **IMPORTANT**: Starting with .NET 9, you no longer need to edit a NuGet package's csproj to enable building and bump the version. Keep in mind that we still need package authoring in .NET 8 and older versions. Co-authored-by: Andy Ayers <andya@microsoft.com>
1 parent a25236e commit 68d697a

File tree

3 files changed

+21
-91
lines changed

3 files changed

+21
-91
lines changed

src/coreclr/jit/assertionprop.cpp

Lines changed: 19 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -725,14 +725,16 @@ void Compiler::optAssertionInit(bool isLocalProp)
725725
optLocalAssertionProp = false;
726726
optCrossBlockLocalAssertionProp = false;
727727

728-
// Heuristic for sizing the assertion table.
728+
// Use a function countFunc to determine a proper maximum assertion count for the
729+
// method being compiled. The function is linear to the IL size for small and
730+
// moderate methods. For large methods, considering throughput impact, we track no
731+
// more than 64 assertions.
732+
// Note this tracks at most only 256 assertions.
729733
//
730-
// The weighting of basicBlocks vs locals reflects their relative contribution observed empirically.
731-
// Validated against 1,115,046 compiled methods:
732-
// - 94.6% of methods stay at the floor of 64 (only 1.9% actually need more).
733-
// - Underpredicts for 481 methods (0.043%), with a worst-case deficit of 127.
734-
// - Only 0.4% of methods hit the 256 cap.
735-
optMaxAssertionCount = (AssertionIndex)max(64, min(256, (int)(lvaTrackedCount + 3 * fgBBcount + 48) >> 2));
734+
static const AssertionIndex countFunc[] = {64, 128, 256, 128, 64};
735+
static const unsigned upperBound = ArrLen(countFunc) - 1;
736+
const unsigned codeSize = info.compILCodeSize / 512;
737+
optMaxAssertionCount = countFunc[min(upperBound, codeSize)];
736738

737739
optComplementaryAssertionMap = new (this, CMK_AssertionProp)
738740
AssertionIndex[optMaxAssertionCount + 1](); // zero-inited (NO_ASSERTION_INDEX)
@@ -1350,8 +1352,6 @@ bool Compiler::optIsTreeKnownIntValue(bool vnBased, GenTree* tree, ssize_t* pCon
13501352
*/
13511353
AssertionIndex Compiler::optAddAssertion(const AssertionDsc& newAssertion)
13521354
{
1353-
bool canAddNewAssertions = optAssertionCount < optMaxAssertionCount;
1354-
13551355
// See if we already have this assertion in the table.
13561356
//
13571357
// For local assertion prop we can speed things up by checking the dep vector.
@@ -1378,40 +1378,21 @@ AssertionIndex Compiler::optAddAssertion(const AssertionDsc& newAssertion)
13781378
}
13791379
else
13801380
{
1381-
bool mayHaveDuplicates =
1382-
optAssertionHasAssertionsForVN(newAssertion.GetOp1().GetVN(), /* addIfNotFound */ canAddNewAssertions);
1383-
// We need to register op2.vn too, even if we know for sure there are no duplicates
1384-
if (newAssertion.GetOp2().KindIs(O2K_CHECKED_BOUND_ADD_CNS))
1385-
{
1386-
mayHaveDuplicates |= optAssertionHasAssertionsForVN(newAssertion.GetOp2().GetCheckedBound(),
1387-
/* addIfNotFound */ canAddNewAssertions);
1388-
1389-
// Additionally, check for the pattern of "VN + const == checkedBndVN" and register "VN" as well.
1390-
ValueNum addOpVN;
1391-
if (vnStore->IsVNBinFuncWithConst<int>(newAssertion.GetOp1().GetVN(), VNF_ADD, &addOpVN, nullptr))
1392-
{
1393-
mayHaveDuplicates |= optAssertionHasAssertionsForVN(addOpVN, /* addIfNotFound */ canAddNewAssertions);
1394-
}
1395-
}
1396-
1397-
if (mayHaveDuplicates)
1381+
// For global prop we search the entire table.
1382+
//
1383+
// Check if exists already, so we can skip adding new one. Search backwards.
1384+
for (AssertionIndex index = optAssertionCount; index >= 1; index--)
13981385
{
1399-
// For global prop we search the entire table.
1400-
//
1401-
// Check if exists already, so we can skip adding new one. Search backwards.
1402-
for (AssertionIndex index = optAssertionCount; index >= 1; index--)
1386+
const AssertionDsc& curAssertion = optGetAssertion(index);
1387+
if (curAssertion.Equals(newAssertion, /* vnBased */ true))
14031388
{
1404-
const AssertionDsc& curAssertion = optGetAssertion(index);
1405-
if (curAssertion.Equals(newAssertion, /* vnBased */ true))
1406-
{
1407-
return index;
1408-
}
1389+
return index;
14091390
}
14101391
}
14111392
}
14121393

14131394
// Check if we are within max count.
1414-
if (!canAddNewAssertions)
1395+
if (optAssertionCount >= optMaxAssertionCount)
14151396
{
14161397
optAssertionOverflow++;
14171398
return NO_ASSERTION_INDEX;
@@ -1458,49 +1439,6 @@ AssertionIndex Compiler::optAddAssertion(const AssertionDsc& newAssertion)
14581439
return optAssertionCount;
14591440
}
14601441

1461-
//------------------------------------------------------------------------
1462-
// optAssertionHasAssertionsForVN: Check if we already have assertions for the given VN.
1463-
// If "addIfNotFound" is true, add the VN to the map if it's not already there.
1464-
//
1465-
// Arguments:
1466-
// vn - the VN to check for
1467-
// addIfNotFound - whether to add the VN to the map if it's not found
1468-
//
1469-
// Return Value:
1470-
// true if we already have assertions for the given VN, false otherwise.
1471-
//
1472-
bool Compiler::optAssertionHasAssertionsForVN(ValueNum vn, bool addIfNotFound)
1473-
{
1474-
assert(!optLocalAssertionProp);
1475-
if (vn == ValueNumStore::NoVN)
1476-
{
1477-
assert(!addIfNotFound);
1478-
return false;
1479-
}
1480-
1481-
if (addIfNotFound)
1482-
{
1483-
// Lazy initialize the map when we first need to add to it
1484-
if (optAssertionVNsMap == nullptr)
1485-
{
1486-
optAssertionVNsMap = new (this, CMK_AssertionProp) VNSet(getAllocator(CMK_AssertionProp));
1487-
}
1488-
1489-
// Avoid double lookup by using the return value of LookupPointerOrAdd to
1490-
// determine whether the VN was already in the map.
1491-
bool* pValue = optAssertionVNsMap->LookupPointerOrAdd(vn, false);
1492-
if (!*pValue)
1493-
{
1494-
*pValue = true;
1495-
return false;
1496-
}
1497-
return true;
1498-
}
1499-
1500-
// Otherwise just do a normal lookup
1501-
return (optAssertionVNsMap != nullptr) && optAssertionVNsMap->Lookup(vn);
1502-
}
1503-
15041442
#ifdef DEBUG
15051443
void Compiler::optDebugCheckAssertion(const AssertionDsc& assertion) const
15061444
{
@@ -3353,18 +3291,12 @@ GenTree* Compiler::optAssertionProp_LclVar(ASSERT_VALARG_TP assertions, GenTreeL
33533291
// For local assertion prop we can filter the assertion set down.
33543292
//
33553293
const unsigned lclNum = tree->GetLclNum();
3356-
ValueNum treeVN = optConservativeNormalVN(tree);
33573294

33583295
ASSERT_TP filteredAssertions = assertions;
33593296
if (optLocalAssertionProp)
33603297
{
33613298
filteredAssertions = BitVecOps::Intersection(apTraits, GetAssertionDep(lclNum), filteredAssertions);
33623299
}
3363-
else if (!optAssertionHasAssertionsForVN(treeVN))
3364-
{
3365-
// There are no assertions for this VN
3366-
return nullptr;
3367-
}
33683300

33693301
BitVecOps::Iter iter(apTraits, filteredAssertions);
33703302
unsigned index = 0;
@@ -3426,7 +3358,7 @@ GenTree* Compiler::optAssertionProp_LclVar(ASSERT_VALARG_TP assertions, GenTreeL
34263358
else
34273359
{
34283360
// Check VN in Global Assertion Prop
3429-
if (curAssertion.GetOp1().GetVN() == treeVN)
3361+
if (curAssertion.GetOp1().GetVN() == vnStore->VNConservativeNormalValue(tree->gtVNPair))
34303362
{
34313363
return optConstantAssertionProp(curAssertion, tree, stmt DEBUGARG(assertionIndex));
34323364
}
@@ -4740,7 +4672,7 @@ bool Compiler::optAssertionVNIsNonNull(ValueNum vn, ASSERT_VALARG_TP assertions)
47404672
return true;
47414673
}
47424674

4743-
if (!BitVecOps::MayBeUninit(assertions) && optAssertionHasAssertionsForVN(vn))
4675+
if (!BitVecOps::MayBeUninit(assertions))
47444676
{
47454677
BitVecOps::Iter iter(apTraits, assertions);
47464678
unsigned index = 0;

src/coreclr/jit/compiler.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8432,8 +8432,7 @@ class Compiler
84328432
JitExpandArray<ASSERT_TP>* optAssertionDep = nullptr; // table that holds dependent assertions (assertions
84338433
// using the value of a local var) for each local var
84348434
AssertionDsc* optAssertionTabPrivate; // table that holds info about assertions
8435-
VNSet* optAssertionVNsMap = nullptr;
8436-
AssertionIndex optAssertionCount = 0; // total number of assertions in the assertion table
8435+
AssertionIndex optAssertionCount = 0; // total number of assertions in the assertion table
84378436
AssertionIndex optMaxAssertionCount;
84388437
bool optCrossBlockLocalAssertionProp;
84398438
unsigned optAssertionOverflow;
@@ -8562,7 +8561,6 @@ class Compiler
85628561

85638562
bool optCreateJumpTableImpliedAssertions(BasicBlock* switchBb);
85648563

8565-
bool optAssertionHasAssertionsForVN(ValueNum vn, bool addIfNotFound = false);
85668564
#ifdef DEBUG
85678565
void optPrintAssertion(const AssertionDsc& newAssertion, AssertionIndex assertionIndex = 0);
85688566
void optPrintAssertionIndex(AssertionIndex index);

src/coreclr/jit/rangecheck.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -977,7 +977,7 @@ void RangeCheck::MergeEdgeAssertions(Compiler* comp,
977977
return;
978978
}
979979

980-
if (!comp->optAssertionHasAssertionsForVN(normalLclVN))
980+
if (normalLclVN == ValueNumStore::NoVN)
981981
{
982982
return;
983983
}

0 commit comments

Comments
 (0)