[PATCH] D51765: [WebAssembly] SIMD comparisons

[Thomas Lively via Phabricator via llvm-commits]

tlively added inline comments.


================
Comment at: lib/Target/WebAssembly/WebAssemblyInstrSIMD.td:149
+multiclass SIMDConditionImpl<ValueType vec_t, ValueType out_t, string vec,
+                             string name, CondCode cond, bits<32> simdop> {
+  defm _#vec_t :
----------------
aheejin wrote:
> When do you use `simdop` and when do you use `baseInst`? Do you use it interchangeably? If so we should use the same variable name I guess. (In previous patches too.)
I try to use `baseInst` when the multiclass will add constants to it to create new opcodes and  `simdop` when it will be used as-is. I will go through and make sure this usage is consistent. I would also be fine with changing them all to `simdop` if you think that would make more sense.


================
Comment at: test/CodeGen/WebAssembly/comparisons_simd.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
----------------
aheejin wrote:
> How about renaming this to `simd-comparisons.ll` to match other file name conventions you've added so far?
I named it this way to be consistent with `comparisons_f32.ll`, `comparisons_i32.ll` and friends. Those files should probably be renamed for consistency in a separate CL as well. What do you think of `comparisons-i32.ll`, `comparisons-simd.ll`, etc?


Repository:
  rL LLVM

https://reviews.llvm.org/D51765