Skip to content

[AArch64] Improve urem by constant costs #122236

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.

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 1 commit into from
Feb 26, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
62 changes: 50 additions & 12 deletions llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3545,20 +3545,58 @@ InstructionCost AArch64TTIImpl::getArithmeticInstrCost(
return Cost;
}
[[fallthrough]];
case ISD::UDIV: {
case ISD::UDIV:
case ISD::UREM: {
auto VT = TLI->getValueType(DL, Ty);
if (Op2Info.isConstant() && Op2Info.isUniform()) {
if (Op2Info.isConstant()) {
// If the operand is a power of 2 we can use the shift or and cost.
if (ISD == ISD::UDIV && Op2Info.isPowerOf2())
return getArithmeticInstrCost(Instruction::LShr, Ty, CostKind,
Op1Info.getNoProps(),
Op2Info.getNoProps());
if (ISD == ISD::UREM && Op2Info.isPowerOf2())
return getArithmeticInstrCost(Instruction::And, Ty, CostKind,
Op1Info.getNoProps(),
Op2Info.getNoProps());

if (ISD == ISD::UDIV || ISD == ISD::UREM) {
// Divides by a constant are expanded to MULHU + SUB + SRL + ADD + SRL.
// The MULHU will be expanded to UMULL for the types not listed below,
// and will become a pair of UMULL+MULL2 for 128bit vectors.
bool HasMULH = VT == MVT::i64 || LT.second == MVT::nxv2i64 ||
LT.second == MVT::nxv4i32 || LT.second == MVT::nxv8i16 ||
LT.second == MVT::nxv16i8;
bool Is128bit = LT.second.is128BitVector();

InstructionCost MulCost =
getArithmeticInstrCost(Instruction::Mul, Ty, CostKind,
Op1Info.getNoProps(), Op2Info.getNoProps());
InstructionCost AddCost =
getArithmeticInstrCost(Instruction::Add, Ty, CostKind,
Op1Info.getNoProps(), Op2Info.getNoProps());
InstructionCost ShrCost =
getArithmeticInstrCost(Instruction::AShr, Ty, CostKind,
Op1Info.getNoProps(), Op2Info.getNoProps());
InstructionCost DivCost = MulCost * (Is128bit ? 2 : 1) + // UMULL/UMULH
(HasMULH ? 0 : ShrCost) + // UMULL shift
AddCost * 2 + ShrCost;
return DivCost + (ISD == ISD::UREM ? MulCost + AddCost : 0);
}

// TODO: Fix SDIV and SREM costs, similar to the above.
if (TLI->isOperationLegalOrCustom(ISD::MULHU, VT) &&
!VT.isScalableVector()) {
Op2Info.isUniform() && !VT.isScalableVector()) {
// Vector signed division by constant are expanded to the
// sequence MULHS + ADD/SUB + SRA + SRL + ADD, and unsigned division
// to MULHS + SUB + SRL + ADD + SRL.
InstructionCost MulCost = getArithmeticInstrCost(
Instruction::Mul, Ty, CostKind, Op1Info.getNoProps(), Op2Info.getNoProps());
InstructionCost AddCost = getArithmeticInstrCost(
Instruction::Add, Ty, CostKind, Op1Info.getNoProps(), Op2Info.getNoProps());
InstructionCost ShrCost = getArithmeticInstrCost(
Instruction::AShr, Ty, CostKind, Op1Info.getNoProps(), Op2Info.getNoProps());
// sequence MULHS + ADD/SUB + SRA + SRL + ADD.
InstructionCost MulCost =
getArithmeticInstrCost(Instruction::Mul, Ty, CostKind,
Op1Info.getNoProps(), Op2Info.getNoProps());
InstructionCost AddCost =
getArithmeticInstrCost(Instruction::Add, Ty, CostKind,
Op1Info.getNoProps(), Op2Info.getNoProps());
InstructionCost ShrCost =
getArithmeticInstrCost(Instruction::AShr, Ty, CostKind,
Op1Info.getNoProps(), Op2Info.getNoProps());
return MulCost * 2 + AddCost * 2 + ShrCost * 2 + 1;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this part can be removed. Division by non-constant, uniform divisor is giving scalar code. Check here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi - This is inside the top-level if (Op2Info.isConstant()) {, so I don't think that should be an issue. We are only trying to update constant costs in this patch, to keep it simpler. I've added some uniform tests in f08824b to check.

I think this bit of code can probably be removed when sdiv gets added. For now I will re-add the isScalableVector check to make sure the sdiv scores don't change yet.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok. Thanks. Will remove this if(){...} when I update the sdiv/srem patch.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks

}
Expand All @@ -3571,7 +3609,7 @@ InstructionCost AArch64TTIImpl::getArithmeticInstrCost(

InstructionCost Cost = BaseT::getArithmeticInstrCost(
Opcode, Ty, CostKind, Op1Info, Op2Info);
if (Ty->isVectorTy()) {
if (Ty->isVectorTy() && (ISD == ISD::SDIV || ISD == ISD::UDIV)) {
if (TLI->isOperationLegalOrCustom(ISD, LT.second) && ST->hasSVE()) {
// SDIV/UDIV operations are lowered using SVE, then we can have less
// costs.
Expand Down
256 changes: 128 additions & 128 deletions llvm/test/Analysis/CostModel/AArch64/div.ll

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions llvm/test/Analysis/CostModel/AArch64/div_cte.ll
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ define <4 x i32> @sdiv32xi4(<4 x i32> %x) {

define <16 x i8> @udiv8xi16(<16 x i8> %x) {
; CHECK-LABEL: 'udiv8xi16'
; CHECK-NEXT: Cost Model: Found an estimated cost of 7 for instruction: %div = udiv <16 x i8> %x, splat (i8 9)
; CHECK-NEXT: Cost Model: Found an estimated cost of 6 for instruction: %div = udiv <16 x i8> %x, splat (i8 9)
; CHECK-NEXT: Cost Model: Found an estimated cost of 0 for instruction: ret <16 x i8> %div
;
%div = udiv <16 x i8> %x, <i8 9, i8 9, i8 9, i8 9, i8 9, i8 9, i8 9, i8 9, i8 9, i8 9, i8 9, i8 9, i8 9, i8 9, i8 9, i8 9>
Expand All @@ -43,7 +43,7 @@ define <16 x i8> @udiv8xi16(<16 x i8> %x) {

define <8 x i16> @udiv16xi8(<8 x i16> %x) {
; CHECK-LABEL: 'udiv16xi8'
; CHECK-NEXT: Cost Model: Found an estimated cost of 7 for instruction: %div = udiv <8 x i16> %x, splat (i16 9)
; CHECK-NEXT: Cost Model: Found an estimated cost of 6 for instruction: %div = udiv <8 x i16> %x, splat (i16 9)
; CHECK-NEXT: Cost Model: Found an estimated cost of 0 for instruction: ret <8 x i16> %div
;
%div = udiv <8 x i16> %x, <i16 9, i16 9, i16 9, i16 9, i16 9, i16 9, i16 9, i16 9>
Expand All @@ -52,7 +52,7 @@ define <8 x i16> @udiv16xi8(<8 x i16> %x) {

define <4 x i32> @udiv32xi4(<4 x i32> %x) {
; CHECK-LABEL: 'udiv32xi4'
; CHECK-NEXT: Cost Model: Found an estimated cost of 7 for instruction: %div = udiv <4 x i32> %x, splat (i32 9)
; CHECK-NEXT: Cost Model: Found an estimated cost of 6 for instruction: %div = udiv <4 x i32> %x, splat (i32 9)
; CHECK-NEXT: Cost Model: Found an estimated cost of 0 for instruction: ret <4 x i32> %div
;
%div = udiv <4 x i32> %x, <i32 9, i32 9, i32 9, i32 9>
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/Analysis/CostModel/AArch64/fshl.ll
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ declare <2 x i64> @llvm.fshl.v4i64(<2 x i64>, <2 x i64>, <2 x i64>)

define <4 x i30> @fshl_v4i30_3rd_arg_var(<4 x i30> %a, <4 x i30> %b, <4 x i30> %c) {
; CHECK-LABEL: 'fshl_v4i30_3rd_arg_var'
; CHECK-NEXT: Cost Model: Found an estimated cost of 34 for instruction: %fshl = tail call <4 x i30> @llvm.fshl.v4i30(<4 x i30> %a, <4 x i30> %b, <4 x i30> %c)
; CHECK-NEXT: Cost Model: Found an estimated cost of 14 for instruction: %fshl = tail call <4 x i30> @llvm.fshl.v4i30(<4 x i30> %a, <4 x i30> %b, <4 x i30> %c)
; CHECK-NEXT: Cost Model: Found an estimated cost of 0 for instruction: ret <4 x i30> %fshl
;
entry:
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/Analysis/CostModel/AArch64/fshr.ll
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ declare <2 x i64> @llvm.fshr.v4i64(<2 x i64>, <2 x i64>, <2 x i64>)

define <4 x i30> @fshr_v4i30_3rd_arg_var(<4 x i30> %a, <4 x i30> %b, <4 x i30> %c) {
; CHECK-LABEL: 'fshr_v4i30_3rd_arg_var'
; CHECK-NEXT: Cost Model: Found an estimated cost of 34 for instruction: %fshr = tail call <4 x i30> @llvm.fshr.v4i30(<4 x i30> %a, <4 x i30> %b, <4 x i30> %c)
; CHECK-NEXT: Cost Model: Found an estimated cost of 14 for instruction: %fshr = tail call <4 x i30> @llvm.fshr.v4i30(<4 x i30> %a, <4 x i30> %b, <4 x i30> %c)
; CHECK-NEXT: Cost Model: Found an estimated cost of 0 for instruction: ret <4 x i30> %fshr
;
entry:
Expand Down
Loading