[PATCH] D50597: [WebAssembly] SIMD extract_lane

[Heejin Ahn via Phabricator via llvm-commits]

aheejin added a comment.

Looks good



================
Comment at: lib/Target/WebAssembly/WebAssemblyInstrSIMD.td:18
+foreach SIZE = [2, 4, 8, 16, 32] in
+def LaneIdx#SIZE : ImmLeaf<i32, "return 0 <= Imm && Imm < "#SIZE#";">;
+
----------------
Question: How does `"#SIZE#"` turn it into a number?


================
Comment at: lib/Target/WebAssembly/WebAssemblyInstrSIMD.td:22
+multiclass ExtractLane<ValueType vec_t, ImmLeaf imm_t,
+                       WebAssemblyRegClass reg_t = I32,
+                       string name, bits<32> simdop,
----------------
Is a default value on an argument meaningful when some of subsequent parameters don't have defaults?


================
Comment at: lib/Target/WebAssembly/WebAssemblyInstrSIMD.td:34
+                   (i32 (sext_inreg
+                     (i32 (vector_extract
+                       node:$vec,
----------------
[[ https://github.com/llvm-mirror/llvm/blob/a297d9641672dda1e268ec23d83387a4d42042ff/include/llvm/Target/TargetSelectionDAG.td#L532-L533 | Looks like ]] `vector_extract` is deprecated and [[ https://github.com/llvm-mirror/llvm/blob/a297d9641672dda1e268ec23d83387a4d42042ff/include/llvm/Target/TargetSelectionDAG.td#L404 | `extractelt` ]] is preferred. For other uses as well.


================
Comment at: lib/Target/WebAssembly/WebAssemblyInstrSIMD.td:76
+def : Pat<(i32 (vector_extract (v8i16 V128:$vec), (i32 LaneIdx8:$idx))),
+          (EXTRACT_LANE_U_I16x8 V128:$vec, (i32 LaneIdx8:$idx))>;
 
----------------
If we don't have these, do they not get selected?


Repository:
  rL LLVM

https://reviews.llvm.org/D50597