[PATCH] D93476: [LV][ARM] Inloop reduction cost modelling

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/include/llvm/Analysis/TargetTransformInfo.h:1191
+  int getExtendedReductionCost(
+      bool IsMLA, bool IsUnsigned, Type *ResTy, VectorType *Ty,
+      TTI::TargetCostKind CostKind = TTI::TCK_RecipThroughput) const;
----------------
SjoerdMeijer wrote:
> SjoerdMeijer wrote:
> > I am wondering if `IsMLA` is a bit too narrow as an interface, perhaps even unclear. If this is similar to `getArithmeticReductionCost` as mentioned in the comment, which takes an `opcode`, should this also take an `opcode` instead of `IsMLA`? The advantage we could describe costs for different type of reductions, or is this not useful/necessary?
> Or now I am thinking if we actually need a new interface at all. Could this just be part of `getArithmeticReductionCost`, maybe with a flag if operands are extended?
I think the only extended patterns that are interesting are add's. Mul's maybe could come up but they are very rare in comparison. As you might imagine adding up number is pretty common in comparison! And/Or/Xor don't really make sense to be extended, as well as min/max I think. Same for floats where I don't know of any systems that convert the float type and reduce at the same time.

In MVE we only have VADDV/VMLAV instructions that need the extension, not any others. I've tried to update the comment to explain better that this is an extended add pattern. (And we can always extend it in the future if needed).

The separate interface was suggested in https://reviews.llvm.org/D93476#2506583, which I like as it keeps the normal reductions in one place, and the extended pattern is matched here.


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

https://reviews.llvm.org/D93476