[PATCH] D88591: [WebAssembly] Emulate v128.const efficiently

[Thomas Lively via Phabricator via llvm-commits]

tlively added inline comments.


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:1586
+    // Otherwise, if this is an integer vector, construct the constant from a
+    // pair of i64s.
+    std::array<uint64_t, 2> I64s({0, 0});
----------------
aheejin wrote:
> It'd be easier to read to have a little more comments here on what we are trying to do and why it is always better than the simpler splatting-and-replacing option which already existed. Something like [[ https://github.com/emscripten-core/emscripten/issues/12224#issuecomment-698048533 | this ]]. (Also it might be a good idea to link this issue in the CL/commit description for full reference).
 Sounds good. I will elaborate here.


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:1601
+        auto *Const = cast<ConstantSDNode>(Lane.getNode());
+        uint64_t Val = byte_swap(Const->getLimitedValue(), little);
+        uint8_t *ValPtr = reinterpret_cast<uint8_t *>(&Val);
----------------
aheejin wrote:
> What does `getLimitedValue` do and why is it necessary?
It gets the constant value, capping it so that it fits in a `uint64_t`. It's similar to `getZExtValue`, but doesn't assert if the value happens to be too large to fit in a uint64_t for some reason.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88591