Author: [email protected]
Date: Fri Apr 3 06:27:14 2009
New Revision: 1677
Added:
branches/bleeding_edge/test/mjsunit/constant-folding.js
Removed:
branches/bleeding_edge/test/mjsunit/compare-constants.js
Modified:
branches/bleeding_edge/src/builtins.h
branches/bleeding_edge/src/codegen-arm.cc
branches/bleeding_edge/src/codegen-ia32.cc
branches/bleeding_edge/src/runtime.js
branches/bleeding_edge/src/virtual-frame-ia32.h
Log:
Rewrite of VisitCountOperation that should speed it up
Review URL: http://codereview.chromium.org/56151
Modified: branches/bleeding_edge/src/builtins.h
==============================================================================
--- branches/bleeding_edge/src/builtins.h (original)
+++ branches/bleeding_edge/src/builtins.h Fri Apr 3 06:27:14 2009
@@ -105,8 +105,6 @@
V(MUL, 1) \
V(DIV, 1) \
V(MOD, 1) \
- V(INC, 0) \
- V(DEC, 0) \
V(BIT_OR, 1) \
V(BIT_AND, 1) \
V(BIT_XOR, 1) \
Modified: branches/bleeding_edge/src/codegen-arm.cc
==============================================================================
--- branches/bleeding_edge/src/codegen-arm.cc (original)
+++ branches/bleeding_edge/src/codegen-arm.cc Fri Apr 3 06:27:14 2009
@@ -708,29 +708,6 @@
};
-class InvokeBuiltinStub : public CodeStub {
- public:
- enum Kind { Inc, Dec, ToNumber };
- InvokeBuiltinStub(Kind kind, int argc) : kind_(kind), argc_(argc) { }
-
- private:
- Kind kind_;
- int argc_;
-
- Major MajorKey() { return InvokeBuiltin; }
- int MinorKey() { return (argc_ << 3) | static_cast<int>(kind_); }
- void Generate(MacroAssembler* masm);
-
-#ifdef DEBUG
- void Print() {
- PrintF("InvokeBuiltinStub (kind %d, argc, %d)\n",
- static_cast<int>(kind_),
- argc_);
- }
-#endif
-};
-
-
void CodeGenerator::GenericBinaryOperation(Token::Value op) {
VirtualFrame::SpilledScope spilled_scope(this);
// sp[0] : y
@@ -3696,22 +3673,27 @@
// Slow case: Convert to number.
slow.Bind();
-
- // Postfix: Convert the operand to a number and store it as the result.
+ {
+ // Convert the operand to a number.
+ frame_->EmitPush(r0);
+ Result arg_count = allocator_->Allocate(r0);
+ ASSERT(arg_count.is_valid());
+ __ mov(arg_count.reg(), Operand(0));
+ frame_->InvokeBuiltin(Builtins::TO_NUMBER, CALL_JS, &arg_count, 1);
+ }
if (is_postfix) {
- InvokeBuiltinStub stub(InvokeBuiltinStub::ToNumber, 2);
- frame_->CallStub(&stub, 0);
- // Store to result (on the stack).
+ // Postfix: store to result (on the stack).
__ str(r0, frame_->ElementAt(target.size()));
}
- // Compute the new value by calling the right JavaScript native.
+ // Compute the new value.
+ __ mov(r1, Operand(Smi::FromInt(1)));
+ frame_->EmitPush(r0);
+ frame_->EmitPush(r1);
if (is_increment) {
- InvokeBuiltinStub stub(InvokeBuiltinStub::Inc, 1);
- frame_->CallStub(&stub, 0);
+ frame_->CallRuntime(Runtime::kNumberAdd, 2);
} else {
- InvokeBuiltinStub stub(InvokeBuiltinStub::Dec, 1);
- frame_->CallStub(&stub, 0);
+ frame_->CallRuntime(Runtime::kNumberSub, 2);
}
// Store the new value in the target if not const.
@@ -4715,19 +4697,6 @@
__ bind(&done);
__ StubReturn(1);
-}
-
-
-void InvokeBuiltinStub::Generate(MacroAssembler* masm) {
- __ push(r0);
- __ mov(r0, Operand(0)); // set number of arguments
- switch (kind_) {
- case ToNumber: __ InvokeBuiltin(Builtins::TO_NUMBER, JUMP_JS); break;
- case Inc: __ InvokeBuiltin(Builtins::INC, JUMP_JS); break;
- case Dec: __ InvokeBuiltin(Builtins::DEC, JUMP_JS); break;
- default: UNREACHABLE();
- }
- __ StubReturn(argc_);
}
Modified: branches/bleeding_edge/src/codegen-ia32.cc
==============================================================================
--- branches/bleeding_edge/src/codegen-ia32.cc (original)
+++ branches/bleeding_edge/src/codegen-ia32.cc Fri Apr 3 06:27:14 2009
@@ -4724,11 +4724,11 @@
DeferredCountOperation(CodeGenerator* generator,
bool is_postfix,
bool is_increment,
- int result_offset)
+ int target_size)
: DeferredCode(generator),
is_postfix_(is_postfix),
is_increment_(is_increment),
- result_offset_(result_offset) {
+ target_size_(target_size) {
set_comment("[ DeferredCountOperation");
}
@@ -4737,75 +4737,38 @@
private:
bool is_postfix_;
bool is_increment_;
- int result_offset_;
-};
-
-
-class RevertToNumberStub: public CodeStub {
- public:
- explicit RevertToNumberStub(bool is_increment)
- : is_increment_(is_increment) { }
-
- private:
- bool is_increment_;
-
- Major MajorKey() { return RevertToNumber; }
- int MinorKey() { return is_increment_ ? 1 : 0; }
- void Generate(MacroAssembler* masm);
-
-#ifdef DEBUG
- void Print() {
- PrintF("RevertToNumberStub (is_increment %s)\n",
- is_increment_ ? "true" : "false");
- }
-#endif
-};
-
-
-class CounterOpStub: public CodeStub {
- public:
- CounterOpStub(int result_offset, bool is_postfix, bool is_increment)
- : result_offset_(result_offset),
- is_postfix_(is_postfix),
- is_increment_(is_increment) { }
-
- private:
- int result_offset_;
- bool is_postfix_;
- bool is_increment_;
-
- Major MajorKey() { return CounterOp; }
- int MinorKey() {
- return ((result_offset_ << 2) |
- (is_postfix_ ? 2 : 0) |
- (is_increment_ ? 1 : 0));
- }
- void Generate(MacroAssembler* masm);
-
-#ifdef DEBUG
- void Print() {
- PrintF("CounterOpStub (result_offset %d), (is_postfix %s),"
- " (is_increment %s)\n",
- result_offset_,
- is_postfix_ ? "true" : "false",
- is_increment_ ? "true" : "false");
- }
-#endif
+ int target_size_;
};
void DeferredCountOperation::Generate() {
CodeGenerator* cgen = generator();
-
Result value(cgen);
enter()->Bind(&value);
- if (is_postfix_) {
- RevertToNumberStub to_number_stub(is_increment_);
- value = generator()->frame()->CallStub(&to_number_stub, &value);
+ VirtualFrame* frame = cgen->frame();
+ // Undo the optimistic smi operation.
+ value.ToRegister();
+ frame->Spill(value.reg());
+ if (is_increment_) {
+ __ sub(Operand(value.reg()), Immediate(Smi::FromInt(1)));
+ } else {
+ __ add(Operand(value.reg()), Immediate(Smi::FromInt(1)));
+ }
+ frame->Push(&value);
+ value = frame->InvokeBuiltin(Builtins::TO_NUMBER, CALL_FUNCTION, 1);
+ frame->Push(&value);
+ if (is_postfix_) { // Fix up copy of old value with ToNumber(value).
+ // This is only safe because VisitCountOperation makes this frame slot
+ // beneath the reference a register, which is spilled at the above
call.
+ // We cannot safely write to constants or copies below the water line.
+ frame->StoreToElementAt(target_size_ + 1);
+ }
+ frame->Push(Smi::FromInt(1));
+ if (is_increment_) {
+ value = frame->CallRuntime(Runtime::kNumberAdd, 2);
+ } else {
+ value = frame->CallRuntime(Runtime::kNumberSub, 2);
}
-
- CounterOpStub stub(result_offset_, is_postfix_, is_increment_);
- value = generator()->frame()->CallStub(&stub, &value);
exit_.Jump(&value);
}
@@ -4819,7 +4782,8 @@
Variable* var = node->expression()->AsVariableProxy()->AsVariable();
bool is_const = (var != NULL && var->mode() == Variable::CONST);
- // Postfix: Make room for the result.
+ // Postfix operators need a stack slot under the reference to hold
+ // the old value while the new one is being stored.
if (is_postfix) {
frame_->Push(Smi::FromInt(0));
}
@@ -4836,16 +4800,21 @@
target.TakeValue(NOT_INSIDE_TYPEOF);
DeferredCountOperation* deferred =
- new DeferredCountOperation(this, is_postfix, is_increment,
- target.size() * kPointerSize);
+ new DeferredCountOperation(this, is_postfix,
+ is_increment, target.size());
Result value = frame_->Pop();
value.ToRegister();
- ASSERT(value.is_valid());
// Postfix: Store the old value as the result.
if (is_postfix) {
- Result old_value = value;
+ // Explicitly back the slot for the old value with a new register.
+ // This improves performance in some cases.
+ Result old_value = allocator_->Allocate();
+ ASSERT(old_value.is_valid());
+ __ mov(old_value.reg(), value.reg());
+ // SetElement must not create a constant element or a copy in this
slot,
+ // since we will write to it, below the waterline, in deferred code.
frame_->SetElementAt(target.size(), &old_value);
}
@@ -4885,7 +4854,7 @@
tmp.Unuse();
__ test(value.reg(), Immediate(kSmiTagMask));
deferred->enter()->Branch(not_zero, &value, not_taken);
- } else {
+ } else { // Otherwise we test separately for overflow and smi check.
deferred->enter()->Branch(overflow, &value, not_taken);
__ test(value.reg(), Immediate(kSmiTagMask));
deferred->enter()->Branch(not_zero, &value, not_taken);
@@ -6776,49 +6745,6 @@
__ jmp(adaptor, RelocInfo::CODE_TARGET);
}
-
-void RevertToNumberStub::Generate(MacroAssembler* masm) {
- // Revert optimistic increment/decrement.
- if (is_increment_) {
- __ sub(Operand(eax), Immediate(Smi::FromInt(1)));
- } else {
- __ add(Operand(eax), Immediate(Smi::FromInt(1)));
- }
-
- __ pop(ecx);
- __ push(eax);
- __ push(ecx);
- __ InvokeBuiltin(Builtins::TO_NUMBER, JUMP_FUNCTION);
- // Code never returns due to JUMP_FUNCTION.
-}
-
-
-void CounterOpStub::Generate(MacroAssembler* masm) {
- // Store to the result on the stack (skip return address) before
- // performing the count operation.
- if (is_postfix_) {
- __ mov(Operand(esp, result_offset_ + kPointerSize), eax);
- }
-
- // Revert optimistic increment/decrement but only for prefix
- // counts. For postfix counts it has already been reverted before
- // the conversion to numbers.
- if (!is_postfix_) {
- if (is_increment_) {
- __ sub(Operand(eax), Immediate(Smi::FromInt(1)));
- } else {
- __ add(Operand(eax), Immediate(Smi::FromInt(1)));
- }
- }
-
- // Compute the new value by calling the right JavaScript native.
- __ pop(ecx);
- __ push(eax);
- __ push(ecx);
- Builtins::JavaScript builtin = is_increment_ ? Builtins::INC :
Builtins::DEC;
- __ InvokeBuiltin(builtin, JUMP_FUNCTION);
- // Code never returns due to JUMP_FUNCTION.
-}
void CEntryStub::GenerateThrowTOS(MacroAssembler* masm) {
Modified: branches/bleeding_edge/src/runtime.js
==============================================================================
--- branches/bleeding_edge/src/runtime.js (original)
+++ branches/bleeding_edge/src/runtime.js Fri Apr 3 06:27:14 2009
@@ -197,18 +197,6 @@
}
-// ECMA-262, section 11.4.4, page 47.
-function INC() {
- return %NumberAdd(%ToNumber(this), 1);
-}
-
-
-// ECMA-262, section 11.4.5, page 48.
-function DEC() {
- return %NumberSub(%ToNumber(this), 1);
-}
-
-
/* -------------------------------------------
- - - B i t o p e r a t i o n s - - -
Modified: branches/bleeding_edge/src/virtual-frame-ia32.h
==============================================================================
--- branches/bleeding_edge/src/virtual-frame-ia32.h (original)
+++ branches/bleeding_edge/src/virtual-frame-ia32.h Fri Apr 3 06:27:14 2009
@@ -172,6 +172,10 @@
PushFrameSlotAt(elements_.length() - index - 1);
}
+ void StoreToElementAt(int index) {
+ StoreToFrameSlotAt(elements_.length() - index - 1);
+ }
+
// A frame-allocated local as an assembly operand.
Operand LocalAt(int index) const {
ASSERT(0 <= index);
Added: branches/bleeding_edge/test/mjsunit/constant-folding.js
==============================================================================
--- (empty file)
+++ branches/bleeding_edge/test/mjsunit/constant-folding.js Fri Apr 3
06:27:14 2009
@@ -0,0 +1,171 @@
+// Copyright 2009 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.
+
+// Test operations that involve one or more constants.
+// The code generator now handles compile-time constants specially.
+// Test the code generated when operands are known at compile time
+
+// Test count operations involving constants
+function test_count() {
+ var x = "foo";
+ var y = "3";
+
+ x += x++; // ++ and -- apply ToNumber to their operand, even for
postfix.
+ assertEquals(x, "fooNaN", "fooNaN test");
+ x = "luft";
+ x += ++x;
+ assertEquals(x, "luftNaN", "luftNaN test");
+
+ assertTrue(y++ === 3, "y++ === 3, where y = \"3\"");
+ y = 3;
+ assertEquals(y++, 3, "y++ == 3, where y = 3");
+ y = "7.1";
+ assertTrue(y++ === 7.1, "y++ === 7.1, where y = \"7.1\"");
+ var z = y = x = "9";
+ assertEquals( z++ + (++y) + x++, 28, "z++ + (++y) + x++ == 28");
+ z = y = x = 13;
+ assertEquals( z++ + (++y) + x++, 40, "z++ + (++y) + x++ == 40");
+ z = y = x = -5.5;
+ assertEquals( z++ + (++y) + x++, -15.5, "z++ + (++y) + x++ == -15.5");
+
+ assertEquals(y, -4.5);
+ z = y;
+ z++;
+ assertEquals(y, -4.5);
+ z = y;
+ y++;
+ assertEquals(z, -4.5);
+
+ y = 20;
+ z = y;
+ z++;
+ assertEquals(y, 20);
+ z = y;
+ y++;
+ assertEquals(z, 20);
+
+ const w = 30;
+ assertEquals(w++, 30);
+ assertEquals(++w, 31);
+ assertEquals(++w, 31);
+}
+
+test_count();
+
+// Test comparison operations that involve one or two constant smis.
+
+function test() {
+ var i = 5;
+ var j = 3;
+
+ assertTrue( j < i );
+ i = 5; j = 3;
+ assertTrue( j <= i );
+ i = 5; j = 3;
+ assertTrue( i > j );
+ i = 5; j = 3;
+ assertTrue( i >= j );
+ i = 5; j = 3;
+ assertTrue( i != j );
+ i = 5; j = 3;
+ assertTrue( i == i );
+ i = 5; j = 3;
+ assertFalse( i < j );
+ i = 5; j = 3;
+ assertFalse( i <= j );
+ i = 5; j = 3;
+ assertFalse( j > i );
+ i = 5; j = 3;
+ assertFalse(j >= i );
+ i = 5; j = 3;
+ assertFalse( j == i);
+ i = 5; j = 3;
+ assertFalse( i != i);
+
+ i = 10 * 10;
+ while ( i < 107 ) {
+ ++i;
+ }
+ j = 21;
+
+ assertTrue( j < i );
+ j = 21;
+ assertTrue( j <= i );
+ j = 21;
+ assertTrue( i > j );
+ j = 21;
+ assertTrue( i >= j );
+ j = 21;
+ assertTrue( i != j );
+ j = 21;
+ assertTrue( i == i );
+ j = 21;
+ assertFalse( i < j );
+ j = 21;
+ assertFalse( i <= j );
+ j = 21;
+ assertFalse( j > i );
+ j = 21;
+ assertFalse(j >= i );
+ j = 21;
+ assertFalse( j == i);
+ j = 21;
+ assertFalse( i != i);
+ j = 21;
+ assertTrue( j == j );
+ j = 21;
+ assertFalse( j != j );
+
+ assertTrue( 100 > 99 );
+ assertTrue( 101 >= 90 );
+ assertTrue( 11111 > -234 );
+ assertTrue( -888 <= -20 );
+
+ while ( 234 > 456 ) {
+ i = i + 1;
+ }
+
+ switch(3) {
+ case 5:
+ assertUnreachable();
+ break;
+ case 3:
+ j = 13;
+ default:
+ i = 2;
+ case 7:
+ j = 17;
+ break;
+ case 9:
+ j = 19;
+ assertUnreachable();
+ break;
+ }
+ assertEquals(17, j, "switch with constant value");
+}
+
+test();
--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---
