Skip to content

Commit b19edc0

Browse files
CopilotEgorBo
Copilot
and
EgorBo
authored
Replace explicit GTF_UNSIGNED flag operations with GenTree helper methods (#123265)
## 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_UNSIGNED` → `SetUnsigned()` (13 occurrences) - `gtFlags &= ~GTF_UNSIGNED` → `ClearUnsigned()` (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 <!-- START COPILOT CODING AGENT SUFFIX --> <!-- START COPILOT ORIGINAL PROMPT --> <details> <summary>Original prompt</summary> > # 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. </details> <!-- START COPILOT CODING AGENT TIPS --> --- ✨ Let Copilot coding agent [set things up for you](https://github.com/dotnet/runtime/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot) — coding agent works faster and does higher quality work when set up for your repo. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com> Co-authored-by: Egor Bogatov <egorbo@gmail.com>
1 parent 8fafcb3 commit b19edc0

23 files changed

Lines changed: 57 additions & 53 deletions

‎src/coreclr/jit/codegenarm.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1429,7 +1429,7 @@ void CodeGen::genLongToIntCast(GenTree* cast)
14291429

14301430
genConsumeRegs(src);
14311431

1432-
var_types srcType = ((cast->gtFlags & GTF_UNSIGNED) != 0) ? TYP_ULONG : TYP_LONG;
1432+
var_types srcType = cast->IsUnsigned() ? TYP_ULONG : TYP_LONG;
14331433
var_types dstType = cast->CastToType();
14341434
regNumber loSrcReg = src->gtGetOp1()->GetRegNum();
14351435
regNumber hiSrcReg = src->gtGetOp2()->GetRegNum();
@@ -1520,7 +1520,7 @@ void CodeGen::genIntToFloatCast(GenTree* treeNode)
15201520
assert(!varTypeIsFloating(srcType) && varTypeIsFloating(dstType));
15211521

15221522
// force the srcType to unsigned if GT_UNSIGNED flag is set
1523-
if (treeNode->gtFlags & GTF_UNSIGNED)
1523+
if (treeNode->IsUnsigned())
15241524
{
15251525
srcType = varTypeToUnsigned(srcType);
15261526
}

‎src/coreclr/jit/codegenarm64.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2431,7 +2431,7 @@ void CodeGen::genCodeForMulHi(GenTreeOp* treeNode)
24312431
var_types targetType = treeNode->TypeGet();
24322432
emitter* emit = GetEmitter();
24332433
emitAttr attr = emitActualTypeSize(treeNode);
2434-
unsigned isUnsigned = (treeNode->gtFlags & GTF_UNSIGNED);
2434+
unsigned isUnsigned = treeNode->IsUnsigned();
24352435

24362436
GenTree* op1 = treeNode->gtGetOp1();
24372437
GenTree* op2 = treeNode->gtGetOp2();
@@ -4387,7 +4387,7 @@ void CodeGen::genIntToFloatCast(GenTree* treeNode)
43874387
assert(!varTypeIsFloating(srcType) && varTypeIsFloating(dstType));
43884388

43894389
// force the srcType to unsigned if GT_UNSIGNED flag is set
4390-
if (treeNode->gtFlags & GTF_UNSIGNED)
4390+
if (treeNode->IsUnsigned())
43914391
{
43924392
srcType = varTypeToUnsigned(srcType);
43934393
}

‎src/coreclr/jit/codegencommon.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1730,7 +1730,7 @@ void CodeGen::genCheckOverflow(GenTree* tree)
17301730
else
17311731
#endif
17321732
{
1733-
bool isUnsignedOverflow = ((tree->gtFlags & GTF_UNSIGNED) != 0);
1733+
bool isUnsignedOverflow = tree->IsUnsigned();
17341734

17351735
#if defined(TARGET_XARCH)
17361736

‎src/coreclr/jit/codegenloongarch64.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1065,7 +1065,7 @@ void CodeGen::genCodeForMulHi(GenTreeOp* treeNode)
10651065
var_types targetType = treeNode->TypeGet();
10661066
emitter* emit = GetEmitter();
10671067
emitAttr attr = emitActualTypeSize(treeNode);
1068-
unsigned isUnsigned = (treeNode->gtFlags & GTF_UNSIGNED);
1068+
unsigned isUnsigned = treeNode->IsUnsigned();
10691069

10701070
GenTree* op1 = treeNode->gtGetOp1();
10711071
GenTree* op2 = treeNode->gtGetOp2();
@@ -2874,7 +2874,7 @@ void CodeGen::genIntToFloatCast(GenTree* treeNode)
28742874
emitAttr srcSize = EA_ATTR(genTypeSize(srcType));
28752875
noway_assert((srcSize == EA_4BYTE) || (srcSize == EA_8BYTE));
28762876

2877-
bool IsUnsigned = treeNode->gtFlags & GTF_UNSIGNED;
2877+
bool IsUnsigned = treeNode->IsUnsigned();
28782878
instruction ins = INS_invalid;
28792879

28802880
genConsumeOperands(treeNode->AsOp());
@@ -3270,7 +3270,7 @@ void CodeGen::genCodeForCompare(GenTreeOp* tree)
32703270
assert(!op1->isContainedIntOrIImmed());
32713271
assert(tree->OperIs(GT_LT, GT_LE, GT_EQ, GT_NE, GT_GT, GT_GE));
32723272

3273-
bool IsUnsigned = (tree->gtFlags & GTF_UNSIGNED) != 0;
3273+
bool IsUnsigned = tree->IsUnsigned();
32743274
regNumber regOp1 = op1->GetRegNum();
32753275

32763276
if (op2->isContainedIntOrIImmed())

‎src/coreclr/jit/codegenriscv64.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1053,7 +1053,7 @@ void CodeGen::genCodeForMulHi(GenTreeOp* treeNode)
10531053
var_types targetType = treeNode->TypeGet();
10541054
emitter* emit = GetEmitter();
10551055
emitAttr attr = emitActualTypeSize(treeNode);
1056-
unsigned isUnsigned = (treeNode->gtFlags & GTF_UNSIGNED);
1056+
unsigned isUnsigned = treeNode->IsUnsigned();
10571057

10581058
GenTree* op1 = treeNode->gtGetOp1();
10591059
GenTree* op2 = treeNode->gtGetOp2();
@@ -2941,7 +2941,7 @@ void CodeGen::genIntToFloatCast(GenTree* treeNode)
29412941
emitAttr srcSize = EA_ATTR(genTypeSize(srcType));
29422942
noway_assert((srcSize == EA_4BYTE) || (srcSize == EA_8BYTE));
29432943

2944-
bool isUnsigned = treeNode->gtFlags & GTF_UNSIGNED;
2944+
bool isUnsigned = treeNode->IsUnsigned();
29452945
instruction ins = INS_invalid;
29462946

29472947
if (isUnsigned)

‎src/coreclr/jit/codegenxarch.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1125,7 +1125,7 @@ void CodeGen::genCodeForMul(GenTreeOp* treeNode)
11251125

11261126
instruction ins;
11271127
emitAttr size = emitTypeSize(treeNode);
1128-
bool isUnsignedMultiply = ((treeNode->gtFlags & GTF_UNSIGNED) != 0);
1128+
bool isUnsignedMultiply = treeNode->IsUnsigned();
11291129
bool requiresOverflowCheck = treeNode->gtOverflowEx();
11301130

11311131
GenTree* op1 = treeNode->gtGetOp1();
@@ -6657,7 +6657,7 @@ void CodeGen::genCompareInt(GenTreeOp* treeNode)
66576657
// The common type cannot be smaller than any of the operand types, we're probably mixing int/long
66586658
assert(genTypeSize(type) >= max(genTypeSize(op1Type), genTypeSize(op2Type)));
66596659
// Small unsigned int types (TYP_BOOL can use anything) should use unsigned comparisons
6660-
assert(!(varTypeIsSmall(type) && varTypeIsUnsigned(type)) || ((tree->gtFlags & GTF_UNSIGNED) != 0));
6660+
assert(!(varTypeIsSmall(type) && varTypeIsUnsigned(type)) || tree->IsUnsigned());
66616661
// If op1 is smaller then it cannot be in memory, we're probably missing a cast
66626662
assert((genTypeSize(op1Type) >= genTypeSize(type)) || !op1->isUsedFromMemory());
66636663
// If op2 is smaller then it cannot be in memory, we're probably missing a cast
@@ -6819,7 +6819,7 @@ void CodeGen::genLongToIntCast(GenTree* cast)
68196819

68206820
genConsumeRegs(src);
68216821

6822-
var_types srcType = ((cast->gtFlags & GTF_UNSIGNED) != 0) ? TYP_ULONG : TYP_LONG;
6822+
var_types srcType = cast->IsUnsigned() ? TYP_ULONG : TYP_LONG;
68236823
var_types dstType = cast->CastToType();
68246824
regNumber loSrcReg = src->gtGetOp1()->GetRegNum();
68256825
regNumber hiSrcReg = src->gtGetOp2()->GetRegNum();

‎src/coreclr/jit/compiler.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9996,7 +9996,7 @@ JITDBGAPI void __cdecl cTreeFlags(Compiler* comp, GenTree* tree)
99969996
{
99979997
chars += printf("[DONT_CSE]");
99989998
}
9999-
if (tree->gtFlags & GTF_UNSIGNED)
9999+
if (tree->IsUnsigned())
1000010000
{
1000110001
chars += printf("[SMALL_UNSIGNED]");
1000210002
}

‎src/coreclr/jit/decomposelongs.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1080,9 +1080,9 @@ GenTree* DecomposeLongs::DecomposeArith(LIR::Use& use)
10801080
hiResult->gtFlags |= GTF_OVERFLOW | GTF_EXCEPT;
10811081
loResult->gtFlags &= ~(GTF_OVERFLOW | GTF_EXCEPT);
10821082
}
1083-
if (loResult->gtFlags & GTF_UNSIGNED)
1083+
if (loResult->IsUnsigned())
10841084
{
1085-
hiResult->gtFlags |= GTF_UNSIGNED;
1085+
hiResult->SetUnsigned();
10861086
}
10871087
}
10881088

‎src/coreclr/jit/emitarm.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8178,7 +8178,7 @@ regNumber emitter::emitInsTernary(instruction ins, emitAttr attr, GenTree* dst,
81788178
regNumber extraReg = codeGen->internalRegisters.GetSingle(dst);
81798179
assert(extraReg != dst->GetRegNum());
81808180

8181-
if ((dst->gtFlags & GTF_UNSIGNED) != 0)
8181+
if (dst->IsUnsigned())
81828182
{
81838183
// Compute 8 byte result from 4 byte by 4 byte multiplication.
81848184
emitIns_R_R_R_R(INS_umull, EA_4BYTE, dst->GetRegNum(), extraReg, src1->GetRegNum(), src2->GetRegNum());
@@ -8214,7 +8214,7 @@ regNumber emitter::emitInsTernary(instruction ins, emitAttr attr, GenTree* dst,
82148214
}
82158215
else
82168216
{
8217-
bool isUnsignedOverflow = ((dst->gtFlags & GTF_UNSIGNED) != 0);
8217+
bool isUnsignedOverflow = dst->IsUnsigned();
82188218
jumpKind = isUnsignedOverflow ? EJ_lo : EJ_vs;
82198219
if (jumpKind == EJ_lo)
82208220
{

‎src/coreclr/jit/emitarm64.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14784,7 +14784,7 @@ regNumber emitter::emitInsTernary(instruction ins, emitAttr attr, GenTree* dst,
1478414784
regNumber extraReg = codeGen->internalRegisters.GetSingle(dst);
1478514785
assert(extraReg != dst->GetRegNum());
1478614786

14787-
if ((dst->gtFlags & GTF_UNSIGNED) != 0)
14787+
if (dst->IsUnsigned())
1478814788
{
1478914789
if (attr == EA_4BYTE)
1479014790
{

0 commit comments

Comments
 (0)