Skip to content

Commit

Permalink
JIT: remove fgAddCodeList (#108527)
Browse files Browse the repository at this point in the history 
AddCodeDscs are currently kept in both a linked list and in a hash map.
Since the order of these descriptors is not important, remove the list
and use map iterators to enumerate them.

This makes it easier to remove ACDs.
  • Loading branch information
@AndyAyersMS
AndyAyersMS authored Oct 3, 2024
1 parent 48355da commit b2c8aa2
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 62 deletions.
11 changes: 7 additions & 4 deletions src/coreclr/jit/codegencommon.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -527,12 +527,15 @@ 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 != nullptr; add = add->acdNext)
if (compiler->fgHasAddCodeDscMap())
{
if (add->acdUsed)
for (Compiler::AddCodeDsc* const add : Compiler::AddCodeDscMap::ValueIteration(compiler->fgGetAddCodeDscMap()))
{
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);
}
}
}

Expand Down
14 changes: 2 additions & 12 deletions src/coreclr/jit/compiler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6797,8 +6797,6 @@ class Compiler

struct AddCodeDsc
{
AddCodeDsc* acdNext;

// After fgCreateThrowHelperBlocks, the block to which
// we jump to raise the exception.
BasicBlock* acdDstBlk;
Expand Down Expand Up @@ -6855,30 +6853,22 @@ class Compiler
};

typedef JitHashTable<AddCodeDscKey, AddCodeDscKey, AddCodeDsc*> AddCodeDscMap;

AddCodeDscMap* fgGetAddCodeDscMap();

private:
static unsigned acdHelper(SpecialCodeKind codeKind);

AddCodeDsc* fgAddCodeList = nullptr;
bool fgRngChkThrowAdded = false;
AddCodeDscMap* fgAddCodeDscMap = nullptr;

void fgAddCodeRef(BasicBlock* srcBlk, SpecialCodeKind kind);
PhaseStatus fgCreateThrowHelperBlocks();


public:
AddCodeDsc* fgFindExcptnTarget(SpecialCodeKind kind, BasicBlock* fromBlock);

bool fgHasAddCodeDscMap() const { return fgAddCodeDscMap != nullptr; }
AddCodeDsc* fgFindExcptnTarget(SpecialCodeKind kind, BasicBlock* fromBlock);
bool fgUseThrowHelperBlocks();

AddCodeDsc* fgGetAdditionalCodeDescriptors()
{
return fgAddCodeList;
}

void fgCreateThrowHelperBlockCode(AddCodeDsc* add);

private:
Expand Down
20 changes: 12 additions & 8 deletions src/coreclr/jit/compiler.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3229,19 +3229,23 @@ inline bool Compiler::fgIsThrowHlpBlk(BasicBlock* block)
}

// We can get to this point for blocks that we didn't create as throw helper blocks
// under stress, with implausible flow graph optimizations. So, walk the fgAddCodeList
// under stress, with implausible flow graph optimizations. So, walk the fgAddCodeDscMap
// for the final determination.

for (AddCodeDsc* add = fgAddCodeList; add != nullptr; add = add->acdNext)
if (fgHasAddCodeDscMap())
{
if (block == add->acdDstBlk)
for (AddCodeDsc* const add : AddCodeDscMap::ValueIteration(fgGetAddCodeDscMap()))
{
return add->acdKind == SCK_RNGCHK_FAIL || add->acdKind == SCK_DIV_BY_ZERO || add->acdKind == SCK_OVERFLOW ||
add->acdKind == SCK_ARG_EXCPN || add->acdKind == SCK_ARG_RNG_EXCPN || add->acdKind == SCK_FAIL_FAST;
if (block == add->acdDstBlk)
{
return add->acdKind == SCK_RNGCHK_FAIL || add->acdKind == SCK_DIV_BY_ZERO ||
add->acdKind == SCK_OVERFLOW || add->acdKind == SCK_ARG_EXCPN ||
add->acdKind == SCK_ARG_RNG_EXCPN || add->acdKind == SCK_FAIL_FAST;
}
}
}

// We couldn't find it in the fgAddCodeList
// We couldn't find it in the fgAddCodeDscMap
return false;
}

Expand All @@ -3255,7 +3259,7 @@ inline bool Compiler::fgIsThrowHlpBlk(BasicBlock* block)

