[PATCH] D22686: [WASM] SIMD128 support.

[Derek Schuff via llvm-commits]

dschuff added inline comments.

================
Comment at: lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp:103
@@ -100,1 +102,3 @@
       return T;
+  if (Subtarget->hasSIMD128()) {
+    for (MVT T : {MVT::v16i8, MVT::v8i16, MVT::v4i32, MVT::v4f32})
----------------
XXX Do we want to make this helper unconditionally return the SIMD type and put the Subtarget check elsewhere?

================
Comment at: lib/Target/WebAssembly/WebAssemblyInstrCall.td:38
@@ +37,3 @@
+multiclass SIMD_CALL<ValueType vt, string prefix> {
+  let Predicates = [HasSIMD128] in {
+  def CALL_#vt : I<(outs V128:$dst), (ins i32imm:$callee, variable_ops),
----------------
Should the predicate go outside the multiclass?

================
Comment at: lib/Target/WebAssembly/WebAssemblyInstrSIMD.td:15
@@ -14,3 +14,3 @@
 
-// TODO: Implement SIMD instructions.
-// Note: use Requires<[HasSIMD128]>.
+let Predicates = [HasSIMD128] in {
+let isCommutable = 1 in {
----------------
Actually, I'm a bit unclear about Predicates [HasSIMD128] vs Requires<[HasSIMD128]>. Why do we need both here?

================
Comment at: lib/Target/WebAssembly/WebAssemblyRegNumbering.cpp:78
@@ +77,3 @@
+      if (!Subtarget.hasSIMD128())
+        break;
+    // fallthrough intended.
----------------
Should this be an assert instead?


Repository:
  rL LLVM

https://reviews.llvm.org/D22686