WebAssembly: test_BuilderJSON.js is broken

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[JF Bastien]

Bug #165886 broke it.

I started looking into it, but should do this in a separate patch.

[JF Bastien]

Created attachment 297167 [details]
Partial fix

[Keith Miller]

Created attachment 297170 [details]
Patch

[Mark Lam]

Comment on attachment 297170 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=297170&action=review

> JSTests/wasm/Builder.js:40
> +    case "i32": return Math.round(value) === value && LLB.varint32Min <= value && value <= LLB.varuint32Max;

Is the use of LLB.varuint32Max correct for i32?  Should it be LLB.varint32Max?

[JF Bastien]

Comment on attachment 297170 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=297170&action=review

Thanks! Looks good.

>> JSTests/wasm/Builder.js:40
>> +    case "i32": return Math.round(value) === value && LLB.varint32Min <= value && value <= LLB.varuint32Max;
> 
> Is the use of LLB.varuint32Max correct for i32?  Should it be LLB.varint32Max?

Here we want everything that can be encoded as a 32-bit integer because that's what wasm supports: i32 is sign-agnostic, the operations on some i32 values are sign-aware. We however want to expose this in a convenient manner to JS, so we allow [INT_MIN, UINT_MAX] as the input range. The code below then further restricts inputs which should be positive as needed: the JSON file identifies each opcode's immediate and for e.g. function_index we say "it has to be a positive i32".

So this is correct.

[Michael Saboff]

Comment on attachment 297170 [details]
Patch

r=me

[Keith Miller]

Committed r209863: <http://trac.webkit.org/changeset/209863>

[Ryan Haddad]

*** Bug 165907 has been marked as a duplicate of this bug. ***