Skip to content

Commit

Permalink
JIT: revise local assertion prop to use bit vectors for live assertio…
Browse files Browse the repository at this point in the history
…n tracking

Track the set of active local assertions via a bit vector, rather than assuming
all entries in the table are live.

Doing so required a number of changes in assertion prop to ensure the vector
is consulted before deciding an assertion is valid.

This will (eventually) allow us to propagate assertions cross-block. For
now we reset the bit vector and assertion table back to empty at the start
of each block so nothing propagates past the end of a block.

The table can fill and cause the JIT to miss assertions in very large blocks
as morph will no longer remove assertions while processing a block. Previously
this would happen if there were more than 64 live assertions in a block, and
now it can happen if there are more than 64 assertions in block (so somewhat
more frequently).

Contributes to dotnet#93246.
  • Loading branch information
AndyAyersMS committed Nov 2, 2023
1 parent 655b177 commit f0b9ee7
Show file tree
Hide file tree
Showing 5 changed files with 124 additions and 143 deletions.
41 changes: 25 additions & 16 deletions src/coreclr/jit/assertionprop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -554,14 +554,14 @@ void Compiler::optAssertionInit(bool isLocalProp)

optLocalAssertionProp = isLocalProp;
optAssertionTabPrivate = new (this, CMK_AssertionProp) AssertionDsc[optMaxAssertionCount];
optComplementaryAssertionMap =
new (this, CMK_AssertionProp) AssertionIndex[optMaxAssertionCount + 1](); // zero-inited (NO_ASSERTION_INDEX)
assert(NO_ASSERTION_INDEX == 0);

if (!isLocalProp)
{
optValueNumToAsserts =
new (getAllocator(CMK_AssertionProp)) ValueNumToAssertsMap(getAllocator(CMK_AssertionProp));
optComplementaryAssertionMap = new (this, CMK_AssertionProp)
AssertionIndex[optMaxAssertionCount + 1](); // zero-inited (NO_ASSERTION_INDEX)
}

if (optAssertionDep == nullptr)
Expand All @@ -572,6 +572,7 @@ void Compiler::optAssertionInit(bool isLocalProp)

optAssertionTraitsInit(optMaxAssertionCount);
optAssertionCount = 0;
optAssertionOverflow = 0;
optAssertionPropagated = false;
bbJtrueAssertionOut = nullptr;
optCanPropLclVar = false;
Expand Down Expand Up @@ -1607,6 +1608,7 @@ AssertionIndex Compiler::optAddAssertion(AssertionDsc* newAssertion)
// Check if we are within max count.
if (optAssertionCount >= optMaxAssertionCount)
{
optAssertionOverflow++;
return NO_ASSERTION_INDEX;
}

Expand Down Expand Up @@ -2370,8 +2372,7 @@ void Compiler::optAssertionGen(GenTree* tree)
break;
}

