From 3d3c6b15991a763cdaace6f16effa163e2d25eeb Mon Sep 17 00:00:00 2001 From: "mvstanton@chromium.org" Date: Mon, 3 Jun 2013 14:46:23 +0000 Subject: [PATCH] Special Array constructor type feedback erroneously recorded when Array was called as a function. Issue was found with optimize_constructed_array turned on. This patch makes the fix, and turns the flag back on. BUG=244461 R=danno@chromium.org Review URL: https://codereview.chromium.org/16057005 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@14917 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/code-stubs-arm.cc | 7 +--- src/flag-definitions.h | 2 +- src/ia32/code-stubs-ia32.cc | 7 +--- src/mips/code-stubs-mips.cc | 7 +--- src/x64/code-stubs-x64.cc | 7 +--- test/mjsunit/allocation-site-info.js | 2 +- test/mjsunit/regress/regress-crbug-244461.js | 41 ++++++++++++++++++++ 7 files changed, 47 insertions(+), 26 deletions(-) create mode 100644 test/mjsunit/regress/regress-crbug-244461.js diff --git a/src/arm/code-stubs-arm.cc b/src/arm/code-stubs-arm.cc index 20d4da5e7f..c15ecb245e 100644 --- a/src/arm/code-stubs-arm.cc +++ b/src/arm/code-stubs-arm.cc @@ -4588,7 +4588,6 @@ static void GenerateRecordCallTargetNoArray(MacroAssembler* masm) { // megamorphic. // r1 : the function to call // r2 : cache cell for call target - ASSERT(!FLAG_optimize_constructed_arrays); Label done; ASSERT_EQ(*TypeFeedbackCells::MegamorphicSentinel(masm->isolate()), @@ -4732,11 +4731,7 @@ void CallFunctionStub::Generate(MacroAssembler* masm) { __ b(ne, &slow); if (RecordCallTarget()) { - if (FLAG_optimize_constructed_arrays) { - GenerateRecordCallTarget(masm); - } else { - GenerateRecordCallTargetNoArray(masm); - } + GenerateRecordCallTargetNoArray(masm); } // Fast-case: Invoke the function now. diff --git a/src/flag-definitions.h b/src/flag-definitions.h index 2023d6ebc9..8fe6778e67 100644 --- a/src/flag-definitions.h +++ b/src/flag-definitions.h @@ -258,7 +258,7 @@ DEFINE_bool(unreachable_code_elimination, false, "eliminate unreachable code (hidden behind soft deopts)") DEFINE_bool(track_allocation_sites, true, "Use allocation site info to reduce transitions") -DEFINE_bool(optimize_constructed_arrays, false, +DEFINE_bool(optimize_constructed_arrays, true, "Use allocation site info on constructed arrays") DEFINE_bool(trace_osr, false, "trace on-stack replacement") DEFINE_int(stress_runs, 0, "number of stress runs") diff --git a/src/ia32/code-stubs-ia32.cc b/src/ia32/code-stubs-ia32.cc index 325e9b78a4..aa533bf836 100644 --- a/src/ia32/code-stubs-ia32.cc +++ b/src/ia32/code-stubs-ia32.cc @@ -4637,7 +4637,6 @@ static void GenerateRecordCallTargetNoArray(MacroAssembler* masm) { // megamorphic. // ebx : cache cell for call target // edi : the function to call - ASSERT(!FLAG_optimize_constructed_arrays); Isolate* isolate = masm->isolate(); Label initialize, done; @@ -4778,11 +4777,7 @@ void CallFunctionStub::Generate(MacroAssembler* masm) { __ j(not_equal, &slow); if (RecordCallTarget()) { - if (FLAG_optimize_constructed_arrays) { - GenerateRecordCallTarget(masm); - } else { - GenerateRecordCallTargetNoArray(masm); - } + GenerateRecordCallTargetNoArray(masm); } // Fast-case: Just invoke the function. diff --git a/src/mips/code-stubs-mips.cc b/src/mips/code-stubs-mips.cc index bd89d44a73..552797adb2 100644 --- a/src/mips/code-stubs-mips.cc +++ b/src/mips/code-stubs-mips.cc @@ -4971,7 +4971,6 @@ static void GenerateRecordCallTargetNoArray(MacroAssembler* masm) { // megamorphic. // a1 : the function to call // a2 : cache cell for call target - ASSERT(!FLAG_optimize_constructed_arrays); Label done; ASSERT_EQ(*TypeFeedbackCells::MegamorphicSentinel(masm->isolate()), @@ -5114,11 +5113,7 @@ void CallFunctionStub::Generate(MacroAssembler* masm) { __ Branch(&slow, ne, a3, Operand(JS_FUNCTION_TYPE)); if (RecordCallTarget()) { - if (FLAG_optimize_constructed_arrays) { - GenerateRecordCallTarget(masm); - } else { - GenerateRecordCallTargetNoArray(masm); - } + GenerateRecordCallTargetNoArray(masm); } // Fast-case: Invoke the function now. diff --git a/src/x64/code-stubs-x64.cc b/src/x64/code-stubs-x64.cc index d2ed601a00..5da0990880 100644 --- a/src/x64/code-stubs-x64.cc +++ b/src/x64/code-stubs-x64.cc @@ -3672,7 +3672,6 @@ static void GenerateRecordCallTargetNoArray(MacroAssembler* masm) { // megamorphic. // rbx : cache cell for call target // rdi : the function to call - ASSERT(!FLAG_optimize_constructed_arrays); Isolate* isolate = masm->isolate(); Label initialize, done; @@ -3810,11 +3809,7 @@ void CallFunctionStub::Generate(MacroAssembler* masm) { __ j(not_equal, &slow); if (RecordCallTarget()) { - if (FLAG_optimize_constructed_arrays) { - GenerateRecordCallTarget(masm); - } else { - GenerateRecordCallTargetNoArray(masm); - } + GenerateRecordCallTargetNoArray(masm); } // Fast-case: Just invoke the function. diff --git a/test/mjsunit/allocation-site-info.js b/test/mjsunit/allocation-site-info.js index d718993214..45605317fe 100644 --- a/test/mjsunit/allocation-site-info.js +++ b/test/mjsunit/allocation-site-info.js @@ -37,7 +37,7 @@ // support_smi_only_arrays = %HasFastSmiElements(new Array(1,2,3,4,5,6,7,8)); support_smi_only_arrays = true; -optimize_constructed_arrays = false; +optimize_constructed_arrays = true; if (support_smi_only_arrays) { print("Tests include smi-only arrays."); diff --git a/test/mjsunit/regress/regress-crbug-244461.js b/test/mjsunit/regress/regress-crbug-244461.js new file mode 100644 index 0000000000..9c7c2b6c43 --- /dev/null +++ b/test/mjsunit/regress/regress-crbug-244461.js @@ -0,0 +1,41 @@ +// 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 --smi-only-arrays +// Flags: --track-allocation-sites + +function foo(arg) { + var a = arg(); + return a; +} + + +foo(Array); +foo(Array); +%OptimizeFunctionOnNextCall(foo); +// Compilation of foo will crash without the bugfix for 244461 +foo(Array);