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.