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

Bill Schmidt via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon May 23 06:24:17 PDT 2022


wjschmidt updated this revision to Diff 431351.
wjschmidt edited the summary of this revision.
wjschmidt added a comment.

This revision now includes fixes for both cases:  a horizontal reduction of fadds fed by a vectorized tree rooted at fmuls, and a vectorized tree of fmuls that feeds arbitrary fadds and/or fsubs.  It was misguided to break this up into two patches without showing you both...

The reviewers of the previous revision requested that the FMA cost calculation be moved into TTI.  I've done that here, but I kept it as a separate calculation rather than incorporating it somehow into getArithmeticInstrCost, for a couple of reasons.  If I understood the proposal correctly, it was to have getArithmeticInstrCost when called on an FAdd (or FSub) produce a lesser cost when fed by an FMul as one of the operands, reporting that the FAdd/FSub is essentially free in those circumstances.  (If I've misunderstood, please correct me.)

First, I didn't find this particularly helpful for the use cases I'm addressing here.  For example, for the tryToVectorizeList() case, we have a collection of FMul instructions.  We can follow the defs to get the FAdd/FSubs to ask about their cost, but that will just get us a raw number.  It doesn't tell us how much we ought to add back to the cost for breaking the FMA.  (I don't think that just having the altered cost for FAdd/FSub will avoid us considering the FMul vectorization in the first place.)

Second, I worry about unintended effects throughout the middle end by altering the cost model this way.  I think it's better to provide the additional interface so that the lost FMA cost can be taken into account only where it's known to matter.

Thanks for the reviews!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125987

Files:
  llvm/include/llvm/Analysis/TargetTransformInfo.h
  llvm/include/llvm/Transforms/Vectorize/SLPVectorizer.h
  llvm/lib/Analysis/TargetTransformInfo.cpp
  llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
  llvm/test/Transforms/SLPVectorizer/X86/slp-fma-loss.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D125987.431351.patch
Type: text/x-patch
Size: 9075 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220523/2491d1e1/attachment.bin>


More information about the llvm-commits mailing list