[PATCH] D50597: [WebAssembly] SIMD extract_lane

[Thomas Lively via Phabricator via llvm-commits]

tlively marked an inline comment as done.
tlively added inline comments.


================
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#";">;
+
----------------
aheejin wrote:
> Question: How does `"#SIZE#"` turn it into a number?
`#` is like a superpowered strconcat in TableGen. In this case it casts `SIZE` from an int to a string then appends it to the `LaneIdx` identifier.


================
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,
----------------
aheejin wrote:
> Is a default value on an argument meaningful when some of subsequent parameters don't have defaults?
Nope, this slipped through a refactoring. Thanks!


================
Comment at: lib/Target/WebAssembly/WebAssemblyInstrSIMD.td:34
+                   (i32 (sext_inreg
+                     (i32 (vector_extract
+                       node:$vec,
----------------
aheejin wrote:
> [[ 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.
It doesn't seem to be a drop-in replacement, though. When I replace `vector_extract` with `extractelt` I get a TableGen compilation error: `Type set is empty for each HW mode: possible type contradiction in the pattern below`. The pattern is `EXTRACT_LANE_S_I8x16: 	(sext_inreg:{ *:[i32] } (extractelt:{ *:[] } V128:{ *:[] }:$vec, (imm:{ *:[i32] })<<P:Predicate_LaneIdx16>>:$idx), i8:{ *:[Other] })`


================
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))>;
 
----------------
aheejin wrote:
> If we don't have these, do they not get selected?
That's correct. Without these patterns you just get a selection error without an explicit extension.


Repository:
  rL LLVM

https://reviews.llvm.org/D50597