Skip to content

Cleanup VisitOperandUses and GenTreeVisitor to check common kinds rather than be a big switch#127972

Open
tannergooding wants to merge 4 commits intodotnet:mainfrom
tannergooding:simplify-visit
Open

Cleanup VisitOperandUses and GenTreeVisitor to check common kinds rather than be a big switch#127972
tannergooding wants to merge 4 commits intodotnet:mainfrom
tannergooding:simplify-visit

Conversation

@tannergooding
Copy link
Copy Markdown
Member

@tannergooding tannergooding commented May 8, 2026

This should notably reduce risk as well. Some handling had been missed before like GT_LZCNT not being handled and hitting the default binary path instead.

TP impact on Linux x64

linux x64
* Overall (-0.09% to -0.04%)
* MinOpts (-0.10% to +0.04%)
* FullOpts (-0.11% to -0.04%)

TP impact on Windows x64

linux arm64
* Overall (-0.21% to -0.15%)
* MinOpts (-0.14% to -0.04%)
* FullOpts (-0.26% to -0.16%)

linux x64
* Overall (-0.23% to -0.15%)
* MinOpts (-0.16% to -0.02%)
* FullOpts (-0.28% to -0.15%)

osx arm64
* Overall (-0.21% to -0.16%)
* MinOpts (-0.13% to -0.06%)
* FullOpts (-0.22% to -0.16%)

windows arm64
* Overall (-0.21% to -0.15%)
* MinOpts (-0.14% to -0.04%)
* FullOpts (-0.27% to -0.16%)

windows x64
* Overall (-0.23% to -0.15%)
* MinOpts (-0.16% to -0.02%)
* FullOpts (-0.29% to -0.15%)

Windows x86 has a TP hit, from -0.00% to +0.06% for MinOpts, but that should be fine given the wins we see elsewhere and the improved robustness for the visitors when new node kinds are introduced.

Copilot AI review requested due to automatic review settings May 8, 2026 21:24
@github-actions github-actions Bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label May 8, 2026
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors GenTree::VisitOperandUses to use OperIsLeaf / OperIsBinary / OperIsUnary fast-path checks instead of an explicit per-op switch, leaving only GTK_SPECIAL nodes handled in a smaller switch.

Changes:

  • Replace the large OperGet() switch with kind-based checks (leaf/binary/unary/special) in VisitOperandUses.
  • Centralize operand walking for most nodes via AsOp() / AsUnOp() handling.
  • Keep explicit operand iteration for special-structure nodes (e.g., call args, phi uses, field list, cmpxchg, arr elem).
Comments suppressed due to low confidence (1)

src/coreclr/jit/compiler.hpp:4438

  • OperIsSpecial() includes ARM64 conditional nodes GT_SELECT_INC, GT_SELECT_INV, and GT_SELECT_NEG (see gtlist.h), but the OperGet() switch here only handles GT_SELECT. As a result, VisitOperandUses will hit the default case for these nodes and skip visiting their operands (and GT_SELECT_* can have an optional gtOp2 that still needs visiting when present). Please add explicit cases for GT_SELECT_INC/INV/NEG (and handle their optional gtOp2), so operand walking remains correct on ARM64.
    assert(OperIsSpecial());

    switch (OperGet())
    {
#if defined(FEATURE_HW_INTRINSICS)
        case GT_HWINTRINSIC:
            for (GenTree** use : this->AsMultiOp()->UseEdges())
            {

Comment thread src/coreclr/jit/compiler.hpp
Comment thread src/coreclr/jit/compiler.hpp
Copilot AI review requested due to automatic review settings May 8, 2026 23:17
@tannergooding tannergooding changed the title Cleanup VisitOperandUses to check common kinds rather than be a big switch Cleanup VisitOperandUses and GenTreeVisitor to check common kinds rather than be a big switch May 8, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comment thread src/coreclr/jit/compiler.hpp
Comment thread src/coreclr/jit/compiler.h
@tannergooding tannergooding marked this pull request as ready for review May 9, 2026 00:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants