[PATCH] D68527: [WebAssembly] v8x16.swizzle and rewrite BUILD_VECTOR lowering

[Heejin Ahn via Phabricator via llvm-commits]

aheejin added a comment.

- I remember before we had a somewhat complicated logic to calculate the number of bytes of total instructions of each case of the case we use `v128.const` and vs. when we use splats. Don't we need that anymore? Can we make the decision solely based the number of swizzles / consts / and splats?

- Is the performance of `v128.const` better than splats? How is performance of swizzles compared to `v128.const`?



================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:1350
     }
+  };
+
----------------
Would using `count_if` in place of `find_if` be simpler?


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:1384
+  SDValue SwizzleSrc;
+  SDValue SwizzleIndices;
+  size_t NumSwizzleLanes = 0;
----------------
Nit: Variable names for the same things in `GetSwizzleSrcs` are `SrcVec` and `IndexVec`. Making the variable names same in the two places might make reading easier.


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:1388
+    std::forward_as_tuple(std::tie(SwizzleSrc, SwizzleIndices),
+                          NumSwizzleLanes) = GetMostCommon(SwizzleCounts);
+
----------------
Is using `forward_as_tuple` any different from using `tie` again in this case, given that this is not passed as an argument to a function?


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:1426
+        (SplattedLoad = dyn_cast<LoadSDNode>(SplatValue)) &&
+        SplattedLoad->getMemoryVT() == VecT.getVectorElementType()) {
+      Result = DAG.getNode(WebAssemblyISD::LOAD_SPLAT, DL, VecT, SplatValue);
----------------
It's not in this CL, but is there a case this condition is not satisfied?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68527