Minor cleanup code in GenerateStoreField.

src/stub-cache-ia32.cc

 // Copyright 2006-2008 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 383 matching lines @@
   // Perform map transition for the receiver if necessary.
   if ((transition != NULL) && (object->map()->unused_property_fields() == 0)) {
     // The properties must be extended before we can store the value.
     // We jump to a runtime call that extends the propeties array.
     __ mov(ecx, Immediate(Handle<Map>(transition)));
     Handle<Code> ic(Builtins::builtin(storage_extend));
     __ jmp(ic, RelocInfo::CODE_TARGET);
     return;
   }
 
-  // Adjust for the number of properties stored in the object. Even in the
-  // face of a transition we can use the old map here because the size of the
-  // object and the number of in-object properties is not going to change.
-  index -= object->map()->inobject_properties();
-
-  if (index >= 0) {
-    // Get the properties array (optimistically).
-    __ mov(scratch, FieldOperand(receiver_reg, JSObject::kPropertiesOffset));
-  }
-
   if (transition != NULL) {
     // Update the map of the object; no write barrier updating is
     // needed because the map is never in new space.
     __ mov(FieldOperand(receiver_reg, HeapObject::kMapOffset),
            Immediate(Handle<Map>(transition)));
   }
 
+  // Adjust for the number of properties stored in the object. Even in the
+  // face of a transition we can use the old map here because the size of the
+  // object and the number of in-object properties is not going to change.
+  index -= object->map()->inobject_properties();
+
   if (index < 0) {
     // Set the property straight into the object.
     int offset = object->map()->instance_size() + (index * kPointerSize);
     __ mov(FieldOperand(receiver_reg, offset), eax);
 
     // Update the write barrier for the array address.
     // Pass the value being stored in the now unused name_reg.
     __ mov(name_reg, Operand(eax));
     __ RecordWrite(receiver_reg, offset, name_reg, scratch);
   } else {
     // Write to the properties array.
     int offset = index * kPointerSize + Array::kHeaderSize;
+    // Get the properties array (optimistically).
+    __ mov(scratch, FieldOperand(receiver_reg, JSObject::kPropertiesOffset));

[iposva, 2008/11/20 01:10:57]

Moving the load of the properties pointer down here only has a potential
performance impact when this store also happens to have a transition. This
should be negligible, but we should verify this with a benchmark run.

[Feng Qian, 2008/11/20 16:30:47]

Running V8 bench suite (Richards, DeltaBlue, Crypto, Raytrace, and EarleyBoyer),
the differences are tiny on Windows:

3166 vs 3168 (after change).

On 2008/11/20 01:10:57, iposva wrote:

     __ mov(FieldOperand(scratch, offset), eax);
 
     // Update the write barrier for the array address.
     // Pass the value being stored in the now unused name_reg.
     __ mov(name_reg, Operand(eax));
     __ RecordWrite(scratch, offset, name_reg, receiver_reg);
   }
 
   // Return the value (register eax).
   __ ret(0);
@@ skipping 759 matching lines @@
   GenerateLoadMiss(masm(), Code::KEYED_LOAD_IC);
 
   // Return the generated code.
   return GetCode(CALLBACKS);
 }
 
 
 #undef __
 
 } }  // namespace v8::internal