[llvm] LV: Expand llvm.histogram intrinsic to support umax, umin, and uadd.sat operations (PR #127399)

[via llvm-commits]

================
@@ -2428,23 +2441,53 @@ InstructionCost VPHistogramRecipe::computeCost(ElementCount VF,
   Type *IncTy = IncAmt->getScalarType();
   VectorType *VTy = VectorType::get(IncTy, VF);
 
-  // Assume that a non-constant update value (or a constant != 1) requires
-  // a multiply, and add that into the cost.
-  InstructionCost MulCost =
-      Ctx.TTI.getArithmeticInstrCost(Instruction::Mul, VTy, Ctx.CostKind);
-  if (match(IncAmt, m_One()))
-    MulCost = TTI::TCC_Free;
+  // For umin/umax, there's no multiplication — the increment value is compared
+  // directly. For add/sub/uadd.sat, assume that a non-constant update value
+  // (or a constant != 1) requires a multiply.
+  InstructionCost MulCost = TTI::TCC_Free;
+  if (!match(IncAmt, m_One()) && UpdateKind != HistogramUpdateKind::UMax &&
+      UpdateKind != HistogramUpdateKind::UMin)
+    MulCost =
+        Ctx.TTI.getArithmeticInstrCost(Instruction::Mul, VTy, Ctx.CostKind);
 
   // Find the cost of the histogram operation itself.
   Type *PtrTy = VectorType::get(AddressTy, VF);
   Type *MaskTy = VectorType::get(Type::getInt1Ty(Ctx.LLVMCtx), VF);
-  IntrinsicCostAttributes ICA(Intrinsic::experimental_vector_histogram_add,
+  IntrinsicCostAttributes ICA(getHistogramIntrinsicID(),
                               Type::getVoidTy(Ctx.LLVMCtx),
                               {PtrTy, IncTy, MaskTy});
 
-  // Add the costs together with the add/sub operation.
+  // Compute the cost of the update operation.
+  InstructionCost UpdateCost;
+  switch (UpdateKind) {
+  case HistogramUpdateKind::Add:
+    UpdateCost =
+        Ctx.TTI.getArithmeticInstrCost(Instruction::Add, VTy, Ctx.CostKind);
+    break;
+  case HistogramUpdateKind::Sub:
+    UpdateCost =
+        Ctx.TTI.getArithmeticInstrCost(Instruction::Sub, VTy, Ctx.CostKind);
+    break;
+  case HistogramUpdateKind::UAddSat: {
+    IntrinsicCostAttributes UpdateICA(Intrinsic::uadd_sat, VTy, {VTy, VTy});
+    UpdateCost = Ctx.TTI.getIntrinsicInstrCost(UpdateICA, Ctx.CostKind);
+    break;
+  }
+  case HistogramUpdateKind::UMax: {
+    IntrinsicCostAttributes UpdateICA(Intrinsic::umax, VTy, {VTy, VTy});
+    UpdateCost = Ctx.TTI.getIntrinsicInstrCost(UpdateICA, Ctx.CostKind);
+    break;
+  }
+  case HistogramUpdateKind::UMin: {
+    IntrinsicCostAttributes UpdateICA(Intrinsic::umin, VTy, {VTy, VTy});
+    UpdateCost = Ctx.TTI.getIntrinsicInstrCost(UpdateICA, Ctx.CostKind);
----------------
RonDahan101 wrote:

You're right — the TTI cost for the histogram intrinsics ( BasicTTIImpl ) already includes the update operation (load + update + store per element). The separate UpdateCost was double-counting. Removed it; the cost is now just  TTI(histogram_intrinsic) + MulCost .

https://github.com/llvm/llvm-project/pull/127399