Subzero - WASM: Codegen fixes, better test infrastructure

Subzero - WASM: Codegen fixes, better test infrastructure

Fixes several bugs in code generation, including handling of booleans,
comparisons and shifts. The tests that get through code generation now
run successfully (except for the tests that are known to fail on
https://wasm-stat.us/).

This change also includes improvements to the test infrastructure. The
wasm test runner has a list of expected failures to skip. The tests
now run in parallel, which significantly cuts down the time to run the
whole test suite.

Finally, there are some minor improvements to the WASM runtime,
including an implementation of syscall20, i.e. getpid().

BUG= https://bugs.chromium.org/p/nativeclient/issues/detail?id=4369
R=kschimpf@google.com, stichnot@chromium.org

Committed: https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-subzero.git;a=commit;h=29acb5751ac1ce388a20569ee54a7ad4cacdb536

[Eric Holk, 2016-04-20 18:44:15]

eholk@chromium.org changed reviewers:
+ jpp@chromium.org, kschimpf@chromium.org, sehr@chromium.org,
stichnot@chromium.org

[John, 2016-04-20 19:18:14]

https://codereview.chromium.org/1900213002/diff/60001/runtime/wasm-runtime.cpp
File runtime/wasm-runtime.cpp (right):

https://codereview.chromium.org/1900213002/diff/60001/runtime/wasm-runtime.cp...
runtime/wasm-runtime.cpp:15: // TODO (eholk): port wasm-runtime.c into this
file, using C++ name mangling.
for the time being, why don't you move the contents in that file to this one?
you can even try the change by doing

extern "C" {
#include "wasm-runtime.c"
}

[Eric Holk, 2016-04-20 19:51:58]

https://codereview.chromium.org/1900213002/diff/60001/runtime/wasm-runtime.cpp
File runtime/wasm-runtime.cpp (right):

https://codereview.chromium.org/1900213002/diff/60001/runtime/wasm-runtime.cp...
runtime/wasm-runtime.cpp:15: // TODO (eholk): port wasm-runtime.c into this
file, using C++ name mangling.
On 2016/04/20 19:18:13, John wrote:
> for the time being, why don't you move the contents in that file to this one?
> you can even try the change by doing
> 
> extern "C" {
> #include "wasm-runtime.c"
> }

Done.

[John, 2016-04-20 20:04:59]

https://codereview.chromium.org/1900213002/diff/80001/runtime/wasm-runtime.cpp
File runtime/wasm-runtime.cpp (right):

https://codereview.chromium.org/1900213002/diff/80001/runtime/wasm-runtime.cp...
runtime/wasm-runtime.cpp:168: }
maybe

} // end of extern "C"

Check with Jim.

https://codereview.chromium.org/1900213002/diff/80001/src/WasmTranslator.cpp
File src/WasmTranslator.cpp (right):

https://codereview.chromium.org/1900213002/diff/80001/src/WasmTranslator.cpp#...
src/WasmTranslator.cpp:647: InstFcmp::create(Func, InstFcmp::Ult, TmpDest, Left,
Right));
Why did you pick unordered comparisons?

https://codereview.chromium.org/1900213002/diff/80001/src/WasmTranslator.cpp#...
src/WasmTranslator.cpp:755: const auto FnName =
Ctx->getGlobalString("_ZN3env5floorEf");
I would use the same name mangling you've been using so far, i.e.,

env$$floor

so that, if and when we start using another name mangling, we fix all instances
of the previous standard.

https://codereview.chromium.org/1900213002/diff/80001/src/WasmTranslator.cpp#...
src/WasmTranslator.cpp:1335: constexpr SizeT BUFFER_SIZE = 256 << 10;
s/BUFFER_SIZE/BufferSize/g

(thanks to the ALCS (Awful LLVM Coding Style))

[Eric Holk, 2016-04-20 20:25:36]

https://codereview.chromium.org/1900213002/diff/80001/runtime/wasm-runtime.cpp
File runtime/wasm-runtime.cpp (right):

https://codereview.chromium.org/1900213002/diff/80001/runtime/wasm-runtime.cp...
runtime/wasm-runtime.cpp:168: }
On 2016/04/20 20:04:59, John wrote:
> maybe
> 
> } // end of extern "C"
> 
> Check with Jim.

