[PATCH] D75525: [TTI][ARM][MVE] Refine gather/scatter cost model

[Anna Welker via Phabricator via llvm-commits]

anwel marked 10 inline comments as done.
anwel added inline comments.


================
Comment at: llvm/include/llvm/Analysis/TargetTransformInfo.h:968
   ///                   that is not a compile-time constant
   /// \p Alignment - alignment of single element
   int getGatherScatterOpCost(unsigned Opcode, Type *DataTy, Value *Ptr,
----------------
RKSimon wrote:
> Please can you add the doxygen param entry?
Yes, thanks for the reminder.


================
Comment at: llvm/include/llvm/Analysis/TargetTransformInfo.h:1012
                             ArrayRef<Value *> Args, FastMathFlags FMF,
-                            unsigned VF = 1) const;
+                            const Instruction *I, unsigned VF = 1) const;
 
----------------
dmgreen wrote:
> Is it possible to keep I last and give it a default argument of nullptr? Or does that cause other problems?
I have a slight aversion against multiple default arguments in one function, as they might lead to confusion.
I've checked though that C++ warns in this case if no VF but an instruction is passed, so as it reduces the amount of necessary changes I'll take your advice and add a default value.


================
Comment at: llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp:921
+        (match(I, m_Intrinsic<Intrinsic::masked_scatter>()) &&
+         I->getNumUses() == 0 && (T = dyn_cast<TruncInst>(I->getOperand(1))))) {
+      // only allow valid type combinations
----------------
samparker wrote:
> dmgreen wrote:
> > I don't think this  I->getNumUses() == 0 is needed?
> Do we really need to check the uses of a store?
No we don't :)


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

https://reviews.llvm.org/D75525