Revision: 11774
Author: [email protected]
Date: Tue Jun 12 05:16:19 2012
Log: Eliminate redundant smi checks
[email protected]
Review URL: https://chromiumcodereview.appspot.com/10543094
http://code.google.com/p/v8/source/detail?r=11774
Added:
/branches/bleeding_edge/test/mjsunit/fast-array-length.js
Modified:
/branches/bleeding_edge/src/arm/lithium-arm.cc
/branches/bleeding_edge/src/arm/lithium-codegen-arm.cc
/branches/bleeding_edge/src/hydrogen-instructions.cc
/branches/bleeding_edge/src/hydrogen-instructions.h
/branches/bleeding_edge/src/hydrogen.cc
/branches/bleeding_edge/src/ia32/lithium-codegen-ia32.cc
/branches/bleeding_edge/src/ia32/lithium-ia32.cc
/branches/bleeding_edge/src/ia32/macro-assembler-ia32.cc
/branches/bleeding_edge/src/x64/lithium-codegen-x64.cc
/branches/bleeding_edge/src/x64/lithium-x64.cc
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/fast-array-length.js Tue Jun 12
05:16:19 2012
@@ -0,0 +1,37 @@
+// Copyright 2012 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
+// with the distribution.
+// * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived
+// from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Flags: --allow-natives-syntax
+
+// This is a regression test for overlapping key and value registers.
+
+
+var a = [0, 1, 2, 3, 4, 5];
+assertTrue(%HasFastSmiElements(a));
+a.length = (1 << 30);
+assertFalse(%HasFastSmiElements(a));
+
=======================================
--- /branches/bleeding_edge/src/arm/lithium-arm.cc Mon Jun 11 05:42:31 2012
+++ /branches/bleeding_edge/src/arm/lithium-arm.cc Tue Jun 12 05:16:19 2012
@@ -1710,10 +1710,9 @@
} else {
ASSERT(to.IsInteger32());
LOperand* value = UseRegisterAtStart(instr->value());
- bool needs_check = !instr->value()->type().IsSmi();
LInstruction* res = NULL;
- if (!needs_check) {
- res = DefineAsRegister(new(zone()) LSmiUntag(value, needs_check));
+ if (instr->value()->type().IsSmi()) {
+ res = DefineAsRegister(new(zone()) LSmiUntag(value, false));
} else {
LOperand* temp1 = TempRegister();
LOperand* temp2 = instr->CanTruncateToInt32() ? TempRegister()
=======================================
--- /branches/bleeding_edge/src/arm/lithium-codegen-arm.cc Mon Jun 11
05:42:31 2012
+++ /branches/bleeding_edge/src/arm/lithium-codegen-arm.cc Tue Jun 12
05:16:19 2012
@@ -2753,9 +2753,14 @@
// Check for the hole value.
if (instr->hydrogen()->RequiresHoleCheck()) {
- __ LoadRoot(scratch, Heap::kTheHoleValueRootIndex);
- __ cmp(result, scratch);
- DeoptimizeIf(eq, instr->environment());
+ if (IsFastSmiElementsKind(instr->hydrogen()->elements_kind())) {
+ __ tst(result, Operand(kSmiTagMask));
+ DeoptimizeIf(ne, instr->environment());
+ } else {
+ __ LoadRoot(scratch, Heap::kTheHoleValueRootIndex);
+ __ cmp(result, scratch);
+ DeoptimizeIf(eq, instr->environment());
+ }
}
}
=======================================
--- /branches/bleeding_edge/src/hydrogen-instructions.cc Mon Jun 11
05:42:31 2012
+++ /branches/bleeding_edge/src/hydrogen-instructions.cc Tue Jun 12
05:16:19 2012
@@ -1712,14 +1712,14 @@
stream->Add("[");
key()->PrintNameTo(stream);
stream->Add("]");
- if (hole_check_mode_ == PERFORM_HOLE_CHECK) {
+ if (RequiresHoleCheck()) {
stream->Add(" check_hole");
}
}
bool HLoadKeyedFastElement::RequiresHoleCheck() {
- if (hole_check_mode_ == OMIT_HOLE_CHECK) {
+ if (IsFastPackedElementsKind(elements_kind())) {
return false;
}
@@ -1765,8 +1765,7 @@
new(block()->zone()) HCheckMapValue(object(),
names_cache->map());
HInstruction* index = new(block()->zone()) HLoadKeyedFastElement(
index_cache,
- key_load->key(),
- OMIT_HOLE_CHECK);
+ key_load->key());
map_check->InsertBefore(this);
index->InsertBefore(this);
HLoadFieldByIndex* load = new(block()->zone()) HLoadFieldByIndex(
=======================================
--- /branches/bleeding_edge/src/hydrogen-instructions.h Mon Jun 11 05:42:31
2012
+++ /branches/bleeding_edge/src/hydrogen-instructions.h Tue Jun 12 05:16:19
2012
@@ -1848,7 +1848,9 @@
class HJSArrayLength: public HTemplateInstruction<2> {
public:
- HJSArrayLength(HValue* value, HValue* typecheck) {
+ HJSArrayLength(HValue* value, HValue* typecheck,
+ HType type = HType::Tagged()) {
+ set_type(type);
// The length of an array is stored as a tagged value in the array
// object. It is guaranteed to be 32 bit integer, but it can be
// represented as either a smi or heap number.
@@ -1872,7 +1874,7 @@
DECLARE_CONCRETE_INSTRUCTION(JSArrayLength)
protected:
- virtual bool DataEquals(HValue* other) { return true; }
+ virtual bool DataEquals(HValue* other_raw) { return true; }
};
@@ -3980,18 +3982,19 @@
virtual ~ArrayInstructionInterface() { };
};
-
-enum HoleCheckMode { PERFORM_HOLE_CHECK, OMIT_HOLE_CHECK };
-
class HLoadKeyedFastElement
: public HTemplateInstruction<2>, public ArrayInstructionInterface {
public:
HLoadKeyedFastElement(HValue* obj,
HValue* key,
- HoleCheckMode hole_check_mode = PERFORM_HOLE_CHECK)
- : hole_check_mode_(hole_check_mode),
- index_offset_(0),
- is_dehoisted_(false) {
+ ElementsKind elements_kind = FAST_ELEMENTS)
+ : bit_field_(0) {
+ ASSERT(IsFastSmiOrObjectElementsKind(elements_kind));
+ bit_field_ = ElementsKindField::encode(elements_kind);
+ if (IsFastSmiElementsKind(elements_kind) &&
+ IsFastPackedElementsKind(elements_kind)) {
+ set_type(HType::Smi());
+ }
SetOperandAt(0, obj);
SetOperandAt(1, key);
set_representation(Representation::Tagged());
@@ -4001,12 +4004,19 @@
HValue* object() { return OperandAt(0); }
HValue* key() { return OperandAt(1); }
- uint32_t index_offset() { return index_offset_; }
- void SetIndexOffset(uint32_t index_offset) { index_offset_ =
index_offset; }
+ uint32_t index_offset() { return IndexOffsetField::decode(bit_field_); }
+ void SetIndexOffset(uint32_t index_offset) {
+ bit_field_ = IndexOffsetField::update(bit_field_, index_offset);
+ }
HValue* GetKey() { return key(); }
void SetKey(HValue* key) { SetOperandAt(1, key); }
- bool IsDehoisted() { return is_dehoisted_; }
- void SetDehoisted(bool is_dehoisted) { is_dehoisted_ = is_dehoisted; }
+ bool IsDehoisted() { return IsDehoistedField::decode(bit_field_); }
+ void SetDehoisted(bool is_dehoisted) {
+ bit_field_ = IsDehoistedField::update(bit_field_, is_dehoisted);
+ }
+ ElementsKind elements_kind() const {
+ return ElementsKindField::decode(bit_field_);
+ }
virtual Representation RequiredInputRepresentation(int index) {
// The key is supposed to be Integer32.
@@ -4025,18 +4035,22 @@
virtual bool DataEquals(HValue* other) {
if (!other->IsLoadKeyedFastElement()) return false;
HLoadKeyedFastElement* other_load = HLoadKeyedFastElement::cast(other);
- if (is_dehoisted_ && index_offset_ != other_load->index_offset_)
+ if (IsDehoisted() && index_offset() != other_load->index_offset())
return false;
- return hole_check_mode_ == other_load->hole_check_mode_;
+ return elements_kind() == other_load->elements_kind();
}
private:
- HoleCheckMode hole_check_mode_;
- uint32_t index_offset_;
- bool is_dehoisted_;
+ class ElementsKindField: public BitField<ElementsKind, 0, 4> {};
+ class IndexOffsetField: public BitField<uint32_t, 4, 27> {};
+ class IsDehoistedField: public BitField<bool, 31, 1> {};
+ uint32_t bit_field_;
};
+enum HoleCheckMode { PERFORM_HOLE_CHECK, OMIT_HOLE_CHECK };
+
+
class HLoadKeyedFastDoubleElement
: public HTemplateInstruction<2>, public ArrayInstructionInterface {
public:
@@ -4044,15 +4058,15 @@
HValue* elements,
HValue* key,
HoleCheckMode hole_check_mode = PERFORM_HOLE_CHECK)
- : index_offset_(0),
- is_dehoisted_(false),
- hole_check_mode_(hole_check_mode) {
- SetOperandAt(0, elements);
- SetOperandAt(1, key);
- set_representation(Representation::Double());
+ : index_offset_(0),
+ is_dehoisted_(false),
+ hole_check_mode_(hole_check_mode) {
+ SetOperandAt(0, elements);
+ SetOperandAt(1, key);
+ set_representation(Representation::Double());
SetGVNFlag(kDependsOnDoubleArrayElements);
SetFlag(kUseGVN);
- }
+ }
HValue* elements() { return OperandAt(0); }
HValue* key() { return OperandAt(1); }
=======================================
--- /branches/bleeding_edge/src/hydrogen.cc Tue Jun 12 03:22:33 2012
+++ /branches/bleeding_edge/src/hydrogen.cc Tue Jun 12 05:16:19 2012
@@ -4128,8 +4128,7 @@
HValue* key = AddInstruction(
new(zone()) HLoadKeyedFastElement(
environment()->ExpressionStackAt(2), // Enum cache.
- environment()->ExpressionStackAt(0), // Iteration index.
- OMIT_HOLE_CHECK));
+ environment()->ExpressionStackAt(0))); // Iteration index.
// Check if the expected map still matches that of the enumerable.
// If not just deoptimize.
@@ -5483,7 +5482,8 @@
if (IsFastDoubleElementsKind(elements_kind)) {
return new(zone()) HLoadKeyedFastDoubleElement(elements, checked_key,
mode);
} else { // Smi or Object elements.
- return new(zone()) HLoadKeyedFastElement(elements, checked_key, mode);
+ return new(zone()) HLoadKeyedFastElement(elements, checked_key,
+ elements_kind);
}
}
@@ -5531,7 +5531,8 @@
fast_elements ||
map->has_fast_double_elements());
if (map->instance_type() == JS_ARRAY_TYPE) {
- length = AddInstruction(new(zone()) HJSArrayLength(object, mapcheck));
+ length = AddInstruction(new(zone()) HJSArrayLength(object, mapcheck,
+ HType::Smi()));
} else {
length = AddInstruction(new(zone()) HFixedArrayBaseLength(elements));
}
@@ -5686,7 +5687,8 @@
set_current_block(if_jsarray);
HInstruction* length;
- length = AddInstruction(new(zone()) HJSArrayLength(object,
typecheck));
+ length = AddInstruction(new(zone()) HJSArrayLength(object,
typecheck,
+ HType::Smi()));
checked_key = AddInstruction(new(zone()) HBoundsCheck(key,
length));
access = AddInstruction(BuildFastElementAccess(
elements, checked_key, val, elements_kind, is_store));
@@ -5886,7 +5888,6 @@
HInstruction* mapcheck =
AddInstruction(HCheckInstanceType::NewIsJSArray(array, zone()));
instr = new(zone()) HJSArrayLength(array, mapcheck);
-
} else if (expr->IsStringLength()) {
HValue* string = Pop();
AddInstruction(new(zone()) HCheckNonSmi(string));
=======================================
--- /branches/bleeding_edge/src/ia32/lithium-codegen-ia32.cc Tue Jun 12
03:22:33 2012
+++ /branches/bleeding_edge/src/ia32/lithium-codegen-ia32.cc Tue Jun 12
05:16:19 2012
@@ -2505,8 +2505,13 @@
// Check for the hole value.
if (instr->hydrogen()->RequiresHoleCheck()) {
- __ cmp(result, factory()->the_hole_value());
- DeoptimizeIf(equal, instr->environment());
+ if (IsFastSmiElementsKind(instr->hydrogen()->elements_kind())) {
+ __ test(result, Immediate(kSmiTagMask));
+ DeoptimizeIf(not_equal, instr->environment());
+ } else {
+ __ cmp(result, factory()->the_hole_value());
+ DeoptimizeIf(equal, instr->environment());
+ }
}
}
@@ -3944,6 +3949,10 @@
if (instr->needs_check()) {
__ test(ToRegister(input), Immediate(kSmiTagMask));
DeoptimizeIf(not_zero, instr->environment());
+ } else {
+ if (FLAG_debug_code) {
+ __ AbortIfNotSmi(ToRegister(input));
+ }
}
__ SmiUntag(ToRegister(input));
}
=======================================
--- /branches/bleeding_edge/src/ia32/lithium-ia32.cc Tue Jun 12 03:22:33
2012
+++ /branches/bleeding_edge/src/ia32/lithium-ia32.cc Tue Jun 12 05:16:19
2012
@@ -1707,8 +1707,9 @@
} else {
ASSERT(to.IsInteger32());
LOperand* value = UseRegister(instr->value());
- bool needs_check = !instr->value()->type().IsSmi();
- if (needs_check) {
+ if (instr->value()->type().IsSmi()) {
+ return DefineSameAsFirst(new(zone()) LSmiUntag(value, false));
+ } else {
bool truncating = instr->CanTruncateToInt32();
LOperand* xmm_temp =
(truncating && CpuFeatures::IsSupported(SSE3))
@@ -1716,8 +1717,6 @@
: FixedTemp(xmm1);
LTaggedToI* res = new(zone()) LTaggedToI(value, xmm_temp);
return AssignEnvironment(DefineSameAsFirst(res));
- } else {
- return DefineSameAsFirst(new(zone()) LSmiUntag(value,
needs_check));
}
}
} else if (from.IsDouble()) {
=======================================
--- /branches/bleeding_edge/src/ia32/macro-assembler-ia32.cc Sun Jun 10
23:59:56 2012
+++ /branches/bleeding_edge/src/ia32/macro-assembler-ia32.cc Tue Jun 12
05:16:19 2012
@@ -243,8 +243,6 @@
Register scratch1,
Register scratch2,
SaveFPRegsMode save_fp) {
- // First, check if a write barrier is even needed. The tests below
- // catch stores of Smis.
Label done;
Register address = scratch1;
@@ -281,7 +279,7 @@
Label::kNear);
// Delay the initialization of |address| and |value| for the stub until
it's
- // known that the will be needed. Up until this point their value are not
+ // known that the will be needed. Up until this point their values are
not
// needed since they are embedded in the operands of instructions that
need
// them.
lea(address, FieldOperand(object, HeapObject::kMapOffset));
=======================================
--- /branches/bleeding_edge/src/x64/lithium-codegen-x64.cc Mon Jun 11
05:42:31 2012
+++ /branches/bleeding_edge/src/x64/lithium-codegen-x64.cc Tue Jun 12
05:16:19 2012
@@ -2389,8 +2389,13 @@
// Check for the hole value.
if (instr->hydrogen()->RequiresHoleCheck()) {
- __ CompareRoot(result, Heap::kTheHoleValueRootIndex);
- DeoptimizeIf(equal, instr->environment());
+ if (IsFastSmiElementsKind(instr->hydrogen()->elements_kind())) {
+ Condition smi = __ CheckSmi(result);
+ DeoptimizeIf(NegateCondition(smi), instr->environment());
+ } else {
+ __ CompareRoot(result, Heap::kTheHoleValueRootIndex);
+ DeoptimizeIf(equal, instr->environment());
+ }
}
}
@@ -3790,6 +3795,10 @@
if (instr->needs_check()) {
Condition is_smi = __ CheckSmi(input);
DeoptimizeIf(NegateCondition(is_smi), instr->environment());
+ } else {
+ if (FLAG_debug_code) {
+ __ AbortIfNotSmi(input);
+ }
}
__ SmiToInteger32(input, input);
}
=======================================
--- /branches/bleeding_edge/src/x64/lithium-x64.cc Mon Jun 11 05:42:31 2012
+++ /branches/bleeding_edge/src/x64/lithium-x64.cc Tue Jun 12 05:16:19 2012
@@ -1644,14 +1644,13 @@
} else {
ASSERT(to.IsInteger32());
LOperand* value = UseRegister(instr->value());
- bool needs_check = !instr->value()->type().IsSmi();
- if (needs_check) {
+ if (instr->value()->type().IsSmi()) {
+ return DefineSameAsFirst(new(zone()) LSmiUntag(value, false));
+ } else {
bool truncating = instr->CanTruncateToInt32();
LOperand* xmm_temp = truncating ? NULL : FixedTemp(xmm1);
LTaggedToI* res = new(zone()) LTaggedToI(value, xmm_temp);
return AssignEnvironment(DefineSameAsFirst(res));
- } else {
- return DefineSameAsFirst(new(zone()) LSmiUntag(value,
needs_check));
}
}
} else if (from.IsDouble()) {
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
