- Fix the x64 store-barrier.

- Fix the x64 store-barrier.
- Allow assembler test drivers to be shared across architectures.
- Add a shared StoreIntoObject assembler test.
Committed: https://code.google.com/p/dart/source/detail?r=9050

[siva, 2012-06-22 23:02:25]

lgtm

https://chromiumcodereview.appspot.com/10636017/diff/9002/runtime/vm/assemble...
File runtime/vm/assembler_x64.cc (right):

https://chromiumcodereview.appspot.com/10636017/diff/9002/runtime/vm/assemble...
runtime/vm/assembler_x64.cc:1: // Copyright (c) 2011, the Dart project authors. 
Please see the AUTHORS file
2012

https://chromiumcodereview.appspot.com/10636017/diff/9002/runtime/vm/assemble...
File runtime/vm/assembler_x64_test.cc (right):

https://chromiumcodereview.appspot.com/10636017/diff/9002/runtime/vm/assemble...
runtime/vm/assembler_x64_test.cc:1: // Copyright (c) 2011, the Dart project
authors.  Please see the AUTHORS file
Ditto.

https://chromiumcodereview.appspot.com/10636017/diff/9002/runtime/vm/unit_test.h
File runtime/vm/unit_test.h (right):

https://chromiumcodereview.appspot.com/10636017/diff/9002/runtime/vm/unit_tes...
runtime/vm/unit_test.h:1: // Copyright (c) 2011, the Dart project authors. 
Please see the AUTHORS file
Ditto.

[cshapiro, 2012-06-22 23:22:40]

lgtm - with comments

https://chromiumcodereview.appspot.com/10636017/diff/9002/runtime/vm/assemble...
File runtime/vm/assembler_test.cc (right):

https://chromiumcodereview.appspot.com/10636017/diff/9002/runtime/vm/assemble...
runtime/vm/assembler_test.cc:15: ASSEMBLER_TEST_RUN(StoreIntoObject, entry) {
This test only needs to test StoreIntoObjectFilter, not all of StoreIntoObject. 
I think this test code may have predated my factoring of the store code in two. 
Nevertheless, it would be helpful to validate only the behavior of the filter to
avoid breaking when StoreIntoObject acquires additional behavior.

https://chromiumcodereview.appspot.com/10636017/diff/9002/runtime/vm/assemble...
runtime/vm/assembler_test.cc:40: for (int i = -32; i < 32; i++) {
Testing the full range of 8-bit immediate values reachable by the test would
help to catch errors in the event that the filter changes.

https://chromiumcodereview.appspot.com/10636017/diff/9002/runtime/vm/assemble...
File runtime/vm/assembler_x64.cc (right):

https://chromiumcodereview.appspot.com/10636017/diff/9002/runtime/vm/assemble...
runtime/vm/assembler_x64.cc:1414: void Assembler::StoreIntoObjectFilter(Register
object,
Document as destroying value?  Maybe in the header?

https://chromiumcodereview.appspot.com/10636017/diff/9002/runtime/vm/assemble...
runtime/vm/assembler_x64.cc:1417: andl(value, Immediate(0x9));
Can't these values be synthesized from constants?  That would help insulate us
against errors.  0x9 is just a mask composed from the object/smi bit and the
generation bit.

https://chromiumcodereview.appspot.com/10636017/diff/9002/runtime/vm/assemble...
runtime/vm/assembler_x64.cc:1420: andl(value, Immediate(0xf));
ditto

https://chromiumcodereview.appspot.com/10636017/diff/9002/runtime/vm/assemble...
runtime/vm/assembler_x64.cc:1421: cmpl(value, Immediate(0x6));
ditto

[Ivan Posva, 2012-06-22 23:42:21]

https://chromiumcodereview.appspot.com/10636017/diff/9002/runtime/vm/assemble...
File runtime/vm/assembler_test.cc (right):

https://chromiumcodereview.appspot.com/10636017/diff/9002/runtime/vm/assemble...
runtime/vm/assembler_test.cc:40: for (int i = -32; i < 32; i++) {
On 2012/06/22 23:22:40, cshapiro wrote:
> Testing the full range of 8-bit immediate values reachable by the test would
> help to catch errors in the event that the filter changes.
 Changed to test from -128 to 128.

https://chromiumcodereview.appspot.com/10636017/diff/9002/runtime/vm/assemble...
File runtime/vm/assembler_x64.cc (right):

https://chromiumcodereview.appspot.com/10636017/diff/9002/runtime/vm/assemble...
runtime/vm/assembler_x64.cc:1: // Copyright (c) 2011, the Dart project authors. 
Please see the AUTHORS file
On 2012/06/22 23:02:25, asiva wrote:
> 2012

Done.

https://chromiumcodereview.appspot.com/10636017/diff/9002/runtime/vm/assemble...
runtime/vm/assembler_x64.cc:1414: void Assembler::StoreIntoObjectFilter(Register
object,
On 2012/06/22 23:22:40, cshapiro wrote:
> Document as destroying value?  Maybe in the header?

Done.

https://chromiumcodereview.appspot.com/10636017/diff/9002/runtime/vm/assemble...
runtime/vm/assembler_x64.cc:1417: andl(value, Immediate(0x9));
On 2012/06/22 23:22:40, cshapiro wrote:
> Can't these values be synthesized from constants?  That would help insulate us
> against errors.  0x9 is just a mask composed from the object/smi bit and the
> generation bit.

Using the constants would actually be a very nice way of documenting the bit
twiddling. Thanks! Next CL.

https://chromiumcodereview.appspot.com/10636017/diff/9002/runtime/vm/assemble...
File runtime/vm/assembler_x64_test.cc (right):

https://chromiumcodereview.appspot.com/10636017/diff/9002/runtime/vm/assemble...
runtime/vm/assembler_x64_test.cc:1: // Copyright (c) 2011, the Dart project
authors.  Please see the AUTHORS file
On 2012/06/22 23:02:25, asiva wrote:
> Ditto.

Done.

[cshapiro, 2012-06-22 23:46:42]

http://codereview.chromium.org/10636017/diff/9002/runtime/vm/assembler_test.cc
File runtime/vm/assembler_test.cc (right):

http://codereview.chromium.org/10636017/diff/9002/runtime/vm/assembler_test.c...
runtime/vm/assembler_test.cc:15: ASSEMBLER_TEST_RUN(StoreIntoObject, entry) {
FYI: an example of this will be with tracing which may log to the store buffer
even if the filter would have otherwise filtered the store.