inline unsigned Compiler::fgThrowHlpBlkStkLevel(BasicBlock* block)
{
for (AddCodeDsc* add = fgAddCodeList; add != nullptr; add = add->acdNext)
for (AddCodeDsc* const add : AddCodeDscMap::ValueIteration(fgGetAddCodeDscMap()))
{
if (block == add->acdDstBlk)
{
Expand All @@ -3273,7 +3277,7 @@ inline unsigned Compiler::fgThrowHlpBlkStkLevel(BasicBlock* block)
}

noway_assert(!"fgThrowHlpBlkStkLevel should only be called if fgIsThrowHlpBlk() is true, but we can't find the "
"block in the fgAddCodeList list");
"block in the fgAddCodeDscMap");

/* We couldn't find the basic block: it must not have been a throw helper block */

Expand Down
9 changes: 6 additions & 3 deletions src/coreclr/jit/fgdiagnostic.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3182,11 +3182,14 @@ void Compiler::fgDebugCheckBBlist(bool checkBBNum /* = false */, bool checkBBRef
}

// Ensure that all throw helper blocks are currently in the block list.
for (Compiler::AddCodeDsc* add = fgAddCodeList; add != nullptr; add = add->acdNext)
if (fgHasAddCodeDscMap())
{
if (add->acdUsed)
for (Compiler::AddCodeDsc* const add : Compiler::AddCodeDscMap::ValueIteration(fgAddCodeDscMap))
{
assert(add->acdDstBlk->bbTraversalStamp == curTraversalStamp);
if (add->acdUsed)
{
assert(add->acdDstBlk->bbTraversalStamp == curTraversalStamp);
}
}
}

Expand Down
7 changes: 2 additions & 5 deletions src/coreclr/jit/flowgraph.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3405,7 +3405,6 @@ void Compiler::fgAddCodeRef(BasicBlock* srcBlk, SpecialCodeKind kind)
// Allocate a new entry and prepend it to the list
//
add = new (this, CMK_Unknown) AddCodeDsc;
add->acdNext = fgAddCodeList;
add->acdDstBlk = nullptr;
add->acdTryIndex = srcBlk->bbTryIndex;
add->acdHndIndex = srcBlk->bbHndIndex;
Expand All @@ -3422,8 +3421,6 @@ void Compiler::fgAddCodeRef(BasicBlock* srcBlk, SpecialCodeKind kind)
#endif // !FEATURE_FIXED_OUT_ARGS
INDEBUG(add->acdNum = acdCount++);

fgAddCodeList = add;

// Add to map
//
AddCodeDscMap* const map = fgGetAddCodeDscMap();
Expand Down Expand Up @@ -3451,7 +3448,7 @@ void Compiler::fgAddCodeRef(BasicBlock* srcBlk, SpecialCodeKind kind)
//
PhaseStatus Compiler::fgCreateThrowHelperBlocks()
{
if (fgAddCodeList == nullptr)
if (fgAddCodeDscMap == nullptr)
{
return PhaseStatus::MODIFIED_NOTHING;
}
Expand All @@ -3472,7 +3469,7 @@ PhaseStatus Compiler::fgCreateThrowHelperBlocks()

noway_assert(sizeof(jumpKinds) == SCK_COUNT); // sanity check

for (AddCodeDsc* add = fgAddCodeList; add != nullptr; add = add->acdNext)
for (AddCodeDsc* const add : AddCodeDscMap::ValueIteration(fgAddCodeDscMap))
{
// Create the target basic block in the region indicated by the acd info
//
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/jiteh.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1371,7 +1371,7 @@ void Compiler::fgRemoveEHTableEntry(unsigned XTnum)
{
assert(compHndBBtabCount > 0);
assert(XTnum < compHndBBtabCount);
assert(fgAddCodeList == nullptr);
assert((fgAddCodeDscMap == nullptr) || (fgAddCodeDscMap->GetCount() == 0));

EHblkDsc* HBtab;

Expand Down
61 changes: 32 additions & 29 deletions src/coreclr/jit/stacklevelsetter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,41 +64,44 @@ PhaseStatus StackLevelSetter::DoPhase()
//
bool madeChanges = false;

if (comp->opts.OptimizationEnabled())
if (comp->fgHasAddCodeDscMap())
{
comp->compUsesThrowHelper = false;
for (Compiler::AddCodeDsc* add = comp->fgGetAdditionalCodeDescriptors(); add != nullptr; add = add->acdNext)
if (comp->opts.OptimizationEnabled())
{
if (add->acdUsed)
comp->compUsesThrowHelper = false;
for (Compiler::AddCodeDsc* const add : Compiler::AddCodeDscMap::ValueIteration(comp->fgGetAddCodeDscMap()))
{
// Create the helper call
//
comp->fgCreateThrowHelperBlockCode(add);
comp->compUsesThrowHelper = true;
}
else
{
// Remove the helper call block
//
BasicBlock* const block = add->acdDstBlk;
assert(block->isEmpty());
JITDUMP("Throw help block " FMT_BB " is unused\n", block->bbNum);
block->RemoveFlags(BBF_DONT_REMOVE);
comp->fgRemoveBlock(block, /* unreachable */ true);
if (add->acdUsed)
{
// Create the helper call
//
comp->fgCreateThrowHelperBlockCode(add);
comp->compUsesThrowHelper = true;
}
else
{
// Remove the helper call block
//
BasicBlock* const block = add->acdDstBlk;
assert(block->isEmpty());
JITDUMP("Throw help block " FMT_BB " is unused\n", block->bbNum);
block->RemoveFlags(BBF_DONT_REMOVE);
comp->fgRemoveBlock(block, /* unreachable */ true);
}

madeChanges = true;
}

madeChanges = true;
}
}
else
{
// Assume all helpers used. Fill in all helper block code.
//
for (Compiler::AddCodeDsc* add = comp->fgGetAdditionalCodeDescriptors(); add != nullptr; add = add->acdNext)
else
{
add->acdUsed = true;
comp->fgCreateThrowHelperBlockCode(add);
madeChanges = true;
// Assume all helpers used. Fill in all helper block code.
//
for (Compiler::AddCodeDsc* const add : Compiler::AddCodeDscMap::ValueIteration(comp->fgGetAddCodeDscMap()))
{
add->acdUsed = true;
comp->fgCreateThrowHelperBlockCode(add);
madeChanges = true;
}
}
}

Expand Down

0 comments on commit b2c8aa2

Please sign in to comment.