PNaCl: Test support for GCC/LLVM vector extensions

PNaCl: Test support for GCC/LLVM vector extensions

This test is a companion to:
  https://codereview.chromium.org/221693002/

It tests that the expected GCC/LLVM vector extensions work, that
operators are supported as expected, and that the expected types get
properly translated. The test isn't thorough in that it doesn't test all
corner cases and undefined behavior, this will be left up to other tests
in the same directory.

BUG= https://code.google.com/p/nativeclient/issues/detail?id=2205
TEST= ./scons run_vector_extension_test -j32 bitcode=1
R=nfullagar@chromium.org

Committed: https://src.chromium.org/viewvc/native_client?view=rev&revision=13051

[nfullagar1, 2014-04-16 23:04:57]

https://codereview.chromium.org/222483002/diff/20001/tests/simd/vector_extens...
File tests/simd/vector_extension.c (right):

https://codereview.chromium.org/222483002/diff/20001/tests/simd/vector_extens...
tests/simd/vector_extension.c:17: 
can you add a description/goal of what this file is testing?  It appears to be a
combination of does-it-compile & crash/no-crash at runtime.  The nested macros
and use of ## may obfuscate failure cases and make them harder to track down.

https://codereview.chromium.org/222483002/diff/20001/tests/simd/vector_extens...
tests/simd/vector_extension.c:74: typedef F32 VF32
__attribute__((vector_size(VEC_BYTES)));
The names (ie 'I8') chosen here (and elsewhere) will clash once larger or
smaller vector (256 bit, 512, 64...) support becomes available.

[JF, 2014-04-17 00:05:43]

As suggested by Derek I updated pnacl/COMPONENT_REVISIONS to pull in the LLVM
change that adds vector extensions, and use FEATURE_VERSION to allow these tests
to run on the toolchain bots prior to rolling the LLVM change into NaCl.

I kicked off toolchain trybots.

https://codereview.chromium.org/222483002/diff/20001/tests/simd/vector_extens...
File tests/simd/vector_extension.c (right):

https://codereview.chromium.org/222483002/diff/20001/tests/simd/vector_extens...
tests/simd/vector_extension.c:17: 
On 2014/04/16 23:04:57, nfullagar1 wrote:
> can you add a description/goal of what this file is testing?  It appears to be
a
> combination of does-it-compile & crash/no-crash at runtime.  The nested macros
> and use of ## may obfuscate failure cases and make them harder to track down.

Done.

https://codereview.chromium.org/222483002/diff/20001/tests/simd/vector_extens...
tests/simd/vector_extension.c:74: typedef F32 VF32
__attribute__((vector_size(VEC_BYTES)));
On 2014/04/16 23:04:57, nfullagar1 wrote:
> The names (ie 'I8') chosen here (and elsewhere) will clash once larger or
> smaller vector (256 bit, 512, 64...) support becomes available.

Agreed, we'll need to add width to the name once we get there.

[nfullagar1, 2014-04-17 00:32:50]

https://codereview.chromium.org/222483002/diff/60001/tests/simd/vector_extens...
File tests/simd/vector_extension.c (right):

https://codereview.chromium.org/222483002/diff/60001/tests/simd/vector_extens...
tests/simd/vector_extension.c:12: * This test is thorough feature-wise, but not
thorough in testing the
should note: excluding __builtin_convertvector & __builtin_shufflevector.

https://codereview.chromium.org/222483002/diff/60001/tests/simd/vector_extens...
File tests/simd/vector_extension.stdout (right):

https://codereview.chromium.org/222483002/diff/60001/tests/simd/vector_extens...
tests/simd/vector_extension.stdout:36: I16 {1,2,3,4,5,6,7,8,} /
{1,15,14,13,12,11,10,9,} = {1,0,0,0,0,0,0,0,}
above (and elsewhere): a lot of these tests (divide, shift right, >, <= for
example) are producing results that don't really reveal if the implementation is
working or not.

[JF, 2014-04-17 00:48:04]

https://codereview.chromium.org/222483002/diff/60001/tests/simd/vector_extens...
File tests/simd/vector_extension.c (right):

https://codereview.chromium.org/222483002/diff/60001/tests/simd/vector_extens...
tests/simd/vector_extension.c:12: * This test is thorough feature-wise, but not
thorough in testing the
On 2014/04/17 00:32:50, nfullagar1 wrote:
> should note: excluding __builtin_convertvector & __builtin_shufflevector.

Right, I'll add a TODO to add them once we support them.

https://codereview.chromium.org/222483002/diff/60001/tests/simd/vector_extens...
File tests/simd/vector_extension.stdout (right):

https://codereview.chromium.org/222483002/diff/60001/tests/simd/vector_extens...
tests/simd/vector_extension.stdout:36: I16 {1,2,3,4,5,6,7,8,} /
{1,15,14,13,12,11,10,9,} = {1,0,0,0,0,0,0,0,}
On 2014/04/17 00:32:50, nfullagar1 wrote:
> above (and elsewhere): a lot of these tests (divide, shift right, >, <= for
> example) are producing results that don't really reveal if the implementation
is
> working or not.

Agreed, the goal of the test isn't to be thorough on the values, it's merely to
see that the vector extensions pass the ABI checks.

The idea is that *all* vector support in PNaCl will go through the same
primitives, so testing the edge cases for every way that vectors can be used
isn't worth it. What I want to do instead is test that each way to use vectors
(here, the vector extensions) can make it through, and then have a thorough
feature test, which you already started.

I don't want both tests to overlap in testing the results generated by vector
operations.

[JF, 2014-04-17 03:44:06]

All the tryjobs passed (on patch set 4), so everything should be good to go.

Any other suggested edits?

[Derek Schuff, 2014-04-17 16:03:49]

On 2014/04/17 03:44:06, JF wrote:
> All the tryjobs passed (on patch set 4), so everything should be good to go.
> 
> Any other suggested edits?

You probably want to do a regular 'git try' to run the main trybots as well as
the toolchain trybots, just to make sure the feature version check works, or
just use the CQ

[JF, 2014-04-17 16:33:05]

On 2014/04/17 16:03:49, Derek Schuff wrote:
> On 2014/04/17 03:44:06, JF wrote:
> > All the tryjobs passed (on patch set 4), so everything should be good to go.
> > 
> > Any other suggested edits?
> 
> You probably want to do a regular 'git try' to run the main trybots as well as
> the toolchain trybots, just to make sure the feature version check works, or
> just use the CQ

Sent.

[nfullagar1, 2014-04-17 17:26:46]

lgtm

[JF, 2014-04-17 19:38:25]

Committed patchset #5 manually as r13051 (presubmit successful).