Skip to content

JIT: Enable fgOptimizeRelationalComparisonWithCasts for EQ/NE#128091

Open
BoyBaykiller wants to merge 1 commit into
dotnet:mainfrom
BoyBaykiller:fgOptimizeRelationalComparisonWithCasts-for-EQ-NE
Open

JIT: Enable fgOptimizeRelationalComparisonWithCasts for EQ/NE#128091
BoyBaykiller wants to merge 1 commit into
dotnet:mainfrom
BoyBaykiller:fgOptimizeRelationalComparisonWithCasts-for-EQ-NE

Conversation

@BoyBaykiller
Copy link
Copy Markdown
Contributor

@BoyBaykiller BoyBaykiller commented May 12, 2026

We were previously only calling that for GT_LT, GT_LE, GT_GE, GT_GT.
Example:

bool Test(uint a)
{
    return (ulong)(a) == uint.MaxValue;
}
;; ------ BASE
G_M000_IG02:
       mov      eax, edx
       mov      ecx, 0xFFFFFFFF
       cmp      rax, rcx
       sete     al
       movzx    rax, al

;; ------ DIFF
G_M60965_IG02:
       cmp      edx, -1
       sete     al
       movzx    rax, al

@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 12, 2026
@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label May 12, 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.

@BoyBaykiller BoyBaykiller force-pushed the fgOptimizeRelationalComparisonWithCasts-for-EQ-NE branch from fe6c9b2 to 4654abb Compare May 21, 2026 02:46
Comment thread src/coreclr/jit/morph.cpp
return cmp;
}

assert(genActualType(cmp->gtGetOp1()) == genActualType(cmp->gtGetOp2()));
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reason I moved this down:

When calling this function for EQ/NE this assert fired in some coreclr_tests. And moving it after the supportedOp check fixes it. This is the tree it failed on:

[000030] --CXG------                         *  EQ        int   
[000013] --CXG+-----                         +--*  CALL help ref    CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPE_MAYBENULL
[000469] n---G+----- arg0 rcx                |  \--*  IND       byref 
[000468] -----+-----                         |     \--*  ADD       byref 
[000011] -----+-----                         |        +--*  LCL_VAR   byref  V01 arg1         
[000467] -----+-----                         |        \--*  CNS_INT   long   8
[000029] ----G+---U-                         \--*  CAST      long <- uint
[000470] ----G+-----                            \--*  EQ        int   
[000473] n---G+-----                               +--*  IND       byref 
[000472] -----+-----                               |  \--*  ADD       byref 
[000018] -----+-----                               |     +--*  LCL_VAR   byref  V02 arg2         
[000471] -----+-----                               |     \--*  CNS_INT   long   8
[000476] n---G+-----                               \--*  IND       byref 
[000475] -----+-----                                  \--*  ADD       byref 
[000025] -----+-----                                     +--*  LCL_VAR   byref  V03 arg3         
[000474] -----+-----                                     \--*  CNS_INT   long   8

Comment thread src/coreclr/jit/morph.cpp
// TODO-CQ: Should be called for all comparisons
if (tree->OperIs(GT_LT, GT_LE, GT_GE, GT_GT) &&
(tree->gtGetOp1()->OperIs(GT_CAST) || tree->gtGetOp2()->OperIs(GT_CAST)))
if (tree->OperIsCmpCompare() && (tree->gtGetOp1()->OperIs(GT_CAST) || tree->gtGetOp2()->OperIs(GT_CAST)))
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is the (only) meaningful change.

@BoyBaykiller
Copy link
Copy Markdown
Contributor Author

@EgorBo PTAL. Simple change

Comment thread src/coreclr/jit/morph.cpp
{
assert(cmp->OperIsCmpCompare());
assert(cmp->gtGetOp1()->OperIs(GT_CAST) || cmp->gtGetOp2()->OperIs(GT_CAST));
assert(genActualType(cmp->gtGetOp1()) == genActualType(cmp->gtGetOp2()));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why? if you think this assert is incorrect you should fix it, not move it to somewhere to silence it

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hm, doesn't it make sense to move it after the supportedOp check

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 community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants