[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