[PATCH] D44034: Reland "[WebAssembly] More uses of uint8_t for single byte values"
Sam Clegg via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 2 13:40:17 PST 2018
sbc100 added a comment.
In https://reviews.llvm.org/D44034#1025945, @ncw wrote:
> LGTM.
>
> The assert came from me, and it was carefully checking the letter and the spirit of the current spec!
>
> I'm not entirely convinced by the change overall - I personally thought it was good to use SLEB before, rather than uint8_t. There's a note in the official spec which makes it very clear "these are really SLEB values in intent", so changing the implementation to uint8_t seems a bit backwards. Yes it matches the official text, but not the official direction of the spec. We're only going to have to put it back to SLEB sometime in the future.
>
> Oh well, it should be a safe change in any case.
I've been making these changes across the board, both in llvm and in wabt. I found it rather strange that the byte constants in the spec did not match those in the code. Also, the spec specificlly says they are single byte values (for now anyway).
Repository:
rL LLVM
https://reviews.llvm.org/D44034
More information about the llvm-commits
mailing list