JIT: Add pred block iterator that tolerates pred list modifications #99466
Conversation
ghost
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
AndyAyersMS
left a comment
AndyAyersMS
left a comment
There was a problem hiding this comment.
I like this direction, but would prefer that the nature of the iterator be obvious from its name..
Maybe something like:
class PredBlockList
{
public:
static PredBlockList PredBlocks() const
{
return PredBlockList(bbPreds, /*allowEdits*/ false);
}
static PredBlockList PredBlocksEditing() const
{
return PredBlockList(bbPreds, /*allowEdits*/ true);
}
private:
PredBlockList(FlowEdge* list, bool allowEdits) {...};
};
src/coreclr/jit/block.h
Outdated
| // If allowEdits=true, the user can modify the predecessor list while traversing it, | ||
| // so (next == m_next) may not be true. Since we cached the next pointer in m_next, this won't break iteration. | ||
| assert((next == m_next) || allowEdits); | ||
| updateNextPointer = true; |
There was a problem hiding this comment.
This seems to be modifying live state under debug, which I think we should avoid.
|
@AndyAyersMS I separated out the |
|
Thanks for the reviews! I should note that this change has small diffs from slight changes in edge weights. |
2 participants
Follow-up to #99362.
fgRedirectTargetEdgemodifies the predecessor lists of the old and new successor blocks, so if we want to be able to use it infgReplaceJumpTarget, we need a pred block iterator that is resilient to these changes, as we frequently call the latter method while iterating predecessors.I haven't found the need to update the pred edge iterator to allow updates too, but I imagine it would look the same as the pred block iterator.
fgTailMergeThrowslooks a bit awkward right now. After this change is merged, I plan to add a new method similar tofgRedirectTargetEdgeforBBJ_CONDblocks, and leverage it infgReplaceJumpTarget. Since that new method will preserve edge weights, we won't lose anything infgTailMergeThrowsby just callingfgReplaceJumpTarget. I think that will be a good opportunity to clean upfgTailMergeThrowsby getting rid of all its helpers for updating edges.cc @dotnet/jit-contrib, @AndyAyersMS PTAL.