// For global assertion prop we must store the assertion number in the tree node
if (assertionInfo.HasAssertion() && assertionProven && !optLocalAssertionProp)
if (assertionInfo.HasAssertion() && assertionProven)
{
tree->SetAssertionInfo(assertionInfo);
}
Expand Down Expand Up @@ -2459,9 +2460,7 @@ AssertionIndex Compiler::optAssertionIsSubrange(GenTree* tree, IntegralRange ran
for (AssertionIndex index = 1; index <= optAssertionCount; index++)
{
AssertionDsc* curAssertion = optGetAssertion(index);
if ((optLocalAssertionProp ||
BitVecOps::IsMember(apTraits, assertions, index - 1)) && // either local prop or use propagated assertions
curAssertion->CanPropSubRange())
if (BitVecOps::IsMember(apTraits, assertions, index - 1) && curAssertion->CanPropSubRange())
{
// For local assertion prop use comparison on locals, and use comparison on vns for global prop.
bool isEqual = optLocalAssertionProp
Expand Down Expand Up @@ -2493,13 +2492,13 @@ AssertionIndex Compiler::optAssertionIsSubrange(GenTree* tree, IntegralRange ran
*/
AssertionIndex Compiler::optAssertionIsSubtype(GenTree* tree, GenTree* methodTableArg, ASSERT_VALARG_TP assertions)
{
if (!optLocalAssertionProp && BitVecOps::IsEmpty(apTraits, assertions))
if (BitVecOps::IsEmpty(apTraits, assertions))
{
return NO_ASSERTION_INDEX;
}
for (AssertionIndex index = 1; index <= optAssertionCount; index++)
{
if (!optLocalAssertionProp && !BitVecOps::IsMember(apTraits, assertions, index - 1))
if (!BitVecOps::IsMember(apTraits, assertions, index - 1))
{
continue;
}
Expand Down Expand Up @@ -3594,15 +3593,15 @@ AssertionIndex Compiler::optLocalAssertionIsEqualOrNotEqual(
{
noway_assert((op1Kind == O1K_LCLVAR) || (op1Kind == O1K_EXACT_TYPE) || (op1Kind == O1K_SUBTYPE));
noway_assert((op2Kind == O2K_CONST_INT) || (op2Kind == O2K_IND_CNS_INT) || (op2Kind == O2K_ZEROOBJ));
if (!optLocalAssertionProp && BitVecOps::IsEmpty(apTraits, assertions))
if (BitVecOps::IsEmpty(apTraits, assertions))
{
return NO_ASSERTION_INDEX;
}

for (AssertionIndex index = 1; index <= optAssertionCount; ++index)
{
AssertionDsc* curAssertion = optGetAssertion(index);
if (optLocalAssertionProp || BitVecOps::IsMember(apTraits, assertions, index - 1))
if (BitVecOps::IsMember(apTraits, assertions, index - 1))
{
if ((curAssertion->assertionKind != OAK_EQUAL) && (curAssertion->assertionKind != OAK_NOT_EQUAL))
{
Expand Down Expand Up @@ -4386,17 +4385,27 @@ AssertionIndex Compiler::optAssertionIsNonNullInternal(GenTree* op,
}
else
{
unsigned lclNum = op->AsLclVarCommon()->GetLclNum();
// Check each assertion to find if we have a variable == or != null assertion.
for (AssertionIndex index = 1; index <= optAssertionCount; index++)
// Find live assertions related to lclNum
//
unsigned const lclNum = op->AsLclVarCommon()->GetLclNum();
ASSERT_TP apDependent = GetAssertionDep(lclNum);
BitVecOps::IntersectionD(apTraits, apDependent, apLocal);

// Scan those looking for a suitable assertion
//
BitVecOps::Iter iter(apTraits, assertions);
unsigned index = 0;
while (iter.NextElem(&index))
{
AssertionDsc* curAssertion = optGetAssertion(index);
AssertionIndex assertionIndex = GetAssertionIndex(index);
AssertionDsc* curAssertion = optGetAssertion(assertionIndex);

if ((curAssertion->assertionKind == OAK_NOT_EQUAL) && // kind
(curAssertion->op1.kind == O1K_LCLVAR) && // op1
(curAssertion->op2.kind == O2K_CONST_INT) && // op2
(curAssertion->op1.lcl.lclNum == lclNum) && (curAssertion->op2.u1.iconVal == 0))
{
return index;
return assertionIndex;
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/bitset.h
Original file line number Diff line number Diff line change
Expand Up @@ -172,11 +172,11 @@ class BitSetOps
// Returns "true" iff "bs" may be the uninit value.
static bool MayBeUninit(BitSetValueArgType bs);

// Returns the a new BitSet that is empty. Uses the Allocator of "env" to allocate memory for
// Returns a new BitSet that is empty. Uses the Allocator of "env" to allocate memory for
// the representation, if necessary.
static BitSetValueRetType MakeEmpty(Env env);

// Returns the a new BitSet that is "full" -- represents all the integers in the current range.
// Returns a new BitSet that is "full" -- represents all the integers in the current range.
// Uses the Allocator of "env" to allocate memory for the representation, if necessary.
static BitSetValueRetType MakeFull(Env env);

Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -6064,6 +6064,7 @@ class Compiler
GenTree* fgMorphTree(GenTree* tree, MorphAddrContext* mac = nullptr);

private:
void fgAssertionGen(GenTree* tree);
void fgKillDependentAssertionsSingle(unsigned lclNum DEBUGARG(GenTree* tree));
void fgKillDependentAssertions(unsigned lclNum DEBUGARG(GenTree* tree));
void fgMorphTreeDone(GenTree* tree);
Expand Down Expand Up @@ -7354,6 +7355,7 @@ class Compiler
// Data structures for assertion prop
BitVecTraits* apTraits;
ASSERT_TP apFull;
ASSERT_TP apLocal;

enum optAssertionKind
{
Expand Down Expand Up @@ -7638,6 +7640,7 @@ class Compiler
AssertionDsc* optAssertionTabPrivate; // table that holds info about value assignments
AssertionIndex optAssertionCount; // total number of assertions in the assertion table
AssertionIndex optMaxAssertionCount;
unsigned optAssertionOverflow;
bool optCanPropLclVar;
bool optCanPropEqual;
bool optCanPropNonNull;
Expand Down
Loading

0 comments on commit f0b9ee7

Please sign in to comment.