Reviewers: Toon Verwaest,

Description:
HStoreNamedField for Smis optimized for x64

Please review this at https://codereview.chromium.org/108413003/

SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge

Affected files (+45, -7 lines):
  M src/hydrogen-instructions.h
  M src/hydrogen.cc
  M src/objects-inl.h
  M src/x64/lithium-codegen-x64.cc


Index: src/hydrogen-instructions.h
diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h
index 23dbbd289c6c5d5a9e07697368e9fba6ab05e22d..71cd70cd89027e86cdec4a3567c3073898582752 100644
--- a/src/hydrogen-instructions.h
+++ b/src/hydrogen-instructions.h
@@ -6532,8 +6532,12 @@ class HStoreNamedField V8_FINAL : public HTemplateInstruction<3> {
           field_representation().IsUInteger16() ||
           field_representation().IsInteger32()) {
         return Representation::Integer32();
-      } else if (field_representation().IsDouble() ||
-                 field_representation().IsSmi()) {
+      } else if (field_representation().IsDouble()) {
+        return field_representation();
+      } else if (field_representation().IsSmi()) {
+        if (SmiValuesAre32Bits()) {
+          return Representation::Integer32();
+        }
         return field_representation();
       } else if (field_representation().IsExternal()) {
         return Representation::External();
@@ -6561,6 +6565,11 @@ class HStoreNamedField V8_FINAL : public HTemplateInstruction<3> {
   HValue* new_space_dominator() const { return new_space_dominator_; }
   bool has_transition() const { return has_transition_; }

+  // Controls whether it is guaranteed that the smi word already contains
+  // correct smi tag (currently applicable only for x64).
+  bool can_omit_smi_tag_store() const { return can_omit_smi_tag_store_; }
+  void set_can_omit_smi_tag_store(bool v) { can_omit_smi_tag_store_ = v; }
+
   Handle<Map> transition_map() const {
     if (has_transition()) {
       return Handle<Map>::cast(
@@ -6614,7 +6623,8 @@ class HStoreNamedField V8_FINAL : public HTemplateInstruction<3> {
       : access_(access),
         new_space_dominator_(NULL),
         write_barrier_mode_(UPDATE_WRITE_BARRIER),
-        has_transition_(false) {
+        has_transition_(false),
+        can_omit_smi_tag_store_(false) {
     SetOperandAt(0, obj);
     SetOperandAt(1, val);
     SetOperandAt(2, obj);
@@ -6625,6 +6635,7 @@ class HStoreNamedField V8_FINAL : public HTemplateInstruction<3> {
   HValue* new_space_dominator_;
   WriteBarrierMode write_barrier_mode_ : 1;
   bool has_transition_ : 1;
+  bool can_omit_smi_tag_store_ : 1;
 };


Index: src/hydrogen.cc
diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index 8dbecac1191d49440fd755860a559cd71b75e2a5..b4be3fbb134832c6750937eedd50c59f873d0b9d 100644
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -5275,6 +5275,8 @@ HInstruction* HOptimizedGraphBuilder::BuildStoreNamedField(
     // TODO(fschneider): Record the new map type of the object in the IR to
     // enable elimination of redundant checks after the transition store.
     instr->SetGVNFlag(kChangesMaps);
+  } else {
+    instr->set_can_omit_smi_tag_store(true);
   }
   return instr;
 }
@@ -9475,7 +9477,7 @@ void HOptimizedGraphBuilder::BuildEmitInObjectProperties(
       Add<HStoreNamedField>(object, access, result);
     } else {
       Representation representation = details.representation();
-      HInstruction* value_instruction = Add<HConstant>(value);
+      HInstruction* value_instruction;

       if (representation.IsDouble()) {
         // Allocate a HeapNumber box and store the value into it.
@@ -9490,8 +9492,12 @@ void HOptimizedGraphBuilder::BuildEmitInObjectProperties(
         AddStoreMapConstant(double_box,
             isolate()->factory()->heap_number_map());
Add<HStoreNamedField>(double_box, HObjectAccess::ForHeapNumberValue(),
-            value_instruction);
+                              Add<HConstant>(value));
         value_instruction = double_box;
+      } else if (representation.IsSmi() && value->IsUninitialized()) {
+        value_instruction = graph()->GetConstant0();
+      } else {
+        value_instruction = Add<HConstant>(value);
       }

       Add<HStoreNamedField>(object, access, value_instruction);
Index: src/objects-inl.h
diff --git a/src/objects-inl.h b/src/objects-inl.h
index 1717a5f4b59a8cba4db473b0e0634e9c9c437b93..47e8970603182cc9a48627791979188d4e9ab663 100644
--- a/src/objects-inl.h
+++ b/src/objects-inl.h
@@ -267,6 +267,9 @@ bool Object::HasValidElements() {

 MaybeObject* Object::AllocateNewStorageFor(Heap* heap,
                                            Representation representation) {
+  if (FLAG_track_fields && representation.IsSmi() && IsUninitialized()) {
+    return Smi::FromInt(0);
+  }
   if (!FLAG_track_double_fields) return this;
   if (!representation.IsDouble()) return this;
   if (IsUninitialized()) {
Index: src/x64/lithium-codegen-x64.cc
diff --git a/src/x64/lithium-codegen-x64.cc b/src/x64/lithium-codegen-x64.cc
index 7c9949a1fad8da5b441e7ecd79d96ab41dbea073..902cea1669e29e0527f14d6a5f95eeb270fe0948 100644
--- a/src/x64/lithium-codegen-x64.cc
+++ b/src/x64/lithium-codegen-x64.cc
@@ -3946,7 +3946,8 @@ void LCodeGen::DoStoreNamedField(LStoreNamedField* instr) {
   if (FLAG_track_fields && representation.IsSmi()) {
     if (instr->value()->IsConstantOperand()) {
LConstantOperand* operand_value = LConstantOperand::cast(instr->value());
-      if (!IsSmiConstant(operand_value)) {
+      if (!IsInteger32Constant(operand_value) &&
+          !IsSmiConstant(operand_value)) {
         DeoptimizeIf(no_condition, instr->environment());
       }
     }
@@ -4001,6 +4002,19 @@ void LCodeGen::DoStoreNamedField(LStoreNamedField* instr) { __ movq(write_register, FieldOperand(object, JSObject::kPropertiesOffset));
   }

+  bool kScratchRegisterContainsZero = false;
+  if (representation.IsSmi()) {
+    if (!instr->hydrogen()->can_omit_smi_tag_store()) {
+ // Clear the smi tag as we are not sure that it contains the right value.
+      __ xorl(kScratchRegister, kScratchRegister);
+      kScratchRegisterContainsZero = true;
+      __ movl(FieldOperand(write_register, offset), kScratchRegister);
+    }
+    // Deal with the upper part of the smi containing the value.
+    offset += kPointerSize / 2;
+    representation = Representation::Integer32();
+  }
+
   if (instr->value()->IsConstantOperand()) {
LConstantOperand* operand_value = LConstantOperand::cast(instr->value());
     if (operand_value->IsRegister()) {
@@ -4009,7 +4023,11 @@ void LCodeGen::DoStoreNamedField(LStoreNamedField* instr) {
     } else if (representation.IsInteger32()) {
       int32_t value = ToInteger32(operand_value);
       ASSERT(!instr->hydrogen()->NeedsWriteBarrier());
-      __ movl(FieldOperand(write_register, offset), Immediate(value));
+      if ((value == 0) && kScratchRegisterContainsZero) {
+        __ movl(FieldOperand(write_register, offset), kScratchRegister);
+      } else {
+        __ movl(FieldOperand(write_register, offset), Immediate(value));
+      }
     } else {
       Handle<Object> handle_value = ToHandle(operand_value);
       ASSERT(!instr->hydrogen()->NeedsWriteBarrier());


--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.

Reply via email to