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

[Dan Weber via Phabricator via llvm-commits]

dweber added inline comments.


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:1603
+        auto *Const = cast<ConstantSDNode>(Lane.getNode());
+        uint64_t Val = byte_swap(Const->getLimitedValue(), little);
+        uint8_t *ValPtr = reinterpret_cast<uint8_t *>(&Val);
----------------
efriedma wrote:
> I think this byte_swap isn't doing the right thing; it's in the wrong width.
> 
> There are a couple of reasonable ways to write this:
> 
> 1. Construct an `std::array<uint8_t, 16>/std::array<ulittle16_t, 8>/std::array<ulittle32_t, 4>`, and memcpy from it to an `std::array<ulittle64_t, 2>`.
> 2. Don't type-pun at all; something like:
> 
> 
>     for (int i = 0; i < NumElements; ++i)
>       Result[i / (NumElements / 2)] |= Elements[i] << ElementWidth * (i  % (NumElements / 2);
When I wrote this, I checked the implementation of byte swap. It's a template that derives the byte_swap from the value type. In this case, getLimitedValue returns u64. It should be right. It makes me wonder since webassembly is guaranteed to be little endian if a conversion occurs before this happens.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88591