Intrinsify writing to Uint8 and Int8 arrays on IA32.

Intrinsify writing to Uint8 and Int8 arrays on IA32.

Committed: https://code.google.com/p/dart/source/detail?r=13703

[cshapiro, 2012-10-08 21:40:41]

http://codereview.chromium.org/11074016/diff/1/runtime/vm/intrinsifier_ia32.cc
File runtime/vm/intrinsifier_ia32.cc (right):

http://codereview.chromium.org/11074016/diff/1/runtime/vm/intrinsifier_ia32.c...
runtime/vm/intrinsifier_ia32.cc:537: __ cmpl(EBX, Immediate(0xFF));
We need to add 128 to EBX for this compare to be semantically correct.

It would be great to add a test for this condition, trying values on either side
of the range to ensure the boundary conditions are correctly accommodated.

http://codereview.chromium.org/11074016/diff/1/runtime/vm/intrinsifier_x64.cc
File runtime/vm/intrinsifier_x64.cc (right):

http://codereview.chromium.org/11074016/diff/1/runtime/vm/intrinsifier_x64.cc...
runtime/vm/intrinsifier_x64.cc:465: Label fall_through;
Still a work in progress?  This should be otherwise identical to the 32-bit
code.

[srdjan, 2012-10-08 21:45:53]

DBC

http://codereview.chromium.org/11074016/diff/1/runtime/vm/intrinsifier_ia32.cc
File runtime/vm/intrinsifier_ia32.cc (right):

http://codereview.chromium.org/11074016/diff/1/runtime/vm/intrinsifier_ia32.c...
runtime/vm/intrinsifier_ia32.cc:526: TestByteArraySetIndex(assembler,
&fall_through);
Please add comments about register content.

http://codereview.chromium.org/11074016/diff/1/runtime/vm/intrinsifier_ia32.c...
runtime/vm/intrinsifier_ia32.cc:565: TestByteArraySetIndex(assembler,
&fall_through);
ditto

[Cutch, 2012-10-12 17:34:26]

Carl- my second patch is waiting for review.

Thanks,
John

[cshapiro, 2012-10-12 23:05:21]

lgtm - srdjan?

https://codereview.chromium.org/11074016/diff/5001/runtime/vm/intrinsifier_ia...
File runtime/vm/intrinsifier_ia32.cc (right):

https://codereview.chromium.org/11074016/diff/5001/runtime/vm/intrinsifier_ia...
runtime/vm/intrinsifier_ia32.cc:541: __ addl(EBX, Immediate(128));
It would be great if we could put these magic numbers somewhere in the
RawInt8Array or Int8Array classes.  I think this is fine for now, but something
we should certainly address when we have the 16 and 32 bit cases.

[srdjan, 2012-10-16 17:56:01]

https://codereview.chromium.org/11074016/diff/5001/tests/standalone/byte_arra...
File tests/standalone/byte_array_test.dart (right):

https://codereview.chromium.org/11074016/diff/5001/tests/standalone/byte_arra...
tests/standalone/byte_array_test.dart:35: // These should eventually throw.
Don't they throw?
You can use Expect.throws(...) to verify.

https://codereview.chromium.org/11074016/diff/5001/tests/standalone/byte_arra...
tests/standalone/byte_array_test.dart:50: // This should eventually throw.
ditto

[srdjan, 2012-10-16 19:46:37]

LGTM since no exception throwing is specified.

[Cutch, 2012-10-16 21:38:57]

On 2012/10/16 19:46:37, srdjan wrote:
> LGTM since no exception throwing is specified.

Ran test suite:

../tools/test.py Test configuration: none_vm_debug_ia32
[06:21 | 100% | + 5852 | -    0]

../tools/test.py -a x64
Test configuration: none_vm_debug_x64
[06:01 | 100% | + 5821 | -    0]