Bug 165883 – WebAssembly: Add various low hanging fruit that will allow us to run the LLVM torture tests in Wasm

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Saam Barati]

...

[Saam Barati]

Created attachment 297152 [details]
patch

[WebKit Commit Bot]

Attachment 297152 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp:455:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp:461:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/wasm/WasmModuleParser.cpp:516:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
ERROR: Source/JavaScriptCore/wasm/WasmModuleParser.cpp:523:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
ERROR: Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.cpp:132:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
ERROR: Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.cpp:140:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 6 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[JF Bastien]

Comment on attachment 297152 [details]
patch

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

Looks good otherwise.

> JSTests/wasm/Builder.js:196
> +        assert.isNumber(index, `Memory exports only support number indices right now`);

FIXME + bug?

> JSTests/wasm/js-api/table.js:93
> +    assertBadBinary(builder, badExportString);

I'd really rather use assert.throws, I have to update the other tests to it right now.

> JSTests/wasm/js-api/test_memory.js:156
> +    return;

?

[Saam Barati]

Comment on attachment 297152 [details]
patch

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

>> JSTests/wasm/js-api/test_memory.js:156
>> +    return;
> 
> ?

This should be removed.

[Keith Miller]

Comment on attachment 297152 [details]
patch

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

r=me.

> JSTests/wasm/js-api/table.js:279
> +    assert.truthy(table === instance.exports.table);
> +    assert.truthy(table === instance.exports.table2);

can't you do assert.eq?

>> JSTests/wasm/js-api/test_memory.js:156
>> +    return;
> 
> ?

Test passes!

> Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp:462
> +        Value* partialResult = m_currentBlock->appendNew<Value>(m_proc, BitAnd, Origin(), 
> +                value,
> +                m_currentBlock->appendNew<Const32Value>(m_proc, Origin(), 0x0000ffff));

Nit, I would do this as:

Value* partialResult = m_currentBlock->appendNew<Value>(m_proc, BitAnd, Origin(), value,
    m_currentBlock->appendNew<Const32Value>(m_proc, Origin(), 0x0000ffff));

[Saam Barati]

landed in:
https://trac.webkit.org/changeset/209850

[Saam Barati]

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

[Saam Barati]

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

[Saam Barati]

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