[PATCH] D144619: [WebAssembly] Skip implied bitmask operation in LowerShift

[JunMa via Phabricator via llvm-commits]

junparser added inline comments.


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:2307
+        MaskOp = LHS;
+    } else {
+      if (!isa<ConstantSDNode>(RHS.getNode()))
----------------
tlively wrote:
> Do we have tests that cover the case where the mask op is not a vector?
Yes, the lshr_v2i64_i32_late in ut is. Sometimes, the DAGCombiner will  transform 

```
  %t = insertelement <2 x i64> undef, i64 %z, i32 0
  %s = shufflevector <2 x i64> %t, <2 x i64> undef, <2 x i32> <i32 0, i32 0>
  %m = and <2 x i64> %s, <i64 63, i64 63>
  %a = lshr <2 x i64> %v, %m
```
into  scalar and which saves cost of  BUILD_VECTOR for constant.

Since this is common optimization, so we should handle it here.



================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:2311-2312
+
+      auto ConstantRHS = dyn_cast<ConstantSDNode>(RHS.getNode());
+      if (ConstantRHS->getAPIntValue() == MaskBits)
+        MaskOp = LHS;
----------------
tlively wrote:
> IIUC, there's no guarantee that the cast succeeds, so we have to guard against null here.
sorry, lost it here. Will update.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144619