From e0985887bf48a31705c7866d31bd3a586fe03c10 Mon Sep 17 00:00:00 2001 From: "fschneider@chromium.org" Date: Mon, 30 May 2011 11:31:41 +0000 Subject: [PATCH] Simple support for const variables in Crankshaft. The approach is to handle the common case in the optimizing compiler and to bailout for the rare corner cases. This is done by initializing all local const-variables with the hole value and disallowing any use of the hole value statically. Review URL: http://codereview.chromium.org/6026006 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@8104 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/lithium-arm.cc | 5 +++ src/hydrogen-instructions.h | 13 ++++++++ src/hydrogen.cc | 47 ++++++++++++++++++++++---- src/hydrogen.h | 2 ++ src/ia32/lithium-ia32.cc | 5 +++ src/x64/lithium-x64.cc | 5 +++ test/mjsunit/compiler/regress-const.js | 60 ++++++++++++++++++++++++++++++++++ 7 files changed, 131 insertions(+), 6 deletions(-) create mode 100644 test/mjsunit/compiler/regress-const.js diff --git a/src/arm/lithium-arm.cc b/src/arm/lithium-arm.cc index dc5bdaf..d4c2ac1 100644 --- a/src/arm/lithium-arm.cc +++ b/src/arm/lithium-arm.cc @@ -1633,6 +1633,11 @@ LInstruction* LChunkBuilder::DoThrow(HThrow* instr) { } +LInstruction* LChunkBuilder::DoUseConst(HUseConst* instr) { + return NULL; +} + + LInstruction* LChunkBuilder::DoForceRepresentation(HForceRepresentation* bad) { // All HForceRepresentation instructions should be eliminated in the // representation change phase of Hydrogen. diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h index fce11a1..40da1b9 100644 --- a/src/hydrogen-instructions.h +++ b/src/hydrogen-instructions.h @@ -168,6 +168,7 @@ class LChunkBuilder; V(TypeofIs) \ V(UnaryMathOperation) \ V(UnknownOSRValue) \ + V(UseConst) \ V(ValueOf) #define GVN_FLAG_LIST(V) \ @@ -1019,6 +1020,18 @@ class HThrow: public HUnaryOperation { }; +class HUseConst: public HUnaryOperation { + public: + explicit HUseConst(HValue* old_value) : HUnaryOperation(old_value) { } + + virtual Representation RequiredInputRepresentation(int index) const { + return Representation::None(); + } + + DECLARE_CONCRETE_INSTRUCTION(UseConst) +}; + + class HForceRepresentation: public HTemplateInstruction<1> { public: HForceRepresentation(HValue* value, Representation required_representation) { diff --git a/src/hydrogen.cc b/src/hydrogen.cc index e683a1e..c76f7c4 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -521,6 +521,12 @@ HConstant* HGraph::GetConstantFalse() { return GetConstant(&constant_false_, isolate()->heap()->false_value()); } + +HConstant* HGraph::GetConstantHole() { + return GetConstant(&constant_hole_, isolate()->heap()->the_hole_value()); +} + + HGraphBuilder::HGraphBuilder(CompilationInfo* info, TypeFeedbackOracle* oracle) : function_state_(NULL), @@ -826,6 +832,10 @@ bool HGraph::CollectPhis() { phi_list_->Add(phi); // We don't support phi uses of arguments for now. if (phi->CheckFlag(HValue::kIsArguments)) return false; + // Check for the hole value (from an uninitialized const). + for (int k = 0; k < phi->OperandCount(); k++) { + if (phi->OperandAt(k) == GetConstantHole()) return false; + } } } return true; @@ -2225,7 +2235,7 @@ HGraph* HGraphBuilder::CreateGraph() { graph()->EliminateRedundantPhis(); if (FLAG_eliminate_dead_phis) graph()->EliminateUnreachablePhis(); if (!graph()->CollectPhis()) { - Bailout("Phi-use of arguments object"); + Bailout("Unsupported phi-use"); return NULL; } @@ -2996,7 +3006,12 @@ void HGraphBuilder::VisitVariableProxy(VariableProxy* expr) { if (variable == NULL) { return Bailout("reference to rewritten variable"); } else if (variable->IsStackAllocated()) { - ast_context()->ReturnValue(environment()->Lookup(variable)); + HValue* value = environment()->Lookup(variable); + if (variable->mode() == Variable::CONST && + value == graph()->GetConstantHole()) { + return Bailout("reference to uninitialized const variable"); + } + ast_context()->ReturnValue(value); } else if (variable->IsContextSlot()) { if (variable->mode() == Variable::CONST) { return Bailout("reference to const context slot"); @@ -3451,6 +3466,10 @@ void HGraphBuilder::HandleCompoundAssignment(Assignment* expr) { BinaryOperation* operation = expr->binary_operation(); if (var != NULL) { + if (var->mode() == Variable::CONST) { + return Bailout("unsupported const compound assignment"); + } + CHECK_ALIVE(VisitForValue(operation)); if (var->is_global()) { @@ -3557,6 +3576,16 @@ void HGraphBuilder::VisitAssignment(Assignment* expr) { } if (var != NULL) { + if (var->mode() == Variable::CONST) { + if (expr->op() != Token::INIT_CONST) { + return Bailout("non-initializer assignment to const"); + } + // We insert a use of the old value to detect unsupported uses of const + // variables (e.g. initialization inside a loop). + HValue* old_value = environment()->Lookup(var); + AddInstruction(new HUseConst(old_value)); + } + if (proxy->IsArguments()) return Bailout("assignment to arguments"); // Handle the assignment. @@ -4871,6 +4900,9 @@ void HGraphBuilder::VisitCountOperation(CountOperation* expr) { HValue* after = NULL; // The result after incrementing or decrementing. if (var != NULL) { + if (var->mode() == Variable::CONST) { + return Bailout("unsupported count operation with const"); + } // Argument of the count operation is a variable, not a property. ASSERT(prop == NULL); CHECK_ALIVE(VisitForValue(target)); @@ -5335,17 +5367,20 @@ void HGraphBuilder::VisitThisFunction(ThisFunction* expr) { void HGraphBuilder::VisitDeclaration(Declaration* decl) { // We allow only declarations that do not require code generation. - // The following all require code generation: global variables and - // functions, variables with slot type LOOKUP, declarations with - // mode CONST, and functions. + // The following all require code generation: global variables, + // functions, and variables with slot type LOOKUP Variable* var = decl->proxy()->var(); Slot* slot = var->AsSlot(); if (var->is_global() || + !var->IsStackAllocated() || (slot != NULL && slot->type() == Slot::LOOKUP) || - decl->mode() == Variable::CONST || decl->fun() != NULL) { return Bailout("unsupported declaration"); } + + if (decl->mode() == Variable::CONST) { + environment()->Bind(var, graph()->GetConstantHole()); + } } diff --git a/src/hydrogen.h b/src/hydrogen.h index 62b362c..e07afbc 100644 --- a/src/hydrogen.h +++ b/src/hydrogen.h @@ -238,6 +238,7 @@ class HGraph: public ZoneObject { HConstant* GetConstantMinus1(); HConstant* GetConstantTrue(); HConstant* GetConstantFalse(); + HConstant* GetConstantHole(); HBasicBlock* CreateBasicBlock(); HArgumentsObject* GetArgumentsObject() const { @@ -299,6 +300,7 @@ class HGraph: public ZoneObject { SetOncePointer constant_minus1_; SetOncePointer constant_true_; SetOncePointer constant_false_; + SetOncePointer constant_hole_; SetOncePointer arguments_object_; DISALLOW_COPY_AND_ASSIGN(HGraph); diff --git a/src/ia32/lithium-ia32.cc b/src/ia32/lithium-ia32.cc index 401da7a..ea25343 100644 --- a/src/ia32/lithium-ia32.cc +++ b/src/ia32/lithium-ia32.cc @@ -1663,6 +1663,11 @@ LInstruction* LChunkBuilder::DoThrow(HThrow* instr) { } +LInstruction* LChunkBuilder::DoUseConst(HUseConst* instr) { + return NULL; +} + + LInstruction* LChunkBuilder::DoForceRepresentation(HForceRepresentation* bad) { // All HForceRepresentation instructions should be eliminated in the // representation change phase of Hydrogen. diff --git a/src/x64/lithium-x64.cc b/src/x64/lithium-x64.cc index adf86c4..8ac7307 100644 --- a/src/x64/lithium-x64.cc +++ b/src/x64/lithium-x64.cc @@ -1634,6 +1634,11 @@ LInstruction* LChunkBuilder::DoThrow(HThrow* instr) { } +LInstruction* LChunkBuilder::DoUseConst(HUseConst* instr) { + return NULL; +} + + LInstruction* LChunkBuilder::DoForceRepresentation(HForceRepresentation* bad) { // All HForceRepresentation instructions should be eliminated in the // representation change phase of Hydrogen. diff --git a/test/mjsunit/compiler/regress-const.js b/test/mjsunit/compiler/regress-const.js new file mode 100644 index 0000000..57f2c0d --- /dev/null +++ b/test/mjsunit/compiler/regress-const.js @@ -0,0 +1,60 @@ +// Copyright 2011 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 const initialization and assignments. +function f() { + var x = 42; + while (true) { + const y = x; + if (--x == 0) return y; + } +} + +function g() { + const x = 42; + x += 1; + return x; +} + +for (var i = 0; i < 1000000; i++) { + f(); + g(); +} + +assertEquals(42, f()); +assertEquals(42, g()); + + +function h(a, b) { + var r = a + b; + const X = 42; + return r + X; +} + +for (var i=0; i<10000000; i++) f(1,2); +assertEquals(45, h(1,2)); +assertEquals("foo742", h("foo", 7)); -- 2.7.4