[PATCH] D88773: Reland "[WebAssembly] Emulate v128.const efficiently""

Dan Weber via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Oct 10 19:28:01 PDT 2020


dweber added a comment.

In D88773#2323431 <https://reviews.llvm.org/D88773#2323431>, @hubert.reinterpretcast wrote:

> Thanks. This matches what I tested with a big endian host system. I'm not sure if @dweber agrees that their comments have been addressed though.

I'm pretty okay with this change. It took me a minute to wrap my head around  because it wasn't immediately obvious how it was placing the integers inside the 64s, but overall it's fine. The only thing I think is missing is a safety check on the bitwise or operation inside the loop. Before it implicitly extracted the least significant bits by byte size meaning there was no room for a value to exceed range (e.g. bits set above the shift before the or). I spoke with @tlively offline about this, and he said he would add an assertion to make sure the value is in range. With that change, this has my blessing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88773



More information about the llvm-commits mailing list