[PATCH] D109416: getVPMemoryOpCost interface

[Bardia Mahjour via Phabricator via llvm-commits]

bmahjour marked an inline comment as done.
bmahjour added inline comments.


================
Comment at: llvm/include/llvm/Analysis/TargetTransformInfo.h:1387
+  /// Use of %evl is discouraged when that is not the case.
+  bool hasActiveVectorLength(Type *DataType, Align Alignment) const;
 
----------------
simoll wrote:
> This is really just a getter that tells you whether using VP intrinsics makes sense on this target at all. It shouldn't be modified this way.
> 
> Couldn't you use the cost function to tell you whether a combination of %evl and type for memory accesses makes sense?
> 
> 
Whether the target supports EVL in hardware may depend on opcode and data type. For example, Power9/10 only support load and stores. Other targets may have dependency on data type or alignment. For these reasons I think we should pass opcode and datatype/alignment as input to this function. I just realized I had forgotten to add opcode to this interface. I'll do that.

Having a function that can tell us whether an opcode/type combination can be efficiently vector predicated (without having to model the cost) is helpful in LoopVectorizer as well. For example when deciding how to vectorize we don't have a VPIntrinsic yet, so we cannot use `getVPLegalizationStrategy`. Having something similar to `isLegalMaskedLoad` would be useful, and I was hoping we use `hasActiveVectorLength()` for that purpose (per previous conversation in D99750#inline-949716). Is there another interface I should we looking at?


================
Comment at: llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp:1341
+
+bool PPCTTIImpl::hasActiveVectorLength(Type *DataType, Align Alignment) const {
+  // TODO
----------------
simoll wrote:
> This patch should only be for TTI. You are using the default impl here anyway - target-specific changes should go in a separate patch.
This is just a stub, but I'll move it to D109417 for cleaner separation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109416