Skip to content
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.

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

JIT: Add a primitive to split statements at a specified tree #83005

Merged
merged 22 commits into from
Mar 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
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
162 changes: 162 additions & 0 deletions src/coreclr/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1776,6 +1776,7 @@ void Compiler::compInit(ArenaAllocator* pAlloc,
// set this early so we can use it without relying on random memory values
verbose = compIsForInlining() ? impInlineInfo->InlinerCompiler->verbose : false;

compNumStatementLinksTraversed = 0;
compPoisoningAnyImplicitByrefs = false;
#endif

Expand Down Expand Up @@ -4995,6 +4996,10 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
fgDomsComputed = false;
optLoopTableValid = false;

#ifdef DEBUG
DoPhase(this, PHASE_STRESS_SPLIT_TREE, &Compiler::StressSplitTree);
#endif

// Insert GC Polls
DoPhase(this, PHASE_INSERT_GC_POLLS, &Compiler::fgInsertGCPolls);

Expand Down Expand Up @@ -5336,6 +5341,163 @@ PhaseStatus Compiler::placeLoopAlignInstructions()
}
#endif

//------------------------------------------------------------------------
// StressSplitTree: A phase that stresses the gtSplitTree function.
//
// Returns:
// Suitable phase status
//
// Notes:
// Stress is applied on a function-by-function basis
//
PhaseStatus Compiler::StressSplitTree()
{
if (compStressCompile(STRESS_SPLIT_TREES_RANDOMLY, 10))
{
SplitTreesRandomly();
return PhaseStatus::MODIFIED_EVERYTHING;
}

if (compStressCompile(STRESS_SPLIT_TREES_REMOVE_COMMAS, 10))
{
SplitTreesRemoveCommas();
return PhaseStatus::MODIFIED_EVERYTHING;
}

return PhaseStatus::MODIFIED_NOTHING;
}

//------------------------------------------------------------------------
// SplitTreesRandomly: Split all statements at a random location.
//
void Compiler::SplitTreesRandomly()
{
#ifdef DEBUG
CLRRandom rng;
jakobbotsch marked this conversation as resolved.
Show resolved Hide resolved
rng.Init(info.compMethodHash() ^ 0x077cc4d4);

for (BasicBlock* block : Blocks())
{
for (Statement* stmt : block->NonPhiStatements())
{
int numTrees = 0;
for (GenTree* tree : stmt->TreeList())
{
if (tree->OperIs(GT_JTRUE)) // Due to relop invariant
{
continue;
}

numTrees++;
}

int splitTree = rng.Next(numTrees);
for (GenTree* tree : stmt->TreeList())
{
if (tree->OperIs(GT_JTRUE))
continue;

if (splitTree == 0)
{
JITDUMP("Splitting " FMT_STMT " at [%06u]\n", stmt->GetID(), dspTreeID(tree));
Statement* newStmt;
GenTree** use;
if (gtSplitTree(block, stmt, tree, &newStmt, &use))
{
while ((newStmt != nullptr) && (newStmt != stmt))
{
fgMorphStmtBlockOps(block, newStmt);
newStmt = newStmt->GetNextStmt();
}

fgMorphStmtBlockOps(block, stmt);
gtUpdateStmtSideEffects(stmt);
}
Comment on lines +5405 to +5415
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@EgorBo Your PR will need to do something similar like this, but note that calling fgMorphStmtBlockOps can invalidate the use, so it should be done only after you've replaced it with the new local.


break;
}

splitTree--;
}
}
}
jakobbotsch marked this conversation as resolved.
Show resolved Hide resolved
#endif
}

//------------------------------------------------------------------------
// SplitTreesRemoveCommas: Split trees to remove all commas.
//
void Compiler::SplitTreesRemoveCommas()
{
for (BasicBlock* block : Blocks())
{
Statement* stmt = block->FirstNonPhiDef();
while (stmt != nullptr)
{
Statement* nextStmt = stmt->GetNextStmt();
for (GenTree* tree : stmt->TreeList())
{
if (!tree->OperIs(GT_COMMA))
{
continue;
}

// Supporting this weird construct would require additional
// handling, we need to sort of move the comma into to the
// next node in execution order. We don't see this so just
// skip it.
assert(!tree->IsReverseOp());

JITDUMP("Removing COMMA [%06u]\n", dspTreeID(tree));
Statement* newStmt;
GenTree** use;
gtSplitTree(block, stmt, tree, &newStmt, &use);
GenTree* op1SideEffects = nullptr;
gtExtractSideEffList(tree->gtGetOp1(), &op1SideEffects);

if (op1SideEffects != nullptr)
{
Statement* op1Stmt = fgNewStmtFromTree(op1SideEffects);
fgInsertStmtBefore(block, stmt, op1Stmt);
if (newStmt == nullptr)
{
newStmt = op1Stmt;
}
}

*use = tree->gtGetOp2();

for (Statement* cur = newStmt; (cur != nullptr) && (cur != stmt); cur = cur->GetNextStmt())
{
fgMorphStmtBlockOps(block, cur);
}

fgMorphStmtBlockOps(block, stmt);
gtUpdateStmtSideEffects(stmt);

// Morphing block ops can introduce commas (and the original
// statement can also have more commas left). Proceed from the
// earliest newly introduced statement.
nextStmt = newStmt != nullptr ? newStmt : stmt;
break;
}

stmt = nextStmt;
}
}

for (BasicBlock* block : Blocks())
{
for (Statement* stmt : block->NonPhiStatements())
{
for (GenTree* tree : stmt->TreeList())
{
assert(!tree->OperIs(GT_COMMA));
}
}
}
}

//------------------------------------------------------------------------
// generatePatchpointInfo: allocate and fill in patchpoint info data,
// and report it to the VM
Expand Down
11 changes: 11 additions & 0 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -2975,6 +2975,9 @@ class Compiler
GenTreeFlags GenTreeFlags = GTF_SIDE_EFFECT,
bool ignoreRoot = false);

bool gtSplitTree(
BasicBlock* block, Statement* stmt, GenTree* splitPoint, Statement** firstNewStmt, GenTree*** splitPointUse);

// Static fields of struct types (and sometimes the types that those are reduced to) are represented by having the
// static field contain an object pointer to the boxed struct. This simplifies the GC implementation...but
// complicates the JIT somewhat. This predicate returns "true" iff a node with type "fieldNodeType", representing
Expand Down Expand Up @@ -4732,6 +4735,7 @@ class Compiler
void fgMergeBlockReturn(BasicBlock* block);

bool fgMorphBlockStmt(BasicBlock* block, Statement* stmt DEBUGARG(const char* msg));
void fgMorphStmtBlockOps(BasicBlock* block, Statement* stmt);

//------------------------------------------------------------------------------------------------------------
// MorphMDArrayTempCache: a simple cache of compiler temporaries in the local variable table, used to minimize
Expand Down Expand Up @@ -5274,6 +5278,9 @@ class Compiler
// Initialize the per-block variable sets (used for liveness analysis).
void fgInitBlockVarSets();

PhaseStatus StressSplitTree();
void SplitTreesRandomly();
void SplitTreesRemoveCommas();
PhaseStatus fgInsertGCPolls();
BasicBlock* fgCreateGCPoll(GCPollType pollType, BasicBlock* block);

Expand Down Expand Up @@ -5913,9 +5920,11 @@ class Compiler
CORINFO_CONTEXT_HANDLE* ExactContextHnd,
methodPointerInfo* ldftnToken);
GenTree* fgMorphLeaf(GenTree* tree);
public:
GenTree* fgMorphInitBlock(GenTree* tree);
GenTree* fgMorphCopyBlock(GenTree* tree);
GenTree* fgMorphStoreDynBlock(GenTreeStoreDynBlk* tree);
private:
GenTree* fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optAssertionPropDone = nullptr);
void fgTryReplaceStructLocalWithField(GenTree* tree);
GenTree* fgOptimizeCast(GenTreeCast* cast);
Expand Down Expand Up @@ -9770,6 +9779,8 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
STRESS_MODE(PROMOTE_FEWER_STRUCTS)/* Don't promote some structs that can be promoted */ \
STRESS_MODE(VN_BUDGET)/* Randomize the VN budget */ \
STRESS_MODE(SSA_INFO) /* Select lower thresholds for "complex" SSA num encoding */ \
STRESS_MODE(SPLIT_TREES_RANDOMLY) /* Split all statements at a random tree */ \
STRESS_MODE(SPLIT_TREES_REMOVE_COMMAS) /* Remove all GT_COMMA nodes */ \
\
/* After COUNT_VARN, stress level 2 does all of these all the time */ \
\
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/compphases.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ CompPhaseNameMacro(PHASE_IF_CONVERSION, "If conversion",
CompPhaseNameMacro(PHASE_VN_BASED_DEAD_STORE_REMOVAL,"VN-based dead store removal", false, -1, false)
CompPhaseNameMacro(PHASE_OPT_UPDATE_FLOW_GRAPH, "Update flow graph opt pass", false, -1, false)
CompPhaseNameMacro(PHASE_COMPUTE_EDGE_WEIGHTS2, "Compute edge weights (2, false)",false, -1, false)
CompPhaseNameMacro(PHASE_STRESS_SPLIT_TREE, "Stress gtSplitTree", false, -1, false)
CompPhaseNameMacro(PHASE_INSERT_GC_POLLS, "Insert GC Polls", false, -1, true)
CompPhaseNameMacro(PHASE_DETERMINE_FIRST_COLD_BLOCK, "Determine first cold block", false, -1, true)
CompPhaseNameMacro(PHASE_RATIONALIZE, "Rationalize IR", false, -1, false)
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/fgstmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ bool Compiler::fgBlockContainsStatementBounded(BasicBlock* block,
Statement* stmt,
bool answerOnBoundExceeded /*= true*/)
{
const __int64 maxLinks = 1000000000;
const __int64 maxLinks = 100000000;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought this limit was a bit on the high side before.


__int64* numTraversed = &JitTls::GetCompiler()->compNumStatementLinksTraversed;

Expand Down
Loading