[PATCH] D52805: [WebAssembly] Bitselect intrinsic and instruction

[Heejin Ahn via Phabricator via llvm-commits]

aheejin added inline comments.


================
Comment at: lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:971
     return DAG.getNode(OpCode, DL, Op.getValueType(), Op.getOperand(1));
   }
   case Intrinsic::wasm_lsda:
----------------
How about adding newlines between categories, like default / bitselect / anytrue+alltrue / lsda ?


================
Comment at: lib/Target/WebAssembly/WebAssemblyInstrSIMD.td:203
+    SIMD_I<(outs V128:$dst), (ins V128:$v1, V128:$v2, V128:$c), (outs), (ins),
+           [(set (vec_t V128:$dst), (vec_t (wasm_bitselect
+             (vec_t V128:$c), (vec_t V128:$v1), (vec_t V128:$v2))
----------------
Maybe start `(vec_t ...` from the next line?


================
Comment at: lib/Target/WebAssembly/WebAssemblyInstrSIMD.td:466
+            (!cast<Instruction>("BITSELECT_"#vec_t)
+              V128:$v1, V128:$v2, V128:$c)>;
+
----------------
Does this handle the case of all different commutative permutations of `or` and `and`? like,
- `(v1 & c) | (v2 & ~c)`
- `(v1 & c) | (~c & v2)`
- `(c & v1) | (~c & v2)`
- `(c & v1) | (v2 & ~c)`
- `(v2 & ~c) | (v1 & c)`
- `(~c & v2) | (v1 & c)`
- `(~c & v2) | (c & v1)`
- `(v2 & ~c) | (c & v1)`

It would be also good to add tests for some of other patterns too.


Repository:
  rL LLVM

https://reviews.llvm.org/D52805