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

Fix phase order problem with throw helper blocks #97201

Merged
merged 1 commit into from
Jan 19, 2024
Merged
Show file tree
Hide file tree
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
11 changes: 8 additions & 3 deletions src/coreclr/jit/codegencommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -455,10 +455,13 @@ void CodeGen::genMarkLabelsForCodegen()
}

// Walk all the exceptional code blocks and mark them, since they don't appear in the normal flow graph.
for (Compiler::AddCodeDsc* add = compiler->fgAddCodeList; add; add = add->acdNext)
for (Compiler::AddCodeDsc* add = compiler->fgAddCodeList; add != nullptr; add = add->acdNext)
{
JITDUMP(" " FMT_BB " : throw helper block\n", add->acdDstBlk->bbNum);
add->acdDstBlk->SetFlags(BBF_HAS_LABEL);
if (add->acdUsed)
{
JITDUMP(" " FMT_BB " : throw helper block\n", add->acdDstBlk->bbNum);
add->acdDstBlk->SetFlags(BBF_HAS_LABEL);
}
}

for (EHblkDsc* const HBtab : EHClauses(compiler))
Expand Down Expand Up @@ -1521,6 +1524,7 @@ void CodeGen::genJumpToThrowHlpBlk(emitJumpKind jumpKind, SpecialCodeKind codeKi
#ifdef DEBUG
Compiler::AddCodeDsc* add =
compiler->fgFindExcptnTarget(codeKind, compiler->bbThrowIndex(compiler->compCurBB));
assert(add->acdUsed);
assert(excpRaisingBlock == add->acdDstBlk);
#if !FEATURE_FIXED_OUT_ARGS
assert(add->acdStkLvlInit || isFramePointerUsed());
Expand All @@ -1533,6 +1537,7 @@ void CodeGen::genJumpToThrowHlpBlk(emitJumpKind jumpKind, SpecialCodeKind codeKi
Compiler::AddCodeDsc* add =
compiler->fgFindExcptnTarget(codeKind, compiler->bbThrowIndex(compiler->compCurBB));
PREFIX_ASSUME_MSG((add != nullptr), ("ERROR: failed to find exception throw block"));
assert(add->acdUsed);
excpRaisingBlock = add->acdDstBlk;
#if !FEATURE_FIXED_OUT_ARGS
assert(add->acdStkLvlInit || isFramePointerUsed());
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/jit/codegenloongarch64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7671,6 +7671,7 @@ inline void CodeGen::genJumpToThrowHlpBlk_la(
#ifdef DEBUG
Compiler::AddCodeDsc* add =
compiler->fgFindExcptnTarget(codeKind, compiler->bbThrowIndex(compiler->compCurBB));
assert(add->acdUsed);
assert(excpRaisingBlock == add->acdDstBlk);
#if !FEATURE_FIXED_OUT_ARGS
assert(add->acdStkLvlInit || isFramePointerUsed());
Expand All @@ -7683,6 +7684,7 @@ inline void CodeGen::genJumpToThrowHlpBlk_la(
Compiler::AddCodeDsc* add =
compiler->fgFindExcptnTarget(codeKind, compiler->bbThrowIndex(compiler->compCurBB));
PREFIX_ASSUME_MSG((add != nullptr), ("ERROR: failed to find exception throw block"));
assert(add->acdUsed);
excpRaisingBlock = add->acdDstBlk;
#if !FEATURE_FIXED_OUT_ARGS
assert(add->acdStkLvlInit || isFramePointerUsed());
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/jit/codegenriscv64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7186,6 +7186,7 @@ inline void CodeGen::genJumpToThrowHlpBlk_la(
#ifdef DEBUG
Compiler::AddCodeDsc* add =
compiler->fgFindExcptnTarget(codeKind, compiler->bbThrowIndex(compiler->compCurBB));
assert(add->acdUsed);
assert(excpRaisingBlock == add->acdDstBlk);
#if !FEATURE_FIXED_OUT_ARGS
assert(add->acdStkLvlInit || isFramePointerUsed());
Expand All @@ -7198,6 +7199,7 @@ inline void CodeGen::genJumpToThrowHlpBlk_la(
Compiler::AddCodeDsc* add =
compiler->fgFindExcptnTarget(codeKind, compiler->bbThrowIndex(compiler->compCurBB));
PREFIX_ASSUME_MSG((add != nullptr), ("ERROR: failed to find exception throw block"));
assert(add->acdUsed);
excpRaisingBlock = add->acdDstBlk;
#if !FEATURE_FIXED_OUT_ARGS
assert(add->acdStkLvlInit || isFramePointerUsed());
Expand Down
6 changes: 5 additions & 1 deletion src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -6554,7 +6554,11 @@ class Compiler
struct AddCodeDsc
{
AddCodeDsc* acdNext;
BasicBlock* acdDstBlk; // block to which we jump

// Initially the source block of the exception. After fgCreateThrowHelperBlocks, the block to which
// we jump to raise the exception.
BasicBlock* acdDstBlk;

unsigned acdData;
SpecialCodeKind acdKind; // what kind of a special block is this?
bool acdUsed; // do we need to keep this helper block?
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/compiler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -3045,7 +3045,7 @@ inline bool Compiler::fgIsThrowHlpBlk(BasicBlock* block)
// under stress, with implausible flow graph optimizations. So, walk the fgAddCodeList
// for the final determination.

for (AddCodeDsc* add = fgAddCodeList; add; add = add->acdNext)
for (AddCodeDsc* add = fgAddCodeList; add != nullptr; add = add->acdNext)
{
if (block == add->acdDstBlk)
{
Expand All @@ -3068,7 +3068,7 @@ inline bool Compiler::fgIsThrowHlpBlk(BasicBlock* block)

inline unsigned Compiler::fgThrowHlpBlkStkLevel(BasicBlock* block)
{
for (AddCodeDsc* add = fgAddCodeList; add; add = add->acdNext)
for (AddCodeDsc* add = fgAddCodeList; add != nullptr; add = add->acdNext)
{
if (block == add->acdDstBlk)
{
Expand Down
10 changes: 9 additions & 1 deletion src/coreclr/jit/emit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4299,7 +4299,15 @@ void emitter::emitDispJumpList()
else
#endif // TARGET_ARM64
{
printf(" -> IG%02u", ((insGroup*)emitCodeGetCookie(jmp->idAddr()->iiaBBlabel))->igNum);
insGroup* targetGroup = (insGroup*)emitCodeGetCookie(jmp->idAddr()->iiaBBlabel);
if (targetGroup == nullptr)
{
printf(" -> ILLEGAL");
}
else
{
printf(" -> IG%02u", targetGroup->igNum);
}
}

if (jmp->idjShort)
Expand Down
8 changes: 4 additions & 4 deletions src/coreclr/jit/flowgraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3539,15 +3539,15 @@ PhaseStatus Compiler::fgCreateThrowHelperBlocks()
}

//------------------------------------------------------------------------
// fgFindExcptnTarget: finds the block to jump that will throw a given kind of exception
// fgFindExcptnTarget: finds the block to jump to that will throw a given kind of exception
//
// Arguments:
// kind - kind of exception to throw
// kind -- kind of exception to throw
// refData -- bbThrowIndex of the block that will jump to the throw helper
//
// Return Value:
// Code descriptor for the appropriate throw helper block, or nullptr if no such
// descriptor exists
// descriptor exists.
//
Compiler::AddCodeDsc* Compiler::fgFindExcptnTarget(SpecialCodeKind kind, unsigned refData)
{
Expand All @@ -3559,7 +3559,7 @@ Compiler::AddCodeDsc* Compiler::fgFindExcptnTarget(SpecialCodeKind kind, unsigne

if (add == nullptr)
{
// We should't be asking for these blocks late in compilation
// We shouldn't be asking for these blocks late in compilation
// unless we know there are entries to be found.
assert(!fgRngChkThrowAdded);
}
Expand Down
11 changes: 4 additions & 7 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6908,7 +6908,6 @@ bool GenTree::OperIsImplicitIndir() const
//------------------------------------------------------------------------------
// OperExceptions: Get exception set this tree may throw.
//
//
// Arguments:
// comp - Compiler instance
//
Expand Down Expand Up @@ -6945,11 +6944,9 @@ ExceptionSetFlags GenTree::OperExceptions(Compiler* comp)

ExceptionSetFlags exSetFlags = ExceptionSetFlags::None;

GenTree* op2 = this->gtGetOp2();

if (!(this->gtFlags & GTF_DIV_MOD_NO_BY_ZERO) && !op2->IsNeverZero())
if (!(this->gtFlags & GTF_DIV_MOD_NO_BY_ZERO) && !this->gtGetOp2()->gtSkipReloadOrCopy()->IsNeverZero())
{
exSetFlags = ExceptionSetFlags::DivideByZeroException;
exSetFlags |= ExceptionSetFlags::DivideByZeroException;
}

if (this->OperIs(GT_DIV, GT_MOD) && this->CanDivOrModPossiblyOverflow(comp))
Expand Down Expand Up @@ -27217,8 +27214,8 @@ bool GenTree::CanDivOrModPossiblyOverflow(Compiler* comp) const
if (this->gtFlags & GTF_DIV_MOD_NO_OVERFLOW)
return false;

GenTree* op1 = this->gtGetOp1();
GenTree* op2 = this->gtGetOp2();
GenTree* op1 = this->gtGetOp1()->gtSkipReloadOrCopy();
GenTree* op2 = this->gtGetOp2()->gtSkipReloadOrCopy();

// If the divisor is known to never be '-1', we cannot overflow.
if (op2->IsNeverNegativeOne(comp))
Expand Down
5 changes: 2 additions & 3 deletions src/coreclr/jit/gentree.h
Original file line number Diff line number Diff line change
Expand Up @@ -8910,7 +8910,6 @@ inline bool GenTree::OperIsCopyBlkOp()
// long constants in a target-independent way.

inline bool GenTree::IsIntegralConst(ssize_t constVal) const

{
if ((gtOper == GT_CNS_INT) && (AsIntConCommon()->IconValue() == constVal))
{
Expand Down Expand Up @@ -9328,9 +9327,9 @@ inline GenTree* GenTree::gtCommaStoreVal()
inline GenTree* GenTree::gtSkipReloadOrCopy()
{
// There can be only one reload or copy (we can't have a reload/copy of a reload/copy)
if (gtOper == GT_RELOAD || gtOper == GT_COPY)
if (OperIs(GT_RELOAD, GT_COPY))
{
assert(gtGetOp1()->OperGet() != GT_RELOAD && gtGetOp1()->OperGet() != GT_COPY);
assert(!gtGetOp1()->OperIs(GT_RELOAD, GT_COPY));
return gtGetOp1();
}
return this;
Expand Down
21 changes: 21 additions & 0 deletions src/coreclr/jit/stacklevelsetter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,14 @@ PhaseStatus StackLevelSetter::DoPhase()
madeChanges = true;
}
}
else
{
// Mark all the throw helpers as used to avoid asserts later.
for (Compiler::AddCodeDsc* add = comp->fgGetAdditionalCodeDescriptors(); add != nullptr; add = add->acdNext)
{
add->acdUsed = true;
}
}

// The above loop might have moved a BBJ_COND's true target to its next block.
// In such cases, reverse the condition so we can remove a branch.
Expand Down Expand Up @@ -190,6 +198,7 @@ void StackLevelSetter::ProcessBlock(BasicBlock* block)
// Arguments:
// node - the node to process;
// block - the source block for the node.
//
void StackLevelSetter::SetThrowHelperBlocks(GenTree* node, BasicBlock* block)
{
assert(node->OperMayThrow(comp));
Expand Down Expand Up @@ -227,11 +236,21 @@ void StackLevelSetter::SetThrowHelperBlocks(GenTree* node, BasicBlock* block)
{
SetThrowHelperBlock(SCK_DIV_BY_ZERO, block);
}
else
{
// Even if we thought it might divide by zero during morph, now we know it never will.
node->gtFlags |= GTF_DIV_MOD_NO_BY_ZERO;
}

if ((exSetFlags & ExceptionSetFlags::ArithmeticException) != ExceptionSetFlags::None)
{
SetThrowHelperBlock(SCK_ARITH_EXCPN, block);
}
else
{
// Even if we thought it might overflow during morph, now we know it never will.
node->gtFlags |= GTF_DIV_MOD_NO_OVERFLOW;
}
}
break;
#endif
Expand All @@ -256,10 +275,12 @@ void StackLevelSetter::SetThrowHelperBlocks(GenTree* node, BasicBlock* block)
// Arguments:
// kind - the special throw-helper kind;
// block - the source block that targets helper.
//
void StackLevelSetter::SetThrowHelperBlock(SpecialCodeKind kind, BasicBlock* block)
{
Compiler::AddCodeDsc* add = comp->fgFindExcptnTarget(kind, comp->bbThrowIndex(block));
assert(add != nullptr);

// We expect we'll actually need this helper.
add->acdUsed = true;

Expand Down
Loading