[PATCH] D107651: [AArch64][SVE] Teach cost model that masked loads/stores are cheap

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:1505
                                       TTI::TargetCostKind CostKind) {
-  if (!isa<ScalableVectorType>(Src))
-    return BaseT::getMaskedMemoryOpCost(Opcode, Src, Alignment, AddressSpace,
----------------
david-arm wrote:
> I don't think we should be removing this because it's needed when we don't know the SVE vector length, in which case we will have to scalarise the masked ops. I think you can add an extra check here to see if we're using SVE for fixed width vectors.
Perhaps just add something like `&& !ST->useSVEForFixedLengthVectors()`


================
Comment at: llvm/test/Analysis/CostModel/AArch64/masked_ldst.ll:2
 ; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py
 ; RUN: opt < %s -cost-model -analyze -mtriple=aarch64-linux-gnu -mattr=+sve | FileCheck %s
 
----------------
MattDevereau wrote:
> bsmith wrote:
> > This test is for scalable types, hence the changes in here are testing the cost of scalable masked loads/stores rather than fixed ones. You'll need to add functions (probably in another test file) that use the vscale_range attribute to specify various different fixed vector lengths)
> What is the difference between this test called "fixed" and the test called "scalable" further down then?
@bsmith Can this not be done using multiple RUN lines instead?  I'm hoping that a single set of CHECK lines can be used by utilising FileChecks math functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107651