Skip to content

Fix removal of unreachable BBJ_CALLFINALLY/BBJ_ALWAYS on Linux/arm #94751

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
BruceForstall merged 1 commit into from
Nov 15, 2023
Merged

Fix removal of unreachable BBJ_CALLFINALLY/BBJ_ALWAYS on Linux/arm #94751

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
65 changes: 38 additions & 27 deletions src/coreclr/jit/fgopt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,14 +453,14 @@ bool Compiler::fgRemoveUnreachableBlocks(CanRemoveBlockBody canRemoveBlock)
// Make sure that the block was marked as removed */
noway_assert(block->bbFlags & BBF_REMOVED);

// Some blocks mark the end of trys and catches
// and can't be removed. We convert these into
// empty blocks of type BBJ_THROW
// Some blocks mark the end of trys and catches and can't be removed. We convert these into
// empty blocks of type BBJ_THROW.

const bool bIsBBCallAlwaysPair = block->isBBCallAlwaysPair();
BasicBlock* leaveBlk = bIsBBCallAlwaysPair ? block->Next() : nullptr;

if (block->bbFlags & BBF_DONT_REMOVE)
{
const bool bIsBBCallAlwaysPair = block->isBBCallAlwaysPair();

// Unmark the block as removed, clear BBF_INTERNAL, and set BBJ_IMPORTED

JITDUMP("Converting BBF_DONT_REMOVE block " FMT_BB " to BBJ_THROW\n", block->bbNum);
Expand All @@ -472,36 +472,47 @@ bool Compiler::fgRemoveUnreachableBlocks(CanRemoveBlockBody canRemoveBlock)
block->bbFlags |= BBF_IMPORTED;
block->SetJumpKindAndTarget(BBJ_THROW DEBUG_ARG(this));
block->bbSetRunRarely();
}
else
{
/* We have to call fgRemoveBlock next */
hasUnreachableBlocks = true;
changed = true;
}

// If this is a <BBJ_CALLFINALLY, BBJ_ALWAYS> pair, we just converted it to a BBJ_THROW.
// Get rid of the BBJ_ALWAYS block which is now dead.
if (bIsBBCallAlwaysPair)
// If this is a <BBJ_CALLFINALLY, BBJ_ALWAYS> pair, get rid of the BBJ_ALWAYS block which is now dead.
if (bIsBBCallAlwaysPair)
{
assert(leaveBlk->KindIs(BBJ_ALWAYS));

if (!block->KindIs(BBJ_THROW))
{
BasicBlock* leaveBlk = block->Next();
noway_assert(leaveBlk->KindIs(BBJ_ALWAYS));
// We didn't convert the BBJ_CALLFINALLY to a throw, above. Since we already marked it as removed,
// change the kind to something else. Otherwise, we can hit asserts below in fgRemoveBlock that
// the leaveBlk BBJ_ALWAYS is not allowed to be a CallAlwaysPairTail.
assert(block->KindIs(BBJ_CALLFINALLY));
block->SetJumpKind(BBJ_NONE);
}

leaveBlk->bbFlags &= ~BBF_DONT_REMOVE;
leaveBlk->bbFlags &= ~BBF_DONT_REMOVE;

for (BasicBlock* const leavePredBlock : leaveBlk->PredBlocks())
{
fgRemoveEhfSuccessor(leavePredBlock, leaveBlk);
}
assert(leaveBlk->bbRefs == 0);
assert(leaveBlk->bbPreds == nullptr);
for (BasicBlock* const leavePredBlock : leaveBlk->PredBlocks())
{
fgRemoveEhfSuccessor(leavePredBlock, leaveBlk);
}
assert(leaveBlk->bbRefs == 0);
assert(leaveBlk->bbPreds == nullptr);

fgRemoveBlock(leaveBlk, /* unreachable */ true);
fgRemoveBlock(leaveBlk, /* unreachable */ true);

#if defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM)
// We have to clear BBF_FINALLY_TARGET flag on the target node (of BBJ_ALWAYS).
fgClearFinallyTargetBit(leaveBlk->GetJumpDest());
// We have to clear BBF_FINALLY_TARGET flag on the target node (of BBJ_ALWAYS).
fgClearFinallyTargetBit(leaveBlk->GetJumpDest());
#endif // defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM)
}
}
else
{
/* We have to call fgRemoveBlock next */
hasUnreachableBlocks = true;
changed = true;

// Note: `changed` will already have been set to true by processing the BBJ_CALLFINALLY.
// `hasUnreachableBlocks` doesn't need to be set for the leaveBlk itself because we've already called
// `fgRemoveBlock` on it.
}
}

Expand Down