Skip to content

Combine cast => negate => compare on ARM64 #112944

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kunalspathak merged 1 commit into from
Feb 28, 2025
Merged

Combine cast => negate => compare on ARM64 #112944

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 37 additions & 1 deletion src/coreclr/jit/codegenarm64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4775,8 +4775,44 @@ void CodeGen::genCodeForCompare(GenTreeOp* tree)
emit->emitIns_R_R_I(ins, cmpSize, op1->GetRegNum(), shiftOp1->GetRegNum(),
shiftOp2->AsIntConCommon()->IntegralValue(),
ShiftOpToInsOpts(oper));
break;
}
case GT_CAST:
{
GenTreeCast* cast = op2->gtGetOp1()->AsCast();

GenIntCastDesc desc(cast);

// These casts should not lead to an overflow check.
assert(desc.CheckKind() == GenIntCastDesc::CHECK_NONE);

insOpts extOpts = INS_OPTS_NONE;
switch (desc.ExtendKind())
{
case GenIntCastDesc::ZERO_EXTEND_SMALL_INT:
extOpts = (desc.ExtendSrcSize() == 1) ? INS_OPTS_UXTB : INS_OPTS_UXTH;
break;
case GenIntCastDesc::SIGN_EXTEND_SMALL_INT:
extOpts = (desc.ExtendSrcSize() == 1) ? INS_OPTS_SXTB : INS_OPTS_SXTH;
break;
case GenIntCastDesc::ZERO_EXTEND_INT:
extOpts = INS_OPTS_UXTW;
break;
case GenIntCastDesc::SIGN_EXTEND_INT:
extOpts = INS_OPTS_SXTW;
break;
case GenIntCastDesc::COPY:
extOpts = INS_OPTS_NONE; // Perform cast implicitly.
break;
default:
// Other casts should not lead here as they will not pass the
// IsContainableUnaryOrBinaryOp check.
unreached();
}

emit->emitIns_R_R(ins, cmpSize, op1->GetRegNum(), cast->CastOp()->GetRegNum(), extOpts);
break;
}
break;

default:
unreached();
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/emitarm64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4695,13 +4695,13 @@ void emitter::emitIns_R_R(instruction ins,
case INS_str:
case INS_strb:
case INS_strh:
case INS_cmn:
case INS_tst:
assert(insOptsNone(opt));
emitIns_R_R_I(ins, attr, reg1, reg2, 0, INS_OPTS_NONE);
return;

case INS_cmp:
case INS_cmn:
emitIns_R_R_I(ins, attr, reg1, reg2, 0, opt);
return;

Expand Down
20 changes: 16 additions & 4 deletions src/coreclr/jit/lowerarmarch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ bool Lowering::IsContainableUnaryOrBinaryOp(GenTree* parentNode, GenTree* childN
if (childNode->OperIs(GT_NEG))
{
// If we have a contained LSH, RSH or RSZ, we can still contain NEG if the parent is a EQ or NE.
if (childNode->gtGetOp1()->isContained() && !childNode->gtGetOp1()->OperIs(GT_LSH, GT_RSH, GT_RSZ))
if (childNode->gtGetOp1()->isContained() && !childNode->gtGetOp1()->OperIs(GT_LSH, GT_RSH, GT_RSZ, GT_CAST))
{
// Cannot contain if the childs op1 is already contained
return false;
Expand All @@ -399,10 +399,23 @@ bool Lowering::IsContainableUnaryOrBinaryOp(GenTree* parentNode, GenTree* childN
// EQ and NE are the only valid comparison ops that can contain NEG.
if (parentNode->OperIs(GT_EQ, GT_NE))
{
if (IsInvariantInRange(childNode, parentNode))
if (!IsInvariantInRange(childNode, parentNode))
{
return true;
return false;
}

if (childNode->gtGetOp1()->OperIs(GT_CAST))
{
// Grab the cast as well, we can contain this with cmn.
GenTreeCast* cast = childNode->gtGetOp1()->AsCast();

assert(!cast->gtOverflow());
assert(varTypeIsIntegral(cast) && varTypeIsIntegral(cast->CastToType()));

MakeSrcContained(childNode, cast);
}

return true;
}

return false;
Expand Down Expand Up @@ -459,7 +472,6 @@ bool Lowering::IsContainableUnaryOrBinaryOp(GenTree* parentNode, GenTree* childN
}
}

// TODO: Handle CMN
return false;
}

Expand Down
67 changes: 67 additions & 0 deletions src/tests/JIT/opt/InstructionCombining/Cmn.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,31 @@ public static int CheckCompareNegative()
fail = true;
}

if (!CmnExtendedB(1, 0xff))
{
fail = true;
}
if (!CmnExtendedH(1, 0xffff))
{
fail = true;
}
if (!CmnExtendedS(1, 0xffffffff))
{
fail = true;
}
if (!CmnExtendedUB(-1, 0x101))
{
fail = true;
}
if (!CmnExtendedUH(-1, 0x10001))
{
fail = true;
}
if (!CmnExtendedUS(-1, 0x100000001))
{
fail = true;
}

if (fail)
{
return 101;
Expand Down Expand Up @@ -134,5 +159,47 @@ static bool CmnLargeShift64Bit(long a, long b)
}
return false;
}

[MethodImpl(MethodImplOptions.NoInlining)]
static bool CmnExtendedB(int a, int b)
{
//ARM64-FULL-LINE: cmn {{w[0-9]+}}, {{w[0-9]+}}, SXTB
return (a == -(sbyte)b);
}

[MethodImpl(MethodImplOptions.NoInlining)]
static bool CmnExtendedH(int a, int b)
{
//ARM64-FULL-LINE: cmn {{w[0-9]+}}, {{w[0-9]+}}, SXTH
return (a == -(short)b);
}

[MethodImpl(MethodImplOptions.NoInlining)]
static bool CmnExtendedS(long a, long b)
{
//ARM64-FULL-LINE: cmn {{x[0-9]+}}, {{w[0-9]+}}, SXTW
return (a == -(long)(int)b);
}

[MethodImpl(MethodImplOptions.NoInlining)]
static bool CmnExtendedUB(int a, int b)
{
//ARM64-FULL-LINE: cmn {{w[0-9]+}}, {{w[0-9]+}}, UXTB
return (a == -(byte)b);
}

[MethodImpl(MethodImplOptions.NoInlining)]
static bool CmnExtendedUH(int a, int b)
{
//ARM64-FULL-LINE: cmn {{w[0-9]+}}, {{w[0-9]+}}, UXTH
return (a == -(ushort)b);
}

[MethodImpl(MethodImplOptions.NoInlining)]
static bool CmnExtendedUS(long a, long b)
{
//ARM64-FULL-LINE: cmn {{x[0-9]+}}, {{w[0-9]+}}, UXTW
return (a == -(uint)b);
}
}
}