Skip to content

Replace explicit GTF_UNSIGNED flag operations with GenTree helper methods#123265

Merged
EgorBo merged 10 commits intomainfrom
copilot/cleanup-gtf-unsigned-usage
Jan 24, 2026
Merged

Replace explicit GTF_UNSIGNED flag operations with GenTree helper methods#123265
EgorBo merged 10 commits intomainfrom
copilot/cleanup-gtf-unsigned-usage

Conversation

Copy link
Contributor

Copilot AI commented Jan 16, 2026

Description

Refactored JIT codebase to use existing GenTree helper methods (IsUnsigned(), SetUnsigned(), ClearUnsigned()) instead of direct GTF_UNSIGNED flag manipulation.

Changes

Replaced 55 explicit flag operations across 22 files:

  • gtFlags |= GTF_UNSIGNEDSetUnsigned() (13 occurrences)
  • gtFlags &= ~GTF_UNSIGNEDClearUnsigned() (3 occurrences)
  • (gtFlags & GTF_UNSIGNED)IsUnsigned() (39 occurrences)

Moved SetUnsigned/ClearUnsigned to gentree.cpp:

  • Implementations moved from inline methods in gentree.h to gentree.cpp
  • gentree.h now contains only method declarations for cleaner separation
  • Added debug assertions that print the node operator type on failure for easier debugging
  • Assertions validate that GTF_UNSIGNED is only set on appropriate node types:
    • Arithmetic operations: GT_ADD, GT_SUB
    • Casts: GT_CAST
    • All comparison operators (using OperIsCompare() helper): GT_EQ, GT_NE, GT_LE, GT_LT, GT_GT, GT_GE, GT_TEST_EQ, GT_TEST_NE, and on XARCH also GT_BITTEST_EQ, GT_BITTEST_NE
    • Multiply operations (via OperIsMul() helper): GT_MUL, GT_MULHI, GT_MUL_LONG

Testing

  • CoreCLR builds successfully
  • Enhanced assertions provide helpful diagnostic output during development
  • No functional changes
Original prompt

A small clenaup task in JIT codebase around GTF_UNSIGNED

in src/coreclr/jit/* replace explicit usages of GTF_UNSIGNED over tree objects (e.g. tmp->gtFlags |= GTF_UNSIGNED;) with the following methods on GenTree:

bool GenTree::IsUnsigned() const
void GenTree::SetUnsigned()
void GenTree::ClearUnsigned()

Make sure code compiles and runtime tests pass.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI changed the title [WIP] Refactor GTF_UNSIGNED usage in JIT codebase Replace explicit GTF_UNSIGNED flag operations with GenTree helper methods Jan 16, 2026
Copilot AI requested a review from EgorBo January 16, 2026 18:45
@EgorBo
Copy link
Member

EgorBo commented Jan 16, 2026

@dotnet/jit-contrib I propose we slowly step-by-step clean up raw flags operations and field accesses with help of Copilot like this. Makes code cleaner and more debugger friendly (e.g. to figure out who exactly clears the unsigned flag we just place a breakpoint in SetUnsigned). No diffs

PS: PTAL

@EgorBo EgorBo requested a review from a team January 16, 2026 20:46
@EgorBo EgorBo marked this pull request as ready for review January 16, 2026 20:46
Copilot AI review requested due to automatic review settings January 16, 2026 20:46
Copy link
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 the JIT codebase to use GenTree helper methods (IsUnsigned(), SetUnsigned(), ClearUnsigned()) instead of direct GTF_UNSIGNED flag manipulation. The refactoring improves code maintainability and readability by encapsulating flag operations behind a consistent API.

Changes:

  • Replaced 55 explicit GTF_UNSIGNED flag operations across 22 files with helper method calls
  • Removed restrictive assertions from SetUnsigned() and ClearUnsigned() methods to match the flag's actual scope of applicability
  • No functional changes - purely a code quality improvement

Reviewed changes

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

Show a summary per file
File Description
src/coreclr/jit/gentree.h Removed assertions from SetUnsigned() and ClearUnsigned() to allow usage on any operator
src/coreclr/jit/gentree.cpp Replaced flag check with IsUnsigned() in Compare() and gtDispTree()
src/coreclr/jit/valuenum.cpp Replaced flag check with IsUnsigned() for cast value numbering
src/coreclr/jit/optimizer.cpp Replaced flag check with IsUnsigned() in loop unrolling
src/coreclr/jit/morph.cpp Replaced flag operations with ClearUnsigned() in relational comparison optimizations
src/coreclr/jit/lower.cpp Replaced flag set with SetUnsigned() for switch default case, comparisons, and unsigned div/mod
src/coreclr/jit/loopcloning.cpp Replaced flag set with SetUnsigned() in loop condition generation
src/coreclr/jit/importercalls.cpp Replaced flag set with SetUnsigned() for SRCS unsafe intrinsic comparisons
src/coreclr/jit/importer.cpp Replaced flag set with SetUnsigned() for overflow and comparison operations
src/coreclr/jit/lowerxarch.cpp Replaced flag check with IsUnsigned() in cast containment
src/coreclr/jit/lsrariscv64.cpp Replaced flag checks with IsUnsigned() in register allocation
src/coreclr/jit/decomposelongs.cpp Replaced flag check and set with IsUnsigned() and SetUnsigned() in long decomposition
src/coreclr/jit/compiler.cpp Replaced flag check with IsUnsigned() in debug tree flag display
src/coreclr/jit/codegenxarch.cpp Replaced flag checks with IsUnsigned() for multiply and cast operations
src/coreclr/jit/codegenriscv64.cpp Replaced flag checks with IsUnsigned() in multiply, cast, and comparison codegen
src/coreclr/jit/codegenloongarch64.cpp Replaced flag checks with IsUnsigned() in multiply, cast, and comparison codegen
src/coreclr/jit/codegencommon.cpp Replaced flag check with IsUnsigned() in overflow checking
src/coreclr/jit/codegenarm64.cpp Replaced flag checks with IsUnsigned() in multiply and cast codegen
src/coreclr/jit/codegenarm.cpp Replaced flag checks with IsUnsigned() in cast codegen
src/coreclr/jit/emitriscv64.cpp Replaced flag checks with IsUnsigned() in ternary instruction emission
src/coreclr/jit/emitloongarch64.cpp Replaced flag checks with IsUnsigned() in ternary instruction emission
src/coreclr/jit/emitarm64.cpp Replaced flag check with IsUnsigned() in ternary instruction emission
src/coreclr/jit/emitarm.cpp Replaced flag checks with IsUnsigned() in ternary instruction emission

Copy link
Contributor

@adamperlin adamperlin left a comment

Choose a reason for hiding this comment

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

This looks good to me, though someone else in @dotnet/jit-contrib should definitely take a look. @EgorBo what do you think?

@EgorBo
Copy link
Member

EgorBo commented Jan 22, 2026

@copilot move void SetUnsigned() from gentree.h to .cpp and change the assert inside it to print what kind of node it is being called on if the assert fails.

Copy link
Contributor Author

Copilot AI commented Jan 22, 2026

@copilot move void SetUnsigned() from gentree.h to .cpp and change the assert inside it to print what kind of node it is being called on if the assert fails.

Done in 6331512. Moved SetUnsigned() and ClearUnsigned() to gentree.cpp with assertions that print the node operator type before aborting. Also updated the assertion to use OperIsCompare() to cover all comparison node types (including TEST_EQ, TEST_NE, BITTEST_EQ, BITTEST_NE).

@EgorBo EgorBo enabled auto-merge (squash) January 24, 2026 21:23
@EgorBo EgorBo merged commit b19edc0 into main Jan 24, 2026
133 checks passed
@EgorBo EgorBo deleted the copilot/cleanup-gtf-unsigned-usage branch January 24, 2026 21:24
@EgorBo EgorBo added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jan 27, 2026
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.

5 participants