Revision: 4147
Author: [email protected]
Date: Tue Mar 16 09:03:40 2010
Log: Fix bug in the count operation where we statically know the input is a
smi.
Even if we know that the input to a count operation is a smi we still need
to check if the result overflowed (and becomes a heap number).
Also fix the smi loop analysis to take two border cases correctly into
account.
Review URL: http://codereview.chromium.org/1040002
http://code.google.com/p/v8/source/detail?r=4147
Added:
/branches/bleeding_edge/test/mjsunit/compiler/loopcount.js
Modified:
/branches/bleeding_edge/src/data-flow.cc
/branches/bleeding_edge/src/ia32/codegen-ia32.cc
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/compiler/loopcount.js Tue Mar 16
09:03:40 2010
@@ -0,0 +1,55 @@
+// Copyright 2010 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 postfix count operations with smis.
+
+function f1() { var x = 0x3fffffff; x++; return x; }
+assertEquals(0x40000000, f1());
+
+
+function f2() { var x = -0x40000000; x--; return x; }
+assertEquals(-0x40000001, f2());
+
+
+function f3(x) { x = x & 0x3fffffff; x++; return x; }
+assertEquals(0x40000000, f3(0x3fffffff));
+
+
+function f4() {
+ var i;
+ for (i = 0x3ffffffe; i <= 0x3fffffff; i++) {}
+ return i;
+}
+assertEquals(0x40000000, f4());
+
+
+function f5() {
+ var i;
+ for (i = -0x3fffffff; i >= -0x40000000; i--) {}
+ return i;
+}
+assertEquals(-0x40000001, f5());
=======================================
--- /branches/bleeding_edge/src/data-flow.cc Tue Mar 16 03:54:02 2010
+++ /branches/bleeding_edge/src/data-flow.cc Tue Mar 16 09:03:40 2010
@@ -1117,6 +1117,12 @@
if (init_value < term_value && update->op() != Token::INC) return NULL;
if (init_value > term_value && update->op() != Token::DEC) return NULL;
+ // Check that the update operation cannot overflow the smi range. This
can
+ // occur in the two cases where the loop bound is equal to the largest or
+ // smallest smi.
+ if (update->op() == Token::INC && term_value == Smi::kMaxValue) return
NULL;
+ if (update->op() == Token::DEC && term_value == Smi::kMinValue) return
NULL;
+
// Found a smi loop variable.
return loop_var;
}
=======================================
--- /branches/bleeding_edge/src/ia32/codegen-ia32.cc Mon Mar 15 07:24:37
2010
+++ /branches/bleeding_edge/src/ia32/codegen-ia32.cc Tue Mar 16 09:03:40
2010
@@ -6684,19 +6684,25 @@
// Ensure the new value is writable.
frame_->Spill(new_value.reg());
- // In order to combine the overflow and the smi tag check, we need
- // to be able to allocate a byte register. We attempt to do so
- // without spilling. If we fail, we will generate separate overflow
- // and smi tag checks.
- //
- // We allocate and clear the temporary byte register before
- // performing the count operation since clearing the register using
- // xor will clear the overflow flag.
- Result tmp = allocator_->AllocateByteRegisterWithoutSpilling();
- if (tmp.is_valid()) {
- __ Set(tmp.reg(), Immediate(0));
- }
-
+ Result tmp;
+ if (new_value.is_smi()) {
+ if (FLAG_debug_code) {
+ __ AbortIfNotSmi(new_value.reg(), "Operand not a smi");
+ }
+ } else {
+ // We don't know statically if the input is a smi.
+ // In order to combine the overflow and the smi tag check, we need
+ // to be able to allocate a byte register. We attempt to do so
+ // without spilling. If we fail, we will generate separate overflow
+ // and smi tag checks.
+ // We allocate and clear a temporary byte register before performing
+ // the count operation since clearing the register using xor will
clear
+ // the overflow flag.
+ tmp = allocator_->AllocateByteRegisterWithoutSpilling();
+ if (tmp.is_valid()) {
+ __ Set(tmp.reg(), Immediate(0));
+ }
+ }
if (is_increment) {
__ add(Operand(new_value.reg()), Immediate(Smi::FromInt(1)));
@@ -6704,28 +6710,26 @@
__ sub(Operand(new_value.reg()), Immediate(Smi::FromInt(1)));
}
- if (new_value.is_smi()) {
- if (FLAG_debug_code) {
- __ AbortIfNotSmi(new_value.reg(), "Argument not a smi");
- }
- if (tmp.is_valid()) tmp.Unuse();
+ DeferredCode* deferred = NULL;
+ if (is_postfix) {
+ deferred = new DeferredPostfixCountOperation(new_value.reg(),
+ old_value.reg(),
+ is_increment);
} else {
- DeferredCode* deferred = NULL;
- if (is_postfix) {
- deferred = new DeferredPostfixCountOperation(new_value.reg(),
- old_value.reg(),
- is_increment);
- } else {
- deferred = new DeferredPrefixCountOperation(new_value.reg(),
- is_increment);
- }
-
+ deferred = new DeferredPrefixCountOperation(new_value.reg(),
+ is_increment);
+ }
+
+ if (new_value.is_smi()) {
+ // In case we have a smi as input just check for overflow.
+ deferred->Branch(overflow);
+ } else {
// If the count operation didn't overflow and the result is a valid
// smi, we're done. Otherwise, we jump to the deferred slow-case
// code.
+ // We combine the overflow and the smi tag check if we could
+ // successfully allocate a temporary byte register.
if (tmp.is_valid()) {
- // We combine the overflow and the smi tag check if we could
- // successfully allocate a temporary byte register.
__ setcc(overflow, tmp.reg());
__ or_(Operand(tmp.reg()), new_value.reg());
__ test(tmp.reg(), Immediate(kSmiTagMask));
@@ -6737,8 +6741,9 @@
__ test(new_value.reg(), Immediate(kSmiTagMask));
deferred->Branch(not_zero);
}
- deferred->BindExit();
- }
+ }
+ deferred->BindExit();
+
// Postfix: store the old value in the allocated slot under the
// reference.
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
