From cb1eedd269a42c886897898c6fdddacbb84a4c55 Mon Sep 17 00:00:00 2001 From: "whesse@chromium.org" Date: Wed, 14 Jul 2010 13:22:47 +0000 Subject: [PATCH] Fix error in x64 fast smi loops, change 4998. Review URL: http://codereview.chromium.org/2925012 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@5069 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/x64/codegen-x64.cc | 65 +++++++++++++----------- test/mjsunit/regress/regress-r4998.js | 94 +++++++++++++++++++++++++++++++++++ 2 files changed, 130 insertions(+), 29 deletions(-) create mode 100644 test/mjsunit/regress/regress-r4998.js diff --git a/src/x64/codegen-x64.cc b/src/x64/codegen-x64.cc index 9b06851..72e7cb3 100644 --- a/src/x64/codegen-x64.cc +++ b/src/x64/codegen-x64.cc @@ -3428,49 +3428,56 @@ void CodeGenerator::GenerateFastSmiLoop(ForStatement* node) { CodeForStatementPosition(node); Slot* loop_var_slot = loop_var->slot(); if (loop_var_slot->type() == Slot::LOCAL) { - frame_->PushLocalAt(loop_var_slot->index()); + frame_->TakeLocalAt(loop_var_slot->index()); } else { ASSERT(loop_var_slot->type() == Slot::PARAMETER); - frame_->PushParameterAt(loop_var_slot->index()); + frame_->TakeParameterAt(loop_var_slot->index()); } Result loop_var_result = frame_->Pop(); if (!loop_var_result.is_register()) { loop_var_result.ToRegister(); } - + Register loop_var_reg = loop_var_result.reg(); + frame_->Spill(loop_var_reg); if (increments) { - __ SmiAddConstant(loop_var_result.reg(), - loop_var_result.reg(), + __ SmiAddConstant(loop_var_reg, + loop_var_reg, Smi::FromInt(1)); } else { - __ SmiSubConstant(loop_var_result.reg(), - loop_var_result.reg(), + __ SmiSubConstant(loop_var_reg, + loop_var_reg, Smi::FromInt(1)); } - { - __ SmiCompare(loop_var_result.reg(), limit_value); - Condition condition; - switch (compare_op) { - case Token::LT: - condition = less; - break; - case Token::LTE: - condition = less_equal; - break; - case Token::GT: - condition = greater; - break; - case Token::GTE: - condition = greater_equal; - break; - default: - condition = never; - UNREACHABLE(); - } - loop.Branch(condition); + frame_->Push(&loop_var_result); + if (loop_var_slot->type() == Slot::LOCAL) { + frame_->StoreToLocalAt(loop_var_slot->index()); + } else { + ASSERT(loop_var_slot->type() == Slot::PARAMETER); + frame_->StoreToParameterAt(loop_var_slot->index()); + } + frame_->Drop(); + + __ SmiCompare(loop_var_reg, limit_value); + Condition condition; + switch (compare_op) { + case Token::LT: + condition = less; + break; + case Token::LTE: + condition = less_equal; + break; + case Token::GT: + condition = greater; + break; + case Token::GTE: + condition = greater_equal; + break; + default: + condition = never; + UNREACHABLE(); } - loop_var_result.Unuse(); + loop.Branch(condition); } if (node->break_target()->is_linked()) { node->break_target()->Bind(); diff --git a/test/mjsunit/regress/regress-r4998.js b/test/mjsunit/regress/regress-r4998.js new file mode 100644 index 0000000..9cf3371 --- /dev/null +++ b/test/mjsunit/regress/regress-r4998.js @@ -0,0 +1,94 @@ +// 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 for a broken fast-smi-loop that does not save the incremented value +// of the loop index. If this test fails, it loops forever, and times out. + +// Flags: --nofull-compiler + +// Calling foo() spills the virtual frame. +function foo() { + return; +} + +function bar() { + var x1 = 3; + var x2 = 3; + var x3 = 3; + var x4 = 3; + var x5 = 3; + var x6 = 3; + var x7 = 3; + var x8 = 3; + var x9 = 3; + var x10 = 3; + var x11 = 3; + var x12 = 3; + var x13 = 3; + + foo(); + + x1 = 257; + x2 = 258; + x3 = 259; + x4 = 260; + x5 = 261; + x6 = 262; + x7 = 263; + x8 = 264; + x9 = 265; + x10 = 266; + x11 = 267; + x12 = 268; + x13 = 269; + + // The loop variable x7 is initialized to 3, + // and then MakeMergeable is called on the virtual frame. + // MakeMergeable has forced the loop variable x7 to be spilled, + // so it is marked as synced + // The back edge then merges its virtual frame, which incorrectly + // claims that x7 is synced, and does not save the modified + // value. + for (x7 = 3; x7 < 10; ++x7) { + foo(); + } +} + +bar(); + +function aliasing() { + var x = 3; + var j; + for (j = 7; j < 11; ++j) { + x = j; + } + + assertEquals(10, x); + assertEquals(11, j); +} + +aliasing(); -- 2.7.4