[PATCH] D125987: [SLP] Account for cost of removing FMA opportunities by horizontal reduction

Thu May 19 10:20:41 PDT 2022

ABataev added inline comments.

================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:9675
+// the tree of multiplies.
+static InstructionCost getFMALossCost(const TargetTransformInfo *TTI,
+                                      Value *TreeRootOp, FastMathFlags FMF,
----------------
RKSimon wrote:
> wjschmidt wrote:
> > ABataev wrote:
> > > wjschmidt wrote:
> > > > ABataev wrote:
> > > > > I think it must be a part of TTI::getArithmeticReductionCost
> > > > Because this will also be called in a case where it's not part of a reduction, I'd like to keep it separate.  (See my comment in the summary.)  Perhaps I shouldn't have split the patches so this would be more evident.
> > > Anyway, maybe better to make it a part of TTI rather than put it to SLPVectorizer?
> > That's certainly possible, and probably a good idea.  We'd need to pull out the final multiplication by Width and the SLP-specific debug stuff, but with that done it would fit nicely in TTI, and there might be potential for reuse.
> Ideally the getArithmeticReductionCost costs wouldn't be affected but getArithmeticInstrCost could be adjusted to account for potential FMA patterns in the scalar/vector FADD costs
Yep, good idea!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125987