[PATCH] D111630: [LoopVectorize][CostModel] Update cost model for fmuladd intrinsic

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added a comment.

This cost calculation seems correct to me. For in-loop reductions it calculates the cost as a single fmul + fadd *reduction*. If this is not an in-loop reduction, or if fmuladd is not used in a reduction, it will follow the regular code-path to get the cost of this intrinsic (either as an FMA or separate fmul+fadd).

So, LGTM! (Please address the minor nits before you commit)



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7927
   case Instruction::Call: {
+    // Recognize a call to the llvm.fmuladd intrinsic.
+    if (RecurrenceDescriptor::isFMulAddIntrinsic(I)) {
----------------
nit: redundant comment (i.e. the code says as much)


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7928
+    // Recognize a call to the llvm.fmuladd intrinsic.
+    if (RecurrenceDescriptor::isFMulAddIntrinsic(I)) {
+      // Detect reduction patterns.
----------------
nit: unnecessary curly braces.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7929
+    if (RecurrenceDescriptor::isFMulAddIntrinsic(I)) {
+      // Detect reduction patterns.
+      if (auto RedCost = getReductionPatternCost(I, VF, VectorTy, CostKind))
----------------
nit: redundant comment.


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

https://reviews.llvm.org/D111630