[PATCH] D66983: [WebAssembly] Add wasm-specific vector shuffle builtin and intrinsic

[Thomas Lively via Phabricator via cfe-commits]

tlively added a comment.

Ok, this is ready for review for real this time. The WebAssembly SIMD contributors have decided that this is an appropriate direction to go in, and we are leaving the door open for future improvements.



================
Comment at: llvm/include/llvm/IR/IntrinsicsWebAssembly.td:117
+             llvm_i32_ty, llvm_i32_ty, llvm_i32_ty, llvm_i32_ty, llvm_i32_ty,
+             llvm_i32_ty, llvm_i32_ty, llvm_i32_ty, llvm_i32_ty],
+            [IntrNoMem, IntrSpeculatable]>;
----------------
aheejin wrote:
> i32 is bigger than `ImmLaneIdx32`. Should we model this into something smaller, like i8? What happens if we specify an index grater than 31? (I think this question also applies to other intrinsics and builtins. I don't think it matters a lot given than all integers are larger than lane types though.)
It turns out that it would have been an ISel failure. I fixed this to replace an out-of-bounds indices with 0.


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:1251
+    return DAG.getNode(WebAssemblyISD::SHUFFLE, DL, Op.getValueType(), Ops);
+  }
   }
----------------
aheejin wrote:
> This looks rather straightforward... Can't we do this in TableGen?
No, I don't know of a simple way to handle undef lanes in TableGen. I could look into using custom patterns and transform nodes, but in the end this code is probably simpler the way it is.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66983/new/

https://reviews.llvm.org/D66983