From 0c844cc590a9a8bce6bbecd52097e2fde171e9ba Mon Sep 17 00:00:00 2001 From: "dcarney@chromium.org" Date: Fri, 14 Feb 2014 14:13:06 +0000 Subject: [PATCH] api accessor store ics should return passed value R=verwaest@chromium.org BUG= Review URL: https://codereview.chromium.org/166653003 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@19380 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/a64/code-stubs-a64.cc | 11 ++++++----- src/a64/stub-cache-a64.cc | 8 +++++--- src/arm/code-stubs-arm.cc | 11 ++++++----- src/arm/stub-cache-arm.cc | 7 ++++--- src/code-stubs.h | 7 ++++--- src/hydrogen.cc | 4 +++- src/ia32/code-stubs-ia32.cc | 11 ++++++----- src/ia32/stub-cache-ia32.cc | 7 ++++--- src/x64/code-stubs-x64.cc | 12 +++++++----- src/x64/stub-cache-x64.cc | 7 ++++--- test/cctest/test-accessors.cc | 15 ++++++++------- test/cctest/test-api.cc | 37 ++++++++++++++++++++++--------------- 12 files changed, 79 insertions(+), 58 deletions(-) diff --git a/src/a64/code-stubs-a64.cc b/src/a64/code-stubs-a64.cc index 269b97f..40aa0b6 100644 --- a/src/a64/code-stubs-a64.cc +++ b/src/a64/code-stubs-a64.cc @@ -5581,7 +5581,7 @@ void CallApiFunctionStub::Generate(MacroAssembler* masm) { Register context = cp; int argc = ArgumentBits::decode(bit_field_); - bool restore_context = RestoreContextBits::decode(bit_field_); + bool is_store = IsStoreBits::decode(bit_field_); bool call_data_undefined = CallDataUndefinedBits::decode(bit_field_); typedef FunctionCallbackArguments FCA; @@ -5654,8 +5654,10 @@ void CallApiFunctionStub::Generate(MacroAssembler* masm) { AllowExternalCallThatCantCauseGC scope(masm); MemOperand context_restore_operand( fp, (2 + FCA::kContextSaveIndex) * kPointerSize); - MemOperand return_value_operand(fp, - (2 + FCA::kReturnValueOffset) * kPointerSize); + // Stores return the first js argument + int return_value_offset = + 2 + (is_store ? FCA::kArgsLength : FCA::kReturnValueOffset); + MemOperand return_value_operand(fp, return_value_offset * kPointerSize); const int spill_offset = 1 + kApiStackSpace; __ CallApiFunctionAndReturn(api_function_address, @@ -5663,8 +5665,7 @@ void CallApiFunctionStub::Generate(MacroAssembler* masm) { kStackUnwindSpace, spill_offset, return_value_operand, - restore_context ? - &context_restore_operand : NULL); + &context_restore_operand); } diff --git a/src/a64/stub-cache-a64.cc b/src/a64/stub-cache-a64.cc index 19f9dd6..adb8355 100644 --- a/src/a64/stub-cache-a64.cc +++ b/src/a64/stub-cache-a64.cc @@ -750,6 +750,7 @@ static void GenerateFastApiCall(MacroAssembler* masm, Handle receiver_map, Register receiver, Register scratch, + bool is_store, int argc, Register* values) { ASSERT(!AreAliased(receiver, scratch)); @@ -815,7 +816,7 @@ static void GenerateFastApiCall(MacroAssembler* masm, __ Mov(api_function_address, Operand(ref)); // Jump to stub. - CallApiFunctionStub stub(true, call_data_undefined, argc); + CallApiFunctionStub stub(is_store, call_data_undefined, argc); __ TailCallStub(&stub); } @@ -1047,7 +1048,8 @@ void LoadStubCompiler::GenerateLoadCallback( const CallOptimization& call_optimization, Handle receiver_map) { GenerateFastApiCall( - masm(), call_optimization, receiver_map, receiver(), scratch3(), 0, NULL); + masm(), call_optimization, receiver_map, + receiver(), scratch3(), false, 0, NULL); } @@ -1540,7 +1542,7 @@ Handle StoreStubCompiler::CompileStoreCallback( Register values[] = { value() }; GenerateFastApiCall(masm(), call_optimization, handle(object->map()), - receiver(), scratch3(), 1, values); + receiver(), scratch3(), true, 1, values); // Return the generated code. return GetCode(kind(), Code::FAST, name); diff --git a/src/arm/code-stubs-arm.cc b/src/arm/code-stubs-arm.cc index 282a6d8..10818ce 100644 --- a/src/arm/code-stubs-arm.cc +++ b/src/arm/code-stubs-arm.cc @@ -5448,7 +5448,7 @@ void CallApiFunctionStub::Generate(MacroAssembler* masm) { Register context = cp; int argc = ArgumentBits::decode(bit_field_); - bool restore_context = RestoreContextBits::decode(bit_field_); + bool is_store = IsStoreBits::decode(bit_field_); bool call_data_undefined = CallDataUndefinedBits::decode(bit_field_); typedef FunctionCallbackArguments FCA; @@ -5526,15 +5526,16 @@ void CallApiFunctionStub::Generate(MacroAssembler* masm) { AllowExternalCallThatCantCauseGC scope(masm); MemOperand context_restore_operand( fp, (2 + FCA::kContextSaveIndex) * kPointerSize); - MemOperand return_value_operand(fp, - (2 + FCA::kReturnValueOffset) * kPointerSize); + // Stores return the first js argument + int return_value_offset = + 2 + (is_store ? FCA::kArgsLength : FCA::kReturnValueOffset); + MemOperand return_value_operand(fp, return_value_offset * kPointerSize); __ CallApiFunctionAndReturn(api_function_address, thunk_ref, kStackUnwindSpace, return_value_operand, - restore_context ? - &context_restore_operand : NULL); + &context_restore_operand); } diff --git a/src/arm/stub-cache-arm.cc b/src/arm/stub-cache-arm.cc index 694a4ed..dd60cc9 100644 --- a/src/arm/stub-cache-arm.cc +++ b/src/arm/stub-cache-arm.cc @@ -788,6 +788,7 @@ static void GenerateFastApiCall(MacroAssembler* masm, Handle receiver_map, Register receiver, Register scratch_in, + bool is_store, int argc, Register* values) { ASSERT(!receiver.is(scratch_in)); @@ -854,7 +855,7 @@ static void GenerateFastApiCall(MacroAssembler* masm, __ mov(api_function_address, Operand(ref)); // Jump to stub. - CallApiFunctionStub stub(true, call_data_undefined, argc); + CallApiFunctionStub stub(is_store, call_data_undefined, argc); __ TailCallStub(&stub); } @@ -1080,7 +1081,7 @@ void LoadStubCompiler::GenerateLoadCallback( Handle receiver_map) { GenerateFastApiCall( masm(), call_optimization, receiver_map, - receiver(), scratch3(), 0, NULL); + receiver(), scratch3(), false, 0, NULL); } @@ -1271,7 +1272,7 @@ Handle StoreStubCompiler::CompileStoreCallback( Register values[] = { value() }; GenerateFastApiCall( masm(), call_optimization, handle(object->map()), - receiver(), scratch3(), 1, values); + receiver(), scratch3(), true, 1, values); // Return the generated code. return GetCode(kind(), Code::FAST, name); diff --git a/src/code-stubs.h b/src/code-stubs.h index d568e34..01d9fe3 100644 --- a/src/code-stubs.h +++ b/src/code-stubs.h @@ -1012,13 +1012,14 @@ class StoreGlobalStub : public HandlerStub { class CallApiFunctionStub : public PlatformCodeStub { public: - CallApiFunctionStub(bool restore_context, + CallApiFunctionStub(bool is_store, bool call_data_undefined, int argc) { bit_field_ = - RestoreContextBits::encode(restore_context) | + IsStoreBits::encode(is_store) | CallDataUndefinedBits::encode(call_data_undefined) | ArgumentBits::encode(argc); + ASSERT(!is_store || argc == 1); } private: @@ -1026,7 +1027,7 @@ class CallApiFunctionStub : public PlatformCodeStub { virtual Major MajorKey() V8_OVERRIDE { return CallApiFunction; } virtual int MinorKey() V8_OVERRIDE { return bit_field_; } - class RestoreContextBits: public BitField {}; + class IsStoreBits: public BitField {}; class CallDataUndefinedBits: public BitField {}; class ArgumentBits: public BitField {}; diff --git a/src/hydrogen.cc b/src/hydrogen.cc index ff98779..f7d2a1f 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -7783,6 +7783,7 @@ bool HOptimizedGraphBuilder::TryInlineApiCall(Handle function, } bool drop_extra = false; + bool is_store = false; switch (call_type) { case kCallApiFunction: case kCallApiMethod: @@ -7809,6 +7810,7 @@ bool HOptimizedGraphBuilder::TryInlineApiCall(Handle function, break; case kCallApiSetter: { + is_store = true; // Receiver and prototype chain cannot have changed. ASSERT_EQ(1, argc); ASSERT_EQ(NULL, receiver); @@ -7854,7 +7856,7 @@ bool HOptimizedGraphBuilder::TryInlineApiCall(Handle function, CallInterfaceDescriptor* descriptor = isolate()->call_descriptor(Isolate::ApiFunctionCall); - CallApiFunctionStub stub(true, call_data_is_undefined, argc); + CallApiFunctionStub stub(is_store, call_data_is_undefined, argc); Handle code = stub.GetCode(isolate()); HConstant* code_value = Add(code); diff --git a/src/ia32/code-stubs-ia32.cc b/src/ia32/code-stubs-ia32.cc index eddd571..9939c52 100644 --- a/src/ia32/code-stubs-ia32.cc +++ b/src/ia32/code-stubs-ia32.cc @@ -5324,7 +5324,7 @@ void CallApiFunctionStub::Generate(MacroAssembler* masm) { Register context = esi; int argc = ArgumentBits::decode(bit_field_); - bool restore_context = RestoreContextBits::decode(bit_field_); + bool is_store = IsStoreBits::decode(bit_field_); bool call_data_undefined = CallDataUndefinedBits::decode(bit_field_); typedef FunctionCallbackArguments FCA; @@ -5405,15 +5405,16 @@ void CallApiFunctionStub::Generate(MacroAssembler* masm) { Operand context_restore_operand(ebp, (2 + FCA::kContextSaveIndex) * kPointerSize); - Operand return_value_operand(ebp, - (2 + FCA::kReturnValueOffset) * kPointerSize); + // Stores return the first js argument + int return_value_offset = + 2 + (is_store ? FCA::kArgsLength : FCA::kReturnValueOffset); + Operand return_value_operand(ebp, return_value_offset * kPointerSize); __ CallApiFunctionAndReturn(api_function_address, thunk_address, ApiParameterOperand(1), argc + FCA::kArgsLength + 1, return_value_operand, - restore_context ? - &context_restore_operand : NULL); + &context_restore_operand); } diff --git a/src/ia32/stub-cache-ia32.cc b/src/ia32/stub-cache-ia32.cc index a5b93b9..0974126 100644 --- a/src/ia32/stub-cache-ia32.cc +++ b/src/ia32/stub-cache-ia32.cc @@ -427,6 +427,7 @@ static void GenerateFastApiCall(MacroAssembler* masm, Handle receiver_map, Register receiver, Register scratch_in, + bool is_store, int argc, Register* values) { // Copy return value. @@ -493,7 +494,7 @@ static void GenerateFastApiCall(MacroAssembler* masm, __ mov(api_function_address, Immediate(function_address)); // Jump to stub. - CallApiFunctionStub stub(true, call_data_undefined, argc); + CallApiFunctionStub stub(is_store, call_data_undefined, argc); __ TailCallStub(&stub); } @@ -1070,7 +1071,7 @@ void LoadStubCompiler::GenerateLoadCallback( Handle receiver_map) { GenerateFastApiCall( masm(), call_optimization, receiver_map, - receiver(), scratch1(), 0, NULL); + receiver(), scratch1(), false, 0, NULL); } @@ -1274,7 +1275,7 @@ Handle StoreStubCompiler::CompileStoreCallback( Register values[] = { value() }; GenerateFastApiCall( masm(), call_optimization, handle(object->map()), - receiver(), scratch1(), 1, values); + receiver(), scratch1(), true, 1, values); // Return the generated code. return GetCode(kind(), Code::FAST, name); diff --git a/src/x64/code-stubs-x64.cc b/src/x64/code-stubs-x64.cc index 0637bd2..8c5a430 100644 --- a/src/x64/code-stubs-x64.cc +++ b/src/x64/code-stubs-x64.cc @@ -5177,7 +5177,7 @@ void CallApiFunctionStub::Generate(MacroAssembler* masm) { Register context = rsi; int argc = ArgumentBits::decode(bit_field_); - bool restore_context = RestoreContextBits::decode(bit_field_); + bool is_store = IsStoreBits::decode(bit_field_); bool call_data_undefined = CallDataUndefinedBits::decode(bit_field_); typedef FunctionCallbackArguments FCA; @@ -5253,19 +5253,21 @@ void CallApiFunctionStub::Generate(MacroAssembler* masm) { Address thunk_address = FUNCTION_ADDR(&InvokeFunctionCallback); - StackArgumentsAccessor args_from_rbp(rbp, FCA::kArgsLength, + // Accessor for FunctionCallbackInfo and first js arg. + StackArgumentsAccessor args_from_rbp(rbp, FCA::kArgsLength + 1, ARGUMENTS_DONT_CONTAIN_RECEIVER); Operand context_restore_operand = args_from_rbp.GetArgumentOperand( - FCA::kArgsLength - 1 - FCA::kContextSaveIndex); + FCA::kArgsLength - FCA::kContextSaveIndex); + // Stores return the first js argument Operand return_value_operand = args_from_rbp.GetArgumentOperand( - FCA::kArgsLength - 1 - FCA::kReturnValueOffset); + is_store ? 0 : FCA::kArgsLength - FCA::kReturnValueOffset); __ CallApiFunctionAndReturn( api_function_address, thunk_address, callback_arg, argc + FCA::kArgsLength + 1, return_value_operand, - restore_context ? &context_restore_operand : NULL); + &context_restore_operand); } diff --git a/src/x64/stub-cache-x64.cc b/src/x64/stub-cache-x64.cc index a43d709..9064fbd 100644 --- a/src/x64/stub-cache-x64.cc +++ b/src/x64/stub-cache-x64.cc @@ -398,6 +398,7 @@ static void GenerateFastApiCall(MacroAssembler* masm, Handle receiver_map, Register receiver, Register scratch_in, + bool is_store, int argc, Register* values) { ASSERT(optimization.is_simple_api_call()); @@ -465,7 +466,7 @@ static void GenerateFastApiCall(MacroAssembler* masm, api_function_address, function_address, RelocInfo::EXTERNAL_REFERENCE); // Jump to stub. - CallApiFunctionStub stub(true, call_data_undefined, argc); + CallApiFunctionStub stub(is_store, call_data_undefined, argc); __ TailCallStub(&stub); } @@ -974,7 +975,7 @@ void LoadStubCompiler::GenerateLoadCallback( Handle receiver_map) { GenerateFastApiCall( masm(), call_optimization, receiver_map, - receiver(), scratch1(), 0, NULL); + receiver(), scratch1(), false, 0, NULL); } @@ -1169,7 +1170,7 @@ Handle StoreStubCompiler::CompileStoreCallback( Register values[] = { value() }; GenerateFastApiCall( masm(), call_optimization, handle(object->map()), - receiver(), scratch1(), 1, values); + receiver(), scratch1(), true, 1, values); // Return the generated code. return GetCode(kind(), Code::FAST, name); diff --git a/test/cctest/test-accessors.cc b/test/cctest/test-accessors.cc index bda09f0..daafb24 100644 --- a/test/cctest/test-accessors.cc +++ b/test/cctest/test-accessors.cc @@ -174,6 +174,7 @@ static void XSetter(Local value, const Info& info, int offset) { CHECK_EQ(x_holder, info.This()); CHECK_EQ(x_holder, info.Holder()); x_register[offset] = value->Int32Value(); + info.GetReturnValue().Set(v8_num(-1)); } @@ -210,20 +211,20 @@ THREADED_TEST(AccessorIC) { "var key_1 = 'x1';" "for (var j = 0; j < 10; j++) {" " var i = 4*j;" - " holder.x0 = i;" + " result.push(holder.x0 = i);" " result.push(obj.x0);" - " holder.x1 = i + 1;" + " result.push(holder.x1 = i + 1);" " result.push(obj.x1);" - " holder[key_0] = i + 2;" + " result.push(holder[key_0] = i + 2);" " result.push(obj[key_0]);" - " holder[key_1] = i + 3;" + " result.push(holder[key_1] = i + 3);" " result.push(obj[key_1]);" "}" "result")); - CHECK_EQ(40, array->Length()); - for (int i = 0; i < 40; i++) { + CHECK_EQ(80, array->Length()); + for (int i = 0; i < 80; i++) { v8::Handle entry = array->Get(v8::Integer::New(isolate, i)); - CHECK_EQ(v8::Integer::New(isolate, i), entry); + CHECK_EQ(v8::Integer::New(isolate, i/2), entry); } } diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 4bef425..58470fc 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -21933,6 +21933,7 @@ class ApiCallOptimizationChecker { } CHECK(holder == info.Holder()); count++; + info.GetReturnValue().Set(v8_str("returned")); } // TODO(dcarney): move this to v8.h @@ -22055,39 +22056,45 @@ class ApiCallOptimizationChecker { wrap_function, "function wrap_f_%d() { var f = g_f; return f(); }\n" "function wrap_get_%d() { return this.g_acc; }\n" - "function wrap_set_%d() { this.g_acc = 1; }\n", + "function wrap_set_%d() { return this.g_acc = 1; }\n", key, key, key); } else { i::OS::SNPrintF( wrap_function, "function wrap_f_%d() { return receiver_subclass.f(); }\n" "function wrap_get_%d() { return receiver_subclass.acc; }\n" - "function wrap_set_%d() { receiver_subclass.acc = 1; }\n", + "function wrap_set_%d() { return receiver_subclass.acc = 1; }\n", key, key, key); } // build source string - i::ScopedVector source(500); + i::ScopedVector source(1000); i::OS::SNPrintF( source, "%s\n" // wrap functions - "function wrap_f() { wrap_f_%d(); }\n" - "function wrap_get() { wrap_get_%d(); }\n" - "function wrap_set() { wrap_set_%d(); }\n" + "function wrap_f() { return wrap_f_%d(); }\n" + "function wrap_get() { return wrap_get_%d(); }\n" + "function wrap_set() { return wrap_set_%d(); }\n" + "check = function(returned) {\n" + " if (returned !== 'returned') { throw returned; }\n" + "}\n" "\n" - "wrap_f();\n" - "wrap_f();\n" + "check(wrap_f());\n" + "check(wrap_f());\n" "%%OptimizeFunctionOnNextCall(wrap_f_%d);\n" - "wrap_f();\n" + "check(wrap_f());\n" "\n" - "wrap_get();\n" - "wrap_get();\n" + "check(wrap_get());\n" + "check(wrap_get());\n" "%%OptimizeFunctionOnNextCall(wrap_get_%d);\n" - "wrap_get();\n" + "check(wrap_get());\n" "\n" - "wrap_set();\n" - "wrap_set();\n" + "check = function(returned) {\n" + " if (returned !== 1) { throw returned; }\n" + "}\n" + "check(wrap_set());\n" + "check(wrap_set());\n" "%%OptimizeFunctionOnNextCall(wrap_set_%d);\n" - "wrap_set();\n", + "check(wrap_set());\n", wrap_function.start(), key, key, key, key, key, key); v8::TryCatch try_catch; CompileRun(source.start()); -- 2.7.4