[PATCH] D132872: [SLP] Account for loss due to FMAs when estimating fmul costs.

Mon Aug 29 11:29:50 PDT 2022

ABataev added a comment.

In D132872#3756244 <https://reviews.llvm.org/D132872#3756244>, @fhahn wrote:

> In D132872#3756205 <https://reviews.llvm.org/D132872#3756205>, @ABataev wrote:
>
>> Similar question to D125987 <https://reviews.llvm.org/D125987>, why we you cannot put related cost changes to getArithmeticInstrCost?
>
> I think one reason is that TTI usually avoids to rely on the contex instruction/use-list scans. Not saying this is a blocker though if we converge on this as solution.

Not quite true, since it accepts operands as an argument.

> The other issue is that we can't estimate the fact that the widened FMUL will also be applicable for fusion because no context instruction to pass exists yet.

Do not see a problem here.

> I think it would be good to role out a solution that first fixes the issue in SLP first and then possible move it to TTI once it has proven itself. This is an issue that mostly impacts the SLP vectorizer AFAICT.

Not sure this a good decision.

================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:6449-6450
+          NumScalarFusable +=
+              TTI->getIntrinsicInstrCost(ICA, TTI::TCK_RecipThroughput) ==
+              TargetTransformInfo::TCC_Basic;
+        }
----------------
The cost-based analysis may lead to the wrong final decision here, need to return something like a flag instead or implement this analysis in TTI. What if the cost of Intrinsic::fmuladd != TCC_Basic?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132872