[PATCH] D51383: [WebAssembly] SIMD loads and stores

[Heejin Ahn via Phabricator via llvm-commits]

aheejin added inline comments.


================
Comment at: lib/Target/WebAssembly/WebAssemblyInstrSIMD.td:264
+def : StorePatGlobalAddrOffOnly<vec_t, store, !cast<NI>("STORE_"#vec_t)>;
+def : StorePatExternSymOffOnly<vec_t, store, !cast<NI>("STORE_"#vec_t)>;
+
----------------
tlively wrote:
> aheejin wrote:
> > Do we have tests for these load/store offset patterns? Tests for scalars are [[ https://github.com/llvm-mirror/llvm/blob/master/test/CodeGen/WebAssembly/offset.ll | here ]], and I think we need similar variation of tests for SIMDs too. We probably wouldn't need all of offset variation tests for all existing vector types though. Maybe offset tests for one SIMD type would be sufficient.
> I implemented the tests for all types for now, since the marginal cost to do so was small. If we value having a smaller test file more than having all the types covered, I can remove some of the tests.
Yeah it looks like you wrote it once and auto-generated for all other types so, more tests wouldn't hurt. :)


================
Comment at: test/CodeGen/WebAssembly/simd-memory.ll:1
+; RUN: llc < %s -disable-wasm-fallthrough-return-opt -wasm-keep-registers -wasm-disable-explicit-locals -wasm-enable-unimplemented-simd -mattr=+simd128,+sign-ext --show-mc-encoding | FileCheck %s --check-prefixes CHECK,SIMD128
+; RUN: llc < %s -disable-wasm-fallthrough-return-opt -wasm-keep-registers -wasm-disable-explicit-locals -mattr=+simd128,+sign-ext --show-mc-encoding | FileCheck %s --check-prefixes CHECK,SIMD128-VM
----------------
How about changing filename to `offset-simd.ll` or something, because it looks like other offset-testing tests follow the same convention.


Repository:
  rL LLVM

https://reviews.llvm.org/D51383