Done.

https://codereview.chromium.org/1900213002/diff/80001/src/WasmTranslator.cpp
File src/WasmTranslator.cpp (right):

https://codereview.chromium.org/1900213002/diff/80001/src/WasmTranslator.cpp#...
src/WasmTranslator.cpp:647: InstFcmp::create(Func, InstFcmp::Ult, TmpDest, Left,
Right));
On 2016/04/20 20:04:59, John wrote:
> Why did you pick unordered comparisons?

The last sentence of
https://github.com/WebAssembly/design/blob/master/AstSemantics.md#floating-po...
says "the operands are unordered if either operand is NaN and ordered otherwise.
This seems to match what http://llvm.org/docs/LangRef.html#id287 says unordered
comparisons mean.

https://codereview.chromium.org/1900213002/diff/80001/src/WasmTranslator.cpp#...
src/WasmTranslator.cpp:755: const auto FnName =
Ctx->getGlobalString("_ZN3env5floorEf");
On 2016/04/20 20:04:59, John wrote:
> I would use the same name mangling you've been using so far, i.e.,
> 
> env$$floor
> 
> so that, if and when we start using another name mangling, we fix all
instances
> of the previous standard.

Done.

https://codereview.chromium.org/1900213002/diff/80001/src/WasmTranslator.cpp#...
src/WasmTranslator.cpp:1335: constexpr SizeT BUFFER_SIZE = 256 << 10;
On 2016/04/20 20:04:59, John wrote:
> s/BUFFER_SIZE/BufferSize/g
> 
> (thanks to the ALCS (Awful LLVM Coding Style))

Oh, that's really terrible. But, done.

[Eric Holk, 2016-04-21 17:52:45]

Description was changed from

==========
Subzero - WASM: Codegen fixes, better test infrastructure

Fixes several bugs in code generation, including handling of booleans,
comparisons and shifts. The tests that get through code generation now
run successfully (except for the tests that are known to fail on
https://wasm-stat.us/).

This change also includes improvements to the test infrastructure. The
wasm test runner has a list of expected failures to skip. The tests
now run in parallel, which significantly cuts down the time to run the
whole test suite.

Finally, there are some minor improvements to the WASM runtime,
including an implementation of syscall20, i.e. getpid().

BUG= https://bugs.chromium.org/p/nativeclient/issues/detail?id=4369
==========

to

==========
Subzero - WASM: Codegen fixes, better test infrastructure

Fixes several bugs in code generation, including handling of booleans,
comparisons and shifts. The tests that get through code generation now
run successfully (except for the tests that are known to fail on
https://wasm-stat.us/).

This change also includes improvements to the test infrastructure. The
wasm test runner has a list of expected failures to skip. The tests
now run in parallel, which significantly cuts down the time to run the
whole test suite.

Finally, there are some minor improvements to the WASM runtime,
including an implementation of syscall20, i.e. getpid().

BUG= https://bugs.chromium.org/p/nativeclient/issues/detail?id=4369
==========

[Eric Holk, 2016-04-21 17:52:45]

eholk@chromium.org changed reviewers:
+ kschimpf@google.com
- kschimpf@chromium.org

[Karl, 2016-04-21 19:14:17]

Otherwise LGTM.

https://codereview.chromium.org/1900213002/diff/100001/pydir/wasm-run-torture...
File pydir/wasm-run-torture-tests.py (right):

https://codereview.chromium.org/1900213002/diff/100001/pydir/wasm-run-torture...
pydir/wasm-run-torture-tests.py:175: print('\033[1;31m[compile fail]\033[1;m',
file=out)
Are these parenthesis necessary? (More instances below).

https://codereview.chromium.org/1900213002/diff/100001/src/WasmTranslator.cpp
File src/WasmTranslator.cpp (right):

https://codereview.chromium.org/1900213002/diff/100001/src/WasmTranslator.cpp...
src/WasmTranslator.cpp:331: // Multiply by 10 in case more things get added
later.
Should 10 be 200?

[Eric Holk, 2016-04-21 21:56:02]

https://codereview.chromium.org/1900213002/diff/100001/pydir/wasm-run-torture...
File pydir/wasm-run-torture-tests.py (right):

https://codereview.chromium.org/1900213002/diff/100001/pydir/wasm-run-torture...
pydir/wasm-run-torture-tests.py:175: print('\033[1;31m[compile fail]\033[1;m',
file=out)
On 2016/04/21 19:14:17, Karl wrote:
> Are these parenthesis necessary? (More instances below).

They are because I imported print_function from __future__. I did that because I
prefer the new print function's way of printing to a file other than stdout.

https://codereview.chromium.org/1900213002/diff/100001/src/WasmTranslator.cpp
File src/WasmTranslator.cpp (right):

https://codereview.chromium.org/1900213002/diff/100001/src/WasmTranslator.cpp...
src/WasmTranslator.cpp:331: // Multiply by 10 in case more things get added
later.
On 2016/04/21 19:14:17, Karl wrote:
> Should 10 be 200?

Done.

[Jim Stichnoth, 2016-04-22 00:26:36]

lgtm

https://codereview.chromium.org/1900213002/diff/120001/src/WasmTranslator.cpp
File src/WasmTranslator.cpp (right):

https://codereview.chromium.org/1900213002/diff/120001/src/WasmTranslator.cpp...
src/WasmTranslator.cpp:465: const SizeT BitCount = typeWidthInBytes(DestTy) * 8;
CHAR_BIT

https://codereview.chromium.org/1900213002/diff/120001/src/WasmTranslator.cpp...
src/WasmTranslator.cpp:491: const SizeT BitCount = typeWidthInBytes(DestTy) * 8;
CHAR_BIT

[Eric Holk, 2016-04-22 16:34:44]

Description was changed from

==========
Subzero - WASM: Codegen fixes, better test infrastructure

Fixes several bugs in code generation, including handling of booleans,
comparisons and shifts. The tests that get through code generation now
run successfully (except for the tests that are known to fail on
https://wasm-stat.us/).

This change also includes improvements to the test infrastructure. The
wasm test runner has a list of expected failures to skip. The tests
now run in parallel, which significantly cuts down the time to run the
whole test suite.

Finally, there are some minor improvements to the WASM runtime,
including an implementation of syscall20, i.e. getpid().

BUG= https://bugs.chromium.org/p/nativeclient/issues/detail?id=4369
==========

to

==========
Subzero - WASM: Codegen fixes, better test infrastructure

Fixes several bugs in code generation, including handling of booleans,
comparisons and shifts. The tests that get through code generation now
run successfully (except for the tests that are known to fail on
https://wasm-stat.us/).

This change also includes improvements to the test infrastructure. The
wasm test runner has a list of expected failures to skip. The tests
now run in parallel, which significantly cuts down the time to run the
whole test suite.

Finally, there are some minor improvements to the WASM runtime,
including an implementation of syscall20, i.e. getpid().

BUG= https://bugs.chromium.org/p/nativeclient/issues/detail?id=4369
R=kschimpf@google.com, stichnot@chromium.org

Committed:
https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-subzero.git;a...
==========

[Eric Holk, 2016-04-22 16:34:45]

Committed patchset #7 (id:120001) manually as
29acb5751ac1ce388a20569ee54a7ad4cacdb536 (presubmit successful).

[Eric Holk, 2016-04-22 16:34:56]

https://codereview.chromium.org/1900213002/diff/120001/src/WasmTranslator.cpp
File src/WasmTranslator.cpp (right):

https://codereview.chromium.org/1900213002/diff/120001/src/WasmTranslator.cpp...
src/WasmTranslator.cpp:465: const SizeT BitCount = typeWidthInBytes(DestTy) * 8;
On 2016/04/22 00:26:36, stichnot wrote:
> CHAR_BIT

Done.

https://codereview.chromium.org/1900213002/diff/120001/src/WasmTranslator.cpp...
src/WasmTranslator.cpp:491: const SizeT BitCount = typeWidthInBytes(DestTy) * 8;
On 2016/04/22 00:26:36, stichnot wrote:
> CHAR_BIT

Done.