[PATCH] D50597: [WebAssembly] SIMD extract_lane

[Heejin Ahn via Phabricator via llvm-commits]

aheejin accepted this revision.
aheejin added inline comments.
This revision is now accepted and ready to land.


================
Comment at: lib/Target/WebAssembly/WebAssemblyInstrSIMD.td:34
+                   (i32 (sext_inreg
+                     (i32 (vector_extract
+                       node:$vec,
----------------
tlively wrote:
> 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] })`
Hmm, looks like `extractelt` has [[ https://github.com/llvm-mirror/llvm/blob/e747ac6d9aaaf2d02c3d0a6772285737ea4f08a3/include/llvm/Target/TargetSelectionDAG.td#L241-L243 | `SDVecExtract` ]] has `SDTCisEltOfVec<0, 1>` property that's not in `vector_extract` and it's causing the failure. Looks like, under this property, the result should be the same with the element type of the first argument, which means, `i32 = extractelt i8v16 v imm` wouldn't work, and I guess that's the reason.

Anyway, `vector_extract` is still being used widely in LLVM, so I guess that'd be OK.


================
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))>;
 
----------------
tlively wrote:
> 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.
About what we talked in person: I was wondering if scalar loads would have similar rules, but it turned out [[ https://github.com/llvm-mirror/llvm/blob/35b2115503a69e695b6dd9c47c94332f3e93b975/lib/Target/WebAssembly/WebAssemblyInstrMemory.td#L256-L303 | they do ]]. So nevermind.


Repository:
  rL LLVM

https://reviews.llvm.org/D50597