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

[Thomas Lively via Phabricator via llvm-commits]

tlively 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:
----------------
aheejin wrote:
> How about adding newlines between categories, like default / bitselect / anytrue+alltrue / lsda ?
sgtm


================
Comment at: lib/Target/WebAssembly/WebAssemblyInstrSIMD.td:466
+            (!cast<Instruction>("BITSELECT_"#vec_t)
+              V128:$v1, V128:$v2, V128:$c)>;
+
----------------
aheejin wrote:
> 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.
yes, thankfully. I rewrote the existing tests so that each one tests a different interesting pattern in addition to a different type.


Repository:
  rL LLVM

https://reviews.llvm.org/D52805