Generate code for store buffer updates in open-coded object field stores.

runtime/vm/assembler_ia32.cc

Index: runtime/vm/assembler_ia32.cc
diff --git a/runtime/vm/assembler_ia32.cc b/runtime/vm/assembler_ia32.cc
index 9a6c5112f8c1b073b161dd32c56c0e8dfcc3c4a7..db066732e94e9d3522b529195fce005992c31086 100644
--- a/runtime/vm/assembler_ia32.cc
+++ b/runtime/vm/assembler_ia32.cc
@@ -13,6 +13,8 @@
 
 namespace dart {
 
+DECLARE_RUNTIME_ENTRY(StoreBuffer);
+
 DEFINE_FLAG(bool, print_stop_message, true, "Print stop message.");
 DEFINE_FLAG(bool, code_comments, false,
             "Include comments into code and disassembly");
@@ -99,6 +101,18 @@ void Assembler::popl(const Address& address) {
 }
 
 
+void Assembler::pushal() {
+  AssemblerBuffer::EnsureCapacity ensured(&buffer_);
+  EmitUint8(0x60);
+}
+
+
+void Assembler::popal() {
+  AssemblerBuffer::EnsureCapacity ensured(&buffer_);
+  EmitUint8(0x61);
+}
+
+
 void Assembler::movl(Register dst, const Immediate& imm) {
   AssemblerBuffer::EnsureCapacity ensured(&buffer_);
   EmitUint8(0xB8 + dst);
@@ -1364,8 +1378,32 @@ void Assembler::CompareObject(Register reg, const Object& object) {
 void Assembler::StoreIntoObject(Register object,
                                 const FieldAddress& dest,
                                 Register value) {
-  // TODO(iposva): Add write barrier.
   movl(dest, value);
+  Label done;
+  // Check that value is a new object.  Store buffer updates are not

[siva, 2012/06/08 17:26:01]

Check that 'value'

[cshapiro, 2012/06/08 23:48:04]

Fixed.

+  // required when storing a smi or an old object.
+  testl(value, Immediate(kNewObjectAlignmentOffset | kHeapObjectTag));
+  j(NOT_EQUAL, &done, Assembler::kNearJump);
+  // Check that object is an old object.  A store buffer update is

[siva, 2012/06/08 17:26:01]

Check that 'object'

[cshapiro, 2012/06/08 23:48:04]

Fixed.

+  // not required when storing into a new object.
+  testl(object, Immediate(kOldObjectAlignmentOffset | kHeapObjectTag));
+  j(NOT_EQUAL, &done, Assembler::kNearJump);
+  // A store buffer update is required.

[Ivan Posva, 2012/06/08 14:39:55]

I was hoping that you could do the regular store into the per-isolate store
buffer right here and only call the runtime when the store buffer was full. The
easiest way is to store, increment and check for fullness. Then you do not need
to pass any parameter to the runtime call.

[cshapiro, 2012/06/08 23:48:04]

Passing arguments is not a problem.  The runtime call unconditionally trashes 2
registers to setup the call and then native code will trash the rest.  Until we
can ask the code generator to spill live values around such operations, we
cannot generally solve this problem.

I think this makes a very strong case for a card marking scheme instead of the
sequential-store-buffer.  The generated code for card marking is shorter,
requires fewer registers, does not have branches, and does not require runtime
calls or deduplication.

+  pushal();

[srdjan, 2012/06/08 05:46:59]

This is bad idea: stack may contain only objects, and registers may not have
objects. No need to preserve anything, at least not here. Check callers if value
or object needs to be preserved.

[cshapiro, 2012/06/08 23:48:04]

It seems that, thanks to the filter, we can get away with preserving the
arguments to StoreIntoObject.  This may not always be true.

+  pushl(dest);
+  CallRuntime(kStoreBufferRuntimeEntry);
+  Drop(1);
+  popal();
+  Bind(&done);
+}

[siva, 2012/06/08 17:26:01]

Maybe we need two versions of this one for calls from the stub code and one when
called from the code generator.

The stub code version could just save all registers if needed as we will still
be looking for tagged values when iterating stub frames.

The code gen version could do the custom preserve state stuff along with a stack
map etc.

+
+
+void Assembler::StoreNullIntoObject(Register object,
+                                    const FieldAddress& dest) {

[srdjan, 2012/06/08 05:46:59]

Why do you need 'object' ?

[cshapiro, 2012/06/08 23:48:04]

Yes, this code does not make use of object.  However, the liveness-tracing tool
needs to trace all assignments and I would like to retain access to the
destination object in this context.  If persisting this additional register of
state is burdensome to the compiler we should discuss an alternative.

In the meantime, I am working on replacing this call with a general routine for
storing a known old value.  This will check the value for old space residency in
debug mode and unconditionally proceed in release mode.

+  const Immediate raw_null =
+      Immediate(reinterpret_cast<intptr_t>(Object::null()));
+  movl(dest, raw_null);
+  // No store buffer update.
 }