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

Use BBF_NO_CSE_IN more widely #104922

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
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
59 changes: 39 additions & 20 deletions src/coreclr/jit/redundantbranchopts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1628,6 +1628,31 @@ bool Compiler::optJumpThreadCore(JumpThreadInfo& jti)
//
JITDUMP("Optimizing via jump threading\n");

bool setNoCseIn = false;

// If this is a phi-based threading, and the block we're bypassing has
// a memory phi, mark the successor blocks with BBF_NO_CSE_IN so we can
// block unsound CSE propagation.
//
if (jti.m_isPhiBased)
{
for (MemoryKind memoryKind : allMemoryKinds())
{
if ((memoryKind == ByrefExposed) && byrefStatesMatchGcHeapStates)
{
continue;
}

if (jti.m_block->bbMemorySsaPhiFunc[memoryKind] != nullptr)
{
JITDUMP(FMT_BB " has %s memory phi; will be marking blocks with BBF_NO_CSE_IN\n", jti.m_block->bbNum,
memoryKindNames[memoryKind]);
setNoCseIn = true;
break;
}
}
}

// Now reroute the flow from the predecessors.
// If this pred is in the set that will reuse block, do nothing.
// Else revise pred to branch directly to the appropriate successor of block.
Expand All @@ -1638,6 +1663,11 @@ bool Compiler::optJumpThreadCore(JumpThreadInfo& jti)
//
if (BlockSetOps::IsMember(this, jti.m_ambiguousPreds, predBlock->bbNum))
{
if (setNoCseIn && !jti.m_block->HasFlag(BBF_NO_CSE_IN))
{
JITDUMP(FMT_BB " => BBF_NO_CSE_IN\n", jti.m_block->bbNum);
jti.m_block->SetFlags(BBF_NO_CSE_IN);
}
continue;
}

Expand All @@ -1652,6 +1682,12 @@ bool Compiler::optJumpThreadCore(JumpThreadInfo& jti)
predBlock->bbNum, jti.m_block->bbNum, predBlock->bbNum, jti.m_trueTarget->bbNum);

fgReplaceJumpTarget(predBlock, jti.m_block, jti.m_trueTarget);

if (setNoCseIn && !jti.m_trueTarget->HasFlag(BBF_NO_CSE_IN))
{
JITDUMP(FMT_BB " => BBF_NO_CSE_IN\n", jti.m_trueTarget->bbNum);
jti.m_trueTarget->SetFlags(BBF_NO_CSE_IN);
}
}
else
{
Expand All @@ -1660,28 +1696,11 @@ bool Compiler::optJumpThreadCore(JumpThreadInfo& jti)
predBlock->bbNum, jti.m_block->bbNum, predBlock->bbNum, jti.m_falseTarget->bbNum);

fgReplaceJumpTarget(predBlock, jti.m_block, jti.m_falseTarget);
}
}

// If this is a phi-based threading, and the block we're bypassing has
// a memory phi, mark the block with BBF_NO_CSE_IN so we can block CSE propagation
// into the block.
//
if (jti.m_isPhiBased)
{
for (MemoryKind memoryKind : allMemoryKinds())
{
if ((memoryKind == ByrefExposed) && byrefStatesMatchGcHeapStates)
{
continue;
}

if (jti.m_block->bbMemorySsaPhiFunc[memoryKind] != nullptr)
if (setNoCseIn && !jti.m_falseTarget->HasFlag(BBF_NO_CSE_IN))
{
JITDUMP(FMT_BB " has %s memory phi; marking as BBF_NO_CSE_IN\n", jti.m_block->bbNum,
memoryKindNames[memoryKind]);
jti.m_block->SetFlags(BBF_NO_CSE_IN);
break;
JITDUMP(FMT_BB " => BBF_NO_CSE_IN\n", jti.m_falseTarget->bbNum);
jti.m_falseTarget->SetFlags(BBF_NO_CSE_IN);
}
}
}
Expand Down
Loading