Skip to content

[JIT] ARM64 - Combine cmp and shift ops into a single cmp op #84605

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
merged 8 commits into from
Apr 17, 2023
Merged

[JIT] ARM64 - Combine cmp and shift ops into a single cmp op #84605

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
252c80a
Combine compare and shift ops into a single compare op
TIHan Apr 11, 2023
6455122
Fix comment
TIHan Apr 11, 2023
cb932ba
Expanding IsContainableBinaryOp.
TIHan Apr 11, 2023
860c8de
Remove commented code
TIHan Apr 11, 2023
6559201
Added more cases
TIHan Apr 11, 2023
b350e97
Update codegenarm64.cpp
TIHan Apr 13, 2023
c197cd8
Feedback
TIHan Apr 13, 2023
d0851d1
Feedback
TIHan Apr 13, 2023
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
1 change: 1 addition & 0 deletions src/coreclr/jit/codegen.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1552,6 +1552,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX

#if defined(TARGET_ARM64)
static insCond JumpKindToInsCond(emitJumpKind condition);
static insOpts ShiftOpToInsOpts(genTreeOps op);
#elif defined(TARGET_XARCH)
static instruction JumpKindToCmov(emitJumpKind condition);
#endif
Expand Down
60 changes: 35 additions & 25 deletions src/coreclr/jit/codegenarm64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2626,31 +2626,7 @@ void CodeGen::genCodeForBinary(GenTreeOp* tree)
}
}

switch (op2->gtOper)
{
case GT_LSH:
{
opt = INS_OPTS_LSL;
break;
}

case GT_RSH:
{
opt = INS_OPTS_ASR;
break;
}

case GT_RSZ:
{
opt = INS_OPTS_LSR;
break;
}

default:
{
unreached();
}
}
opt = ShiftOpToInsOpts(op2->gtOper);

emit->emitIns_R_R_R_I(ins, emitActualTypeSize(tree), targetReg, a->GetRegNum(), b->GetRegNum(),
c->AsIntConCommon()->IconValue(), opt);
Expand Down Expand Up @@ -4546,6 +4522,15 @@ void CodeGen::genCodeForCompare(GenTreeOp* tree)

emit->emitIns_R_I(ins, cmpSize, op1Reg, intConst->IconValue());
}
else if (op2->isContained())
{
assert(op2->OperIs(GT_LSH, GT_RSH, GT_RSZ));
assert(op2->gtGetOp2()->IsCnsIntOrI());
assert(op2->gtGetOp2()->isContained());

emit->emitIns_R_R_I(ins, cmpSize, op1->GetRegNum(), op2->gtGetOp1()->GetRegNum(),
op2->gtGetOp2()->AsIntConCommon()->IntegralValue(), ShiftOpToInsOpts(op2->gtOper));
}
else
{
emit->emitIns_R_R(ins, cmpSize, op1->GetRegNum(), op2->GetRegNum());
Expand Down Expand Up @@ -10329,4 +10314,29 @@ insCond CodeGen::JumpKindToInsCond(emitJumpKind condition)
}
}

//------------------------------------------------------------------------
// ShiftOpToInsOpts: Convert a shift-op to a insOpts.
//
// Arguments:
// shiftOp - the shift-op
//
insOpts CodeGen::ShiftOpToInsOpts(genTreeOps shiftOp)
{
switch (shiftOp)
{
case GT_LSH:
return INS_OPTS_LSL;

case GT_RSH:
return INS_OPTS_ASR;

case GT_RSZ:
return INS_OPTS_LSR;

default:
NO_WAY("expected a shift-op");
return INS_OPTS_NONE;
Comment on lines +10337 to +10338
Copy link
Member

@tannergooding tannergooding Apr 13, 2023

Choose a reason for hiding this comment

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

Should this stay unreached()?

Copy link
Contributor Author

@TIHan TIHan Apr 13, 2023

Choose a reason for hiding this comment

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

Shouldn't matter as long as something fails; also the NO_WAY is more descriptive. There are similar functions doing this.

}
}

#endif // TARGET_ARM64
34 changes: 32 additions & 2 deletions src/coreclr/jit/lowerarmarch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ bool Lowering::IsContainableBinaryOp(GenTree* parentNode, GenTree* childNode) co
return false;
}

if (parentNode->OperIs(GT_CMP, GT_OR, GT_XOR))
if (parentNode->OperIs(GT_CMP, GT_OR, GT_XOR) || parentNode->OperIsCompare())
{
if (IsInvariantInRange(childNode, parentNode))
{
Expand Down Expand Up @@ -2264,7 +2264,37 @@ void Lowering::ContainCheckCast(GenTreeCast* node)
//
void Lowering::ContainCheckCompare(GenTreeOp* cmp)
{
CheckImmedAndMakeContained(cmp, cmp->gtOp2);
GenTree* op1 = cmp->gtGetOp1();
GenTree* op2 = cmp->gtGetOp2();

if (CheckImmedAndMakeContained(cmp, op2))
return;

if (cmp->OperIsCompare() && CheckImmedAndMakeContained(cmp, op1))
{
std::swap(cmp->gtOp1, cmp->gtOp2);
cmp->ChangeOper(cmp->SwapRelop(cmp->gtOper));
return;
}

#ifdef TARGET_ARM64
if (comp->opts.OptimizationEnabled() && cmp->OperIsCompare())
{
if (IsContainableBinaryOp(cmp, op2))
{
MakeSrcContained(cmp, op2);
return;
}

if (IsContainableBinaryOp(cmp, op1))
{
MakeSrcContained(cmp, op1);
std::swap(cmp->gtOp1, cmp->gtOp2);
cmp->ChangeOper(cmp->SwapRelop(cmp->gtOper));
return;
}
}
#endif
}

#ifdef TARGET_ARM64
Expand Down