Skip to content

JIT: Refactor CSE candidate storage slightly #116009

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jakobbotsch merged 2 commits into from
May 28, 2025
Merged

JIT: Refactor CSE candidate storage slightly #116009

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
47f052d
JIT: Refactor CSE candidate storage slightly
jakobbotsch May 27, 2025
b8d4201
Update src/coreclr/jit/optcse.cpp
jakobbotsch May 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
210 changes: 99 additions & 111 deletions src/coreclr/jit/optcse.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,8 +298,8 @@ bool Compiler::optCSE_canSwap(GenTree* op1, GenTree* op2)
/* static */
bool Compiler::optCSEcostCmpEx::operator()(const CSEdsc* dsc1, const CSEdsc* dsc2)
{
GenTree* exp1 = dsc1->csdTree;
GenTree* exp2 = dsc2->csdTree;
GenTree* exp1 = dsc1->csdTreeList.tslTree;
GenTree* exp2 = dsc2->csdTreeList.tslTree;

auto expCost1 = exp1->GetCostEx();
auto expCost2 = exp2->GetCostEx();
Expand Down Expand Up @@ -334,8 +334,8 @@ bool Compiler::optCSEcostCmpEx::operator()(const CSEdsc* dsc1, const CSEdsc* dsc
/* static */
bool Compiler::optCSEcostCmpSz::operator()(const CSEdsc* dsc1, const CSEdsc* dsc2)
{
GenTree* exp1 = dsc1->csdTree;
GenTree* exp2 = dsc2->csdTree;
GenTree* exp1 = dsc1->csdTreeList.tslTree;
GenTree* exp2 = dsc2->csdTreeList.tslTree;

auto expCost1 = exp1->GetCostSz();
auto expCost2 = exp2->GetCostSz();
Expand Down Expand Up @@ -434,7 +434,7 @@ void CSEdsc::ComputeNumLocals(Compiler* compiler)
};

LocalCountingVisitor lcv(compiler);
lcv.WalkTree(&csdTree, nullptr);
lcv.WalkTree(&csdTreeList.tslTree, nullptr);

numDistinctLocals = lcv.m_count;
numLocalOccurrences = lcv.m_occurrences;
Expand Down Expand Up @@ -615,99 +615,85 @@ unsigned Compiler::optValnumCSE_Index(GenTree* tree, Statement* stmt)

for (hashDsc = optCSEhash[hval]; hashDsc; hashDsc = hashDsc->csdNextInBucket)
{
if (hashDsc->csdHashKey == key)
if (hashDsc->csdHashKey != key)
{
// Check for mismatched types on GT_CNS_INT nodes
if ((tree->OperGet() == GT_CNS_INT) && (tree->TypeGet() != hashDsc->csdTree->TypeGet()))
{
continue;
}
continue;
}

assert(hashDsc->csdTreeList.tslTree != nullptr);

treeStmtLst* newElem;
// Check for mismatched types on GT_CNS_INT nodes
if ((tree->OperGet() == GT_CNS_INT) && (tree->TypeGet() != hashDsc->csdTreeList.tslTree->TypeGet()))
{
continue;
}

// Have we started the list of matching nodes?
// Have we started the list of matching nodes?

if (hashDsc->csdTreeList == nullptr)
if (hashDsc->csdTreeList.tslNext == nullptr)
{
// This is the second time we see this value. Handle cases
// where the first value dominates the second one and we can
// already prove that the first one is _not_ going to be a
// valid def for the second one, due to the second one having
// more exceptions. This happens for example in code like
// CASTCLASS(x, y) where the "CASTCLASS" just adds exceptions
// on top of "x". In those cases it is always better to let the
// second value be the def.
// It also happens for GT_COMMA, but that one is special cased
// above; this handling is a less special-casey version of the
// GT_COMMA handling above. However, it is quite limited since
// it only handles the def/use being in the same block.
if (compCurBB == hashDsc->csdTreeList.tslBlock)
{
// This is the second time we see this value. Handle cases
// where the first value dominates the second one and we can
// already prove that the first one is _not_ going to be a
// valid def for the second one, due to the second one having
// more exceptions. This happens for example in code like
// CASTCLASS(x, y) where the "CASTCLASS" just adds exceptions
// on top of "x". In those cases it is always better to let the
// second value be the def.
// It also happens for GT_COMMA, but that one is special cased
// above; this handling is a less special-casey version of the
// GT_COMMA handling above. However, it is quite limited since
// it only handles the def/use being in the same block.
if (compCurBB == hashDsc->csdBlock)
GenTree* prevTree = hashDsc->csdTreeList.tslTree;
ValueNum prevVnLib = prevTree->GetVN(VNK_Liberal);
if (prevVnLib != vnLib)
{
GenTree* prevTree = hashDsc->csdTree;
ValueNum prevVnLib = prevTree->GetVN(VNK_Liberal);
if (prevVnLib != vnLib)
ValueNum prevExceptionSet = vnStore->VNExceptionSet(prevVnLib);
ValueNum curExceptionSet = vnStore->VNExceptionSet(vnLib);
if ((prevExceptionSet != curExceptionSet) &&
vnStore->VNExcIsSubset(curExceptionSet, prevExceptionSet))
{
ValueNum prevExceptionSet = vnStore->VNExceptionSet(prevVnLib);
ValueNum curExceptionSet = vnStore->VNExceptionSet(vnLib);
if ((prevExceptionSet != curExceptionSet) &&
vnStore->VNExcIsSubset(curExceptionSet, prevExceptionSet))
{
JITDUMP("Skipping CSE candidate for tree [%06u]; tree [%06u] is a better candidate with "
"more exceptions\n",
prevTree->gtTreeID, tree->gtTreeID);
prevTree->gtCSEnum = 0;
hashDsc->csdStmt = stmt;
hashDsc->csdTree = tree;
tree->gtCSEnum = (signed char)hashDsc->csdIndex;
return hashDsc->csdIndex;
}
JITDUMP("Skipping CSE candidate for tree [%06u]; tree [%06u] is a better candidate with "
"more exceptions\n",
prevTree->gtTreeID, tree->gtTreeID);
prevTree->gtCSEnum = 0;
hashDsc->csdTreeList.tslStmt = stmt;
hashDsc->csdTreeList.tslTree = tree;
tree->gtCSEnum = (signed char)hashDsc->csdIndex;
return hashDsc->csdIndex;
}
}

// Create the new element based upon the matching hashDsc element.

newElem = new (this, CMK_TreeStatementList) treeStmtLst;

newElem->tslTree = hashDsc->csdTree;
newElem->tslStmt = hashDsc->csdStmt;
newElem->tslBlock = hashDsc->csdBlock;
newElem->tslNext = nullptr;

/* Start the list with the first CSE candidate recorded */

hashDsc->csdTreeList = newElem;
hashDsc->csdTreeLast = newElem;

hashDsc->csdIsSharedConst = isSharedConst;
}

noway_assert(hashDsc->csdTreeList);
hashDsc->csdIsSharedConst = isSharedConst;
}

/* Append this expression to the end of the list */
// Append this expression to the end of the list

newElem = new (this, CMK_TreeStatementList) treeStmtLst;
treeStmtLst* newElem = new (this, CMK_TreeStatementList) treeStmtLst;

newElem->tslTree = tree;
newElem->tslStmt = stmt;
newElem->tslBlock = compCurBB;
newElem->tslNext = nullptr;
newElem->tslTree = tree;
newElem->tslStmt = stmt;
newElem->tslBlock = compCurBB;
newElem->tslNext = nullptr;

hashDsc->csdTreeLast->tslNext = newElem;
hashDsc->csdTreeLast = newElem;
hashDsc->csdTreeLast->tslNext = newElem;
hashDsc->csdTreeLast = newElem;

optDoCSE = true; // Found a duplicate CSE tree
optDoCSE = true; // Found a duplicate CSE tree

/* Have we assigned a CSE index? */
if (hashDsc->csdIndex == 0)
{
newCSE = true;
break;
}

assert(FitsIn<signed char>(hashDsc->csdIndex));
tree->gtCSEnum = ((signed char)hashDsc->csdIndex);
return hashDsc->csdIndex;
/* Have we assigned a CSE index? */
if (hashDsc->csdIndex == 0)
{
newCSE = true;
break;
}

assert(FitsIn<signed char>(hashDsc->csdIndex));
tree->gtCSEnum = ((signed char)hashDsc->csdIndex);
return hashDsc->csdIndex;
}

if (!newCSE)
Expand Down Expand Up @@ -763,10 +749,12 @@ unsigned Compiler::optValnumCSE_Index(GenTree* tree, Statement* stmt)
hashDsc->defExcSetPromise = vnStore->VNForEmptyExcSet();
hashDsc->defExcSetCurrent = vnStore->VNForNull(); // uninit value

hashDsc->csdTree = tree;
hashDsc->csdStmt = stmt;
hashDsc->csdBlock = compCurBB;
hashDsc->csdTreeList = nullptr;
hashDsc->csdTreeList.tslTree = tree;
hashDsc->csdTreeList.tslStmt = stmt;
hashDsc->csdTreeList.tslBlock = compCurBB;
hashDsc->csdTreeList.tslNext = nullptr;

hashDsc->csdTreeLast = &hashDsc->csdTreeList;

/* Append the entry to the hash bucket */

Expand Down Expand Up @@ -801,11 +789,11 @@ unsigned Compiler::optValnumCSE_Index(GenTree* tree, Statement* stmt)
hashDsc->csdIndex = CSEindex;

/* Update the gtCSEnum field in the original tree */
noway_assert(hashDsc->csdTreeList->tslTree->gtCSEnum == 0);
noway_assert(hashDsc->csdTreeList.tslTree->gtCSEnum == 0);
assert(FitsIn<signed char>(CSEindex));

hashDsc->csdTreeList->tslTree->gtCSEnum = ((signed char)CSEindex);
noway_assert(((unsigned)hashDsc->csdTreeList->tslTree->gtCSEnum) == CSEindex);
hashDsc->csdTreeList.tslTree->gtCSEnum = ((signed char)CSEindex);
noway_assert(((unsigned)hashDsc->csdTreeList.tslTree->gtCSEnum) == CSEindex);

tree->gtCSEnum = ((signed char)CSEindex);

Expand Down Expand Up @@ -975,7 +963,7 @@ void Compiler::optValnumCSE_InitDataFlow()
{
CSEdsc* dsc = optCSEtab[inx];
unsigned CSEindex = dsc->csdIndex;
treeStmtLst* lst = dsc->csdTreeList;
treeStmtLst* lst = &dsc->csdTreeList;
noway_assert(lst);

while (lst != nullptr)
Expand Down Expand Up @@ -1101,13 +1089,13 @@ void Compiler::optValnumCSE_SetUpAsyncByrefKills()
CSEdsc* dsc = optCSEtab[inx - 1];
assert(dsc->csdIndex == inx);
bool isByRef = false;
if (dsc->csdTree->TypeIs(TYP_BYREF))
if (dsc->csdTreeList.tslTree->TypeIs(TYP_BYREF))
{
isByRef = true;
}
else if (dsc->csdTree->TypeIs(TYP_STRUCT))
else if (dsc->csdTreeList.tslTree->TypeIs(TYP_STRUCT))
{
ClassLayout* layout = dsc->csdTree->GetLayout(this);
ClassLayout* layout = dsc->csdTreeList.tslTree->GetLayout(this);
isByRef = layout->HasGCByRef();
}

Expand Down Expand Up @@ -2530,14 +2518,14 @@ void CSE_HeuristicParameterized::GetFeatures(CSEdsc* cse, double* features)
return;
}

const unsigned char costEx = cse->csdTree->GetCostEx();
const unsigned char costEx = cse->csdTreeList.tslTree->GetCostEx();
const double deMinimis = 1e-3;
const double deMinimusAdj = -log(deMinimis);

features[0] = costEx;
features[1] = deMinimusAdj + log(max(deMinimis, cse->csdUseWtCnt));
features[2] = deMinimusAdj + log(max(deMinimis, cse->csdDefWtCnt));
features[3] = cse->csdTree->GetCostSz();
features[3] = cse->csdTreeList.tslTree->GetCostSz();
features[4] = cse->csdUseCount;
features[5] = cse->csdDefCount;

Expand All @@ -2547,9 +2535,9 @@ void CSE_HeuristicParameterized::GetFeatures(CSEdsc* cse, double* features)
const bool isLiveAcrossCall = cse->csdLiveAcrossCall;

features[6] = booleanScale * isLiveAcrossCall;
features[7] = booleanScale * varTypeUsesIntReg(cse->csdTree->TypeGet());
features[7] = booleanScale * varTypeUsesIntReg(cse->csdTreeList.tslTree->TypeGet());

const bool isConstant = cse->csdTree->OperIsConst();
const bool isConstant = cse->csdTreeList.tslTree->OperIsConst();
const bool isSharedConstant = cse->csdIsSharedConst;

features[8] = booleanScale * (isConstant & !isSharedConstant);
Expand All @@ -2575,7 +2563,7 @@ void CSE_HeuristicParameterized::GetFeatures(CSEdsc* cse, double* features)
unsigned maxPostorderNum = 0;
BasicBlock* minPostorderBlock = nullptr;
BasicBlock* maxPostorderBlock = nullptr;
for (treeStmtLst* treeList = cse->csdTreeList; treeList != nullptr; treeList = treeList->tslNext)
for (treeStmtLst* treeList = &cse->csdTreeList; treeList != nullptr; treeList = treeList->tslNext)
{
BasicBlock* const treeBlock = treeList->tslBlock;
unsigned postorderNum = treeBlock->bbPostorderNum;
Expand Down Expand Up @@ -2604,12 +2592,12 @@ void CSE_HeuristicParameterized::GetFeatures(CSEdsc* cse, double* features)

// More
//
features[17] = booleanScale * ((cse->csdTree->gtFlags & GTF_CALL) != 0);
features[17] = booleanScale * ((cse->csdTreeList.tslTree->gtFlags & GTF_CALL) != 0);
features[18] = deMinimusAdj + log(max(deMinimis, cse->csdUseCount * cse->csdUseWtCnt));
features[19] = deMinimusAdj + log(max(deMinimis, cse->numLocalOccurrences * cse->csdUseWtCnt));
features[20] = booleanScale * ((double)(blockSpread) / numBBs);

const bool isContainable = cse->csdTree->OperIs(GT_ADD, GT_NOT, GT_MUL, GT_LSH);
const bool isContainable = cse->csdTreeList.tslTree->OperIs(GT_ADD, GT_NOT, GT_MUL, GT_LSH);
features[21] = booleanScale * isContainable;
features[22] = booleanScale * (isContainable && isLowCost);

Expand Down Expand Up @@ -3174,7 +3162,7 @@ void CSE_HeuristicRLHook::GetFeatures(CSEdsc* cse, int* features)
unsigned maxPostorderNum = 0;
BasicBlock* minPostorderBlock = nullptr;
BasicBlock* maxPostorderBlock = nullptr;
for (treeStmtLst* treeList = cse->csdTreeList; treeList != nullptr; treeList = treeList->tslNext)
for (treeStmtLst* treeList = &cse->csdTreeList; treeList != nullptr; treeList = treeList->tslNext)
{
BasicBlock* const treeBlock = treeList->tslBlock;
unsigned postorderNum = treeBlock->bbPostorderNum;
Expand Down Expand Up @@ -3234,13 +3222,13 @@ void CSE_HeuristicRLHook::GetFeatures(CSEdsc* cse, int* features)
features[i++] = type;
features[i++] = cse->IsViable() ? 1 : 0;
features[i++] = cse->csdLiveAcrossCall ? 1 : 0;
features[i++] = cse->csdTree->OperIsConst() ? 1 : 0;
features[i++] = cse->csdTreeList.tslTree->OperIsConst() ? 1 : 0;
features[i++] = cse->csdIsSharedConst ? 1 : 0;
features[i++] = isMakeCse ? 1 : 0;
features[i++] = ((cse->csdTree->gtFlags & GTF_CALL) != 0) ? 1 : 0;
features[i++] = cse->csdTree->OperIs(GT_ADD, GT_NOT, GT_MUL, GT_LSH) ? 1 : 0;
features[i++] = cse->csdTree->GetCostEx();
features[i++] = cse->csdTree->GetCostSz();
features[i++] = ((cse->csdTreeList.tslTree->gtFlags & GTF_CALL) != 0) ? 1 : 0;
features[i++] = cse->csdTreeList.tslTree->OperIs(GT_ADD, GT_NOT, GT_MUL, GT_LSH) ? 1 : 0;
features[i++] = cse->csdTreeList.tslTree->GetCostEx();
features[i++] = cse->csdTreeList.tslTree->GetCostSz();
features[i++] = cse->csdUseCount;
features[i++] = cse->csdDefCount;
features[i++] = (int)cse->csdUseWtCnt;
Expand Down Expand Up @@ -4273,7 +4261,7 @@ void CSE_Heuristic::SortCandidates()
for (unsigned cnt = 0; cnt < m_pCompiler->optCSECandidateCount; cnt++)
{
CSEdsc* dsc = sortTab[cnt];
GenTree* expr = dsc->csdTree;
GenTree* expr = dsc->csdTreeList.tslTree;

weight_t def;
weight_t use;
Expand All @@ -4283,13 +4271,13 @@ void CSE_Heuristic::SortCandidates()
{
def = dsc->csdDefCount; // def count
use = dsc->csdUseCount; // use count (excluding the implicit uses at defs)
cost = dsc->csdTree->GetCostSz();
cost = dsc->csdTreeList.tslTree->GetCostSz();
}
else
{
def = dsc->csdDefWtCnt; // weighted def count
use = dsc->csdUseWtCnt; // weighted use count (excluding the implicit uses at defs)
cost = dsc->csdTree->GetCostEx();
cost = dsc->csdTreeList.tslTree->GetCostEx();
}

if (!Compiler::Is_Shared_Const_CSE(dsc->csdHashKey))
Expand Down Expand Up @@ -4842,7 +4830,7 @@ void CSE_HeuristicCommon::PerformCSE(CSE_Candidate* successfulCandidate)
ValueNum bestVN = ValueNumStore::NoVN;
bool bestIsDef = false;
ssize_t bestConstValue = 0;
treeStmtLst* lst = dsc->csdTreeList;
treeStmtLst* lst = &dsc->csdTreeList;

while (lst != nullptr)
{
Expand Down Expand Up @@ -4933,7 +4921,7 @@ void CSE_HeuristicCommon::PerformCSE(CSE_Candidate* successfulCandidate)
}
else // !isSharedConst
{
lst = dsc->csdTreeList;
lst = &dsc->csdTreeList;
GenTree* firstTree = lst->tslTree;
printf("In %s, CSE (oper = %s, type = %s) has differing VNs: ", m_pCompiler->info.compFullName,
GenTree::OpName(firstTree->OperGet()), varTypeName(firstTree->TypeGet()));
Expand All @@ -4958,7 +4946,7 @@ void CSE_HeuristicCommon::PerformCSE(CSE_Candidate* successfulCandidate)
ArrayStack<UseDefLocation> defUses(m_pCompiler->getAllocator(CMK_CSE));

// First process the defs.
for (lst = dsc->csdTreeList; lst != nullptr; lst = lst->tslNext)
for (lst = &dsc->csdTreeList; lst != nullptr; lst = lst->tslNext)
{
GenTree* const exp = lst->tslTree;
Statement* const stmt = lst->tslStmt;
Expand Down Expand Up @@ -5067,7 +5055,7 @@ void CSE_HeuristicCommon::PerformCSE(CSE_Candidate* successfulCandidate)
}

// Now process the actual uses.
for (lst = dsc->csdTreeList; lst != nullptr; lst = lst->tslNext)
for (lst = &dsc->csdTreeList; lst != nullptr; lst = lst->tslNext)
{
GenTree* const exp = lst->tslTree;
Statement* const stmt = lst->tslStmt;
Expand Down
Loading
Loading