[PATCH] D135991: [AArch64] Fix cost model for `udiv` instruction when one of the operands is a uniform constant

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

Can you explain more about the motivating case you have? Why should the cost of the vector divide be so low?



================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:2109
+            (Op2Info.isConstant() && Op2Info.isUniform())) {
+          InstructionCost InsertCost = getArithmeticInstrCost(
+              Instruction::InsertElement, Ty, CostKind, Op1Info, Op2Info);
----------------
getArithmeticInstrCost should not be used with InsertElement or ExtractElement. It is used for arithmetic instructions like Add and Div. getVectorInstrCost should be used with InsertElement and ExtractElement.

For the i64 Mul case below we hard-coded the values 2 instead, due to the more regular nature of the scalarization. (It was originally 1, but we had to increase it as it was not quite high enough in cases).


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:2114
+          FixedVectorType *VTy = cast<FixedVectorType>(Ty);
+          LLVM_DEBUG(dbgs() << "Insert Cost: " << InsertCost
+                            << " Extract Cost: " << ExtractCost << "\n");
----------------
Remove the debug message in the final version.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:2116
+                            << " Extract Cost: " << ExtractCost << "\n");
+          return InsertCost + ExtractCost + VTy->getNumElements();
+        }
----------------
This is assuming that the cost of the actual divide is 1? That sounds too low in some cases, compared to scalar. Should it be `(InsertCost + ExtractCost + Cost) * VTy->getNumElements()`? 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135991/new/

https://reviews.llvm.org/D135991