Skip to content

Commit

Permalink
JIT: Process all EH successors for "in-block" defs in SSA (#94955)
Browse files Browse the repository at this point in the history
When we saw a def we were only adding phi args to enclosing handlers,
but this misses the special case of throws inside filters, where control
can flow to enclosed handlers.

Fix #94954
  • Loading branch information
jakobbotsch authored Nov 21, 2023
1 parent 784537a commit 79e42f9
Show file tree
Hide file tree
Showing 4 changed files with 147 additions and 126 deletions.
3 changes: 3 additions & 0 deletions src/coreclr/jit/block.h
Original file line number Diff line number Diff line change
Expand Up @@ -1415,6 +1415,9 @@ struct BasicBlock : private LIR::Range
template <typename TFunc>
BasicBlockVisit VisitAllSuccs(Compiler* comp, TFunc func);

template <typename TFunc>
BasicBlockVisit VisitEHSuccs(Compiler* comp, TFunc func);

template <typename TFunc>
BasicBlockVisit VisitRegularSuccs(Compiler* comp, TFunc func);

Expand Down
63 changes: 45 additions & 18 deletions src/coreclr/jit/compiler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -487,9 +487,14 @@ BasicBlockVisit BasicBlock::VisitEHSecondPassSuccs(Compiler* comp, TFunc func)
}

//------------------------------------------------------------------------------
// VisitEHSuccessors: Given a block inside a handler region, visit all handlers
// VisitEHSuccs: Given a block inside a handler region, visit all handlers
// that control may flow to as part of EH.
//
// Type arguments:
// skipJumpDest - Whether the jump destination has already been
// yielded, in which case it should be skipped here.
// TFunc - Functor type
//
// Arguments:
// comp - Compiler instance
// block - The block
Expand All @@ -499,12 +504,13 @@ BasicBlockVisit BasicBlock::VisitEHSecondPassSuccs(Compiler* comp, TFunc func)
// Whether or not the visiting should proceed.
//
// Remarks:
// This encapsulates the "exception handling" successors of a block. That is,
// if a basic block BB1 occurs in a try block, we consider the first basic
// block BB2 of the corresponding handler to be an "EH successor" of BB1.
// This encapsulates the "exception handling" successors of a block. These
// are the blocks that control may flow to as part of exception handling:
// 1. On thrown exceptions, control may flow to handlers
// 2. As part of two pass EH, control may flow from filters to enclosed handlers
//
template <typename TFunc>
static BasicBlockVisit VisitEHSuccessors(Compiler* comp, BasicBlock* block, TFunc func)
template <bool skipJumpDest, typename TFunc>
static BasicBlockVisit VisitEHSuccs(Compiler* comp, BasicBlock* block, TFunc func)
{
if (!block->HasPotentialEHSuccs(comp))
{
Expand All @@ -516,12 +522,10 @@ static BasicBlockVisit VisitEHSuccessors(Compiler* comp, BasicBlock* block, TFun
{
while (true)
{
// If the original block whose EH successors we're iterating over
// is a BBJ_CALLFINALLY, that finally clause's first block
// will be yielded as a normal successor. Don't also yield as
// an exceptional successor.
// For BBJ_CALLFINALLY the user may already have processed one of
// the EH successors as a regular successor; skip it if requested.
BasicBlock* flowBlock = eh->ExFlowBlock();
if (!block->KindIs(BBJ_CALLFINALLY) || !block->HasJumpTo(flowBlock))
if (!skipJumpDest || !block->HasJumpTo(flowBlock))
{
RETURN_ON_ABORT(func(flowBlock));
}
Expand All @@ -538,6 +542,26 @@ static BasicBlockVisit VisitEHSuccessors(Compiler* comp, BasicBlock* block, TFun
return block->VisitEHSecondPassSuccs(comp, func);
}

//------------------------------------------------------------------------------
// VisitEHSuccs: Given a block inside a handler region, visit all handlers
// that control may flow to as part of EH.
//
// Arguments:
// comp - Compiler instance
// func - Callback
//
// Returns:
// Whether or not the visiting should proceed.
//
// Remarks:
// For more documentation, see ::VisitEHSuccs.
//
template <typename TFunc>
BasicBlockVisit BasicBlock::VisitEHSuccs(Compiler* comp, TFunc func)
{
return ::VisitEHSuccs</* skipJumpDest */ false, TFunc>(comp, this, func);
}

//------------------------------------------------------------------------------
// VisitAllSuccs: Visit all successors (including EH successors) of this block.
//
Expand All @@ -559,14 +583,17 @@ BasicBlockVisit BasicBlock::VisitAllSuccs(Compiler* comp, TFunc func)
RETURN_ON_ABORT(func(bbJumpEhf->bbeSuccs[i]));
}

return VisitEHSuccessors(comp, this, func);
return VisitEHSuccs(comp, func);

case BBJ_CALLFINALLY:
RETURN_ON_ABORT(func(bbJumpDest));
return ::VisitEHSuccs</* skipJumpDest */ true, TFunc>(comp, this, func);

case BBJ_EHCATCHRET:
case BBJ_EHFILTERRET:
case BBJ_LEAVE:
RETURN_ON_ABORT(func(bbJumpDest));
return VisitEHSuccessors(comp, this, func);
return VisitEHSuccs(comp, func);

case BBJ_ALWAYS:
RETURN_ON_ABORT(func(bbJumpDest));
Expand All @@ -577,14 +604,14 @@ BasicBlockVisit BasicBlock::VisitAllSuccs(Compiler* comp, TFunc func)
// and we skip its normal EH successors.
if (!isBBCallAlwaysPairTail())
{
RETURN_ON_ABORT(VisitEHSuccessors(comp, this, func));
RETURN_ON_ABORT(VisitEHSuccs(comp, func));
}

return BasicBlockVisit::Continue;

case BBJ_NONE:
RETURN_ON_ABORT(func(bbNext));
return VisitEHSuccessors(comp, this, func);
return VisitEHSuccs(comp, func);

case BBJ_COND:
RETURN_ON_ABORT(func(bbNext));
Expand All @@ -594,7 +621,7 @@ BasicBlockVisit BasicBlock::VisitAllSuccs(Compiler* comp, TFunc func)
RETURN_ON_ABORT(func(bbJumpDest));
}

return VisitEHSuccessors(comp, this, func);
return VisitEHSuccs(comp, func);

case BBJ_SWITCH:
{
Expand All @@ -604,13 +631,13 @@ BasicBlockVisit BasicBlock::VisitAllSuccs(Compiler* comp, TFunc func)
RETURN_ON_ABORT(func(sd.nonDuplicates[i]));
}

return VisitEHSuccessors(comp, this, func);
return VisitEHSuccs(comp, func);
}

case BBJ_THROW:
case BBJ_RETURN:
case BBJ_EHFAULTRET:
return VisitEHSuccessors(comp, this, func);
return VisitEHSuccs(comp, func);

default:
unreached();
Expand Down
Loading

0 comments on commit 79e42f9

Please sign in to comment.