[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/test/Analysis/CostModel/AArch64/masked_ldst_vls.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py
+; RUN: opt < %s -cost-model -analyze -mtriple=aarch64-linux-gnu -mattr=+sve -aarch64-sve-vector-bits-min=256 | FileCheck %s -D#VBITS=256
----------------
This should be removed as I imagine it's not true and you just inherited it from a file you copied.


================
Comment at: llvm/test/Analysis/CostModel/AArch64/masked_ldst_vls.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 -aarch64-sve-vector-bits-min=256 | FileCheck %s -D#VBITS=256
+; RUN: opt < %s -cost-model -analyze -mtriple=aarch64-linux-gnu -mattr=+sve -aarch64-sve-vector-bits-min=384 | FileCheck %s -D#VBITS=256
----------------
Rather than having `-mtriple=aarch64-linux-gnu -mattr=+sve` on ever `RUN` line can you use LLVM IR directly. For example: 
```
target triple = "aarch64-unknown-linux-gnu"

define void @fixed-sve-vls() #0 {
....
}

attributes #0 = { "target-features"="+sve" }
```
It just means you need to know less of the details when running the test manually.




================
Comment at: llvm/test/Analysis/CostModel/AArch64/masked_ldst_vls.ll:29
+entry:
+  %v1024i1 = call <1024 x i1> @llvm.masked.load.v1024i1.p0v1024i1(<1024 x i1> *undef, i32 8, <1024 x i1> undef, <1024 x i1> undef)
+  %v256i8 = call <256 x i8> @llvm.masked.load.v256i8.p0v256i8(<256 x i8> *undef, i32 8, <256 x i1> undef, <256 x i8> undef)
----------------
Storing vectors of i1 is a thorny issue so I think it best to not bother testing them just yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107651