Uh oh!
There was an error while loading. Please reload this page.
JIT: replace fgWalkTree callbacks with GenTreeVisitor implementations - #132469
Merged
Conversation
The fgWalkTreePre/fgWalkTreePost/fgWalkTree family dispatched every node visit through a function pointer stored in fgWalkData. That indirect call could not be inlined into GenTreeVisitor::WalkTree and, on Windows, went through a Control Flow Guard dispatch stub per node. Convert the remaining users to direct GenTreeVisitor implementations and delete fgWalkTreePre, fgWalkTreePost, fgWalkTree, fgWalkAllTreesPre, GenericTreeWalker, fgWalkData and the fgWalkPreFn/fgWalkPostFn typedefs. Several parameters turned out to be already dead and are gone with them: GenericTreeWalker hardcoded ComputeStack to false and nothing read fgWalkData::parentStack, so computeStack had no effect; lclVarsOnly had no remaining users; and fgWalkData::printModified was never set. The unused fgSplitPredicate typedef is removed as well. gtHasCatchArg is reimplemented on the existing gtFindNodeInTree helper. Descending only into subtrees with GTF_ORDER_SIDEEFF preserves its previous root-level early-out and additionally prunes at every level; GT_CATCH_ARG sets that flag on itself and effect flags propagate to parents, so a subtree without it cannot contain one. SuperPMI asmdiffs over all 11 windows-x64 collections show no diffs. Throughput on benchmarks.run, measured as instructions retired: -1.94% for the checked JIT and -0.33% for release built with NoPgoOptimize. A default release build instead shows +0.19%, because the checked-in PGO profile is trained on the old code shape. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 064035bf-fb1f-450e-afd2-08a7700aa736
|
Azure Pipelines: Successfully started running 5 pipeline(s). 11 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors CoreCLR JIT IR tree walking by replacing the legacy fgWalkTreePre/fgWalkTreePost/fgWalkTree* function-pointer callback mechanism with direct GenTreeVisitor implementations, and removes the associated callback/walker infrastructure. It also reworks a few helpers that previously used fgWalkTree* (e.g., gtHasCatchArg) to use existing visitor/find-node helpers.
Changes:
- Convert remaining
fgWalkTree*users to dedicatedGenTreeVisitortypes across multiple JIT passes (morph, loop cloning, assertion prop, diagnostics, etc.). - Delete the
fgWalkTree*APIs and supporting types (fgWalkData,GenericTreeWalker, callback typedefs) fromcompiler.h/compiler.hpp. - Reimplement
gtHasCatchArgviagtFindNodeInTreewith pruning based onGTF_ORDER_SIDEEFF.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/jit/rangecheckcloning.cpp | Replace debug-only bounds-check search walk with a small GenTreeVisitor. |
| src/coreclr/jit/morph.cpp | Replace colon-conditional marking walk with a dedicated visitor. |
| src/coreclr/jit/loopcloning.cpp | Replace loop cloning profitability/tree scanning callback with a visitor. |
| src/coreclr/jit/lclvars.cpp | Replace lcl-fld stress walker callback with a visitor and a member helper. |
| src/coreclr/jit/indirectcalltransformer.cpp | Replace debug validation callback with a visitor. |
| src/coreclr/jit/importercalls.cpp | Replace ret_expr spill callback with a GenTreeVisitor helper. |
| src/coreclr/jit/gentree.cpp | Replace colon-cond clearing walk; reimplement gtHasCatchArg via gtFindNodeInTree. |
| src/coreclr/jit/fgopt.cpp | Replace node-counting callback with a visitor. |
| src/coreclr/jit/fginline.cpp | Replace debug inline-candidate validation walks with visitors (adds a friend for one case). |
| src/coreclr/jit/fgdiagnostic.cpp | Replace debug stress-mul and uniqueness-check callbacks with visitors. |
| src/coreclr/jit/compiler.hpp | Remove inline implementations of fgWalkTreePre/Post/Tree and related helpers. |
| src/coreclr/jit/compiler.h | Remove declarations/types for the deleted walkers; add Mark/ClearColonCondVisitor in-header visitors. |
Uh oh!
There was an error while loading. Please reload this page.
jakobbotsch
marked this pull request as ready for review
August 19, 2026 08:52
jakobbotsch
commented
Aug 19, 2026
MemberAuthor
|
Azure Pipelines: Successfully started running 5 pipeline(s). 11 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
EgorBo
approved these changes
Aug 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The fgWalkTreePre/fgWalkTreePost/fgWalkTree family dispatched every node visit through a function pointer stored in fgWalkData. That indirect call could not be inlined into GenTreeVisitor::WalkTree and, on Windows, went through a Control Flow Guard dispatch stub per node.
Convert the remaining users to direct GenTreeVisitor implementations and delete fgWalkTreePre, fgWalkTreePost, fgWalkTree, fgWalkAllTreesPre, GenericTreeWalker, fgWalkData and the fgWalkPreFn/fgWalkPostFn typedefs.
Several parameters turned out to be already dead and are gone with them: GenericTreeWalker hardcoded ComputeStack to false and nothing read fgWalkData::parentStack, so computeStack had no effect; lclVarsOnly had no remaining users; and fgWalkData::printModified was never set. The unused fgSplitPredicate typedef is removed as well.
gtHasCatchArg is reimplemented on the existing gtFindNodeInTree helper. Descending only into subtrees with GTF_ORDER_SIDEEFF preserves its previous root-level early-out and additionally prunes at every level; GT_CATCH_ARG sets that flag on itself and effect flags propagate to parents, so a subtree without it cannot contain one.
SuperPMI asmdiffs over all 11 windows-x64 collections show no diffs. Throughput on benchmarks.run, measured as instructions retired: -1.94% for the checked JIT and -0.33% for release built with NoPgoOptimize.