Skip to content

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

Merged
tannergooding merged 4 commits into
dotnet:mainfrom
tannergooding:simplify-visit
May 9, 2026
Merged

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

Conversation

@tannergooding

@tannergooding tannergooding commented May 8, 2026

Copy link
Copy Markdown
Member

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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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
@tannergooding
tannergooding merged commit e01b19d into dotnet:main May 9, 2026
140 checks passed
@tannergooding
tannergooding deleted the simplify-visit branch May 9, 2026 15:17
jakobbotsch pushed a commit to jakobbotsch/runtime that referenced this pull request May 12, 2026
…her than be a big switch (dotnet#127972)

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.
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 9, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

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.

3 participants