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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to