From 8d334ed1dca922c76853591ec3b3f9beb5db8f7a Mon Sep 17 00:00:00 2001 From: "alexandre.rames@arm.com" Date: Fri, 23 May 2014 14:06:42 +0000 Subject: [PATCH] Allow HPushArgument to handle more than one argument. R=ulan@chromium.org Review URL: https://codereview.chromium.org/296113008 git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@21468 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/lithium-arm.cc | 10 +++++-- src/arm64/lithium-arm64.cc | 18 ++++++++++-- src/arm64/lithium-arm64.h | 48 ++++++++++++++++++++++++++++---- src/arm64/lithium-codegen-arm64.cc | 27 +++++++++++++----- src/arm64/macro-assembler-arm64.cc | 7 +++-- src/arm64/macro-assembler-arm64.h | 24 +++++++++------- src/code-stubs-hydrogen.cc | 2 +- src/hydrogen-instructions.cc | 2 +- src/hydrogen-instructions.h | 57 +++++++++++++++++++++++++++++++++----- src/hydrogen.cc | 55 +++++++++++++++++------------------- src/ia32/lithium-ia32.cc | 10 +++++-- src/lithium.cc | 4 +-- src/mips/lithium-mips.cc | 10 +++++-- src/x64/lithium-x64.cc | 10 +++++-- 14 files changed, 204 insertions(+), 80 deletions(-) diff --git a/src/arm/lithium-arm.cc b/src/arm/lithium-arm.cc index 5ffe74a..5e4b23e 100644 --- a/src/arm/lithium-arm.cc +++ b/src/arm/lithium-arm.cc @@ -1005,9 +1005,13 @@ LInstruction* LChunkBuilder::DoApplyArguments(HApplyArguments* instr) { } -LInstruction* LChunkBuilder::DoPushArgument(HPushArgument* instr) { - LOperand* argument = Use(instr->argument()); - return new(zone()) LPushArgument(argument); +LInstruction* LChunkBuilder::DoPushArguments(HPushArguments* instr) { + int argc = instr->OperandCount(); + for (int i = 0; i < argc; ++i) { + LOperand* argument = Use(instr->argument(i)); + AddInstruction(new(zone()) LPushArgument(argument), instr); + } + return NULL; } diff --git a/src/arm64/lithium-arm64.cc b/src/arm64/lithium-arm64.cc index d6b4176..50e0f44 100644 --- a/src/arm64/lithium-arm64.cc +++ b/src/arm64/lithium-arm64.cc @@ -1995,9 +1995,21 @@ LInstruction* LChunkBuilder::DoPower(HPower* instr) { } -LInstruction* LChunkBuilder::DoPushArgument(HPushArgument* instr) { - LOperand* argument = UseRegister(instr->argument()); - return new(zone()) LPushArgument(argument); +LInstruction* LChunkBuilder::DoPushArguments(HPushArguments* instr) { + int argc = instr->OperandCount(); + AddInstruction(new(zone()) LPreparePushArguments(argc), instr); + + LPushArguments* push_args = new(zone()) LPushArguments(zone()); + + for (int i = 0; i < argc; ++i) { + if (push_args->ShouldSplitPush()) { + AddInstruction(push_args, instr); + push_args = new(zone()) LPushArguments(zone()); + } + push_args->AddArgument(UseRegister(instr->argument(i))); + } + + return push_args; } diff --git a/src/arm64/lithium-arm64.h b/src/arm64/lithium-arm64.h index bca5e54..72ae57d 100644 --- a/src/arm64/lithium-arm64.h +++ b/src/arm64/lithium-arm64.h @@ -136,7 +136,8 @@ class LCodeGen; V(OsrEntry) \ V(Parameter) \ V(Power) \ - V(PushArgument) \ + V(PreparePushArguments) \ + V(PushArguments) \ V(RegExpLiteral) \ V(Return) \ V(SeqStringGetChar) \ @@ -2249,15 +2250,50 @@ class LPower V8_FINAL : public LTemplateInstruction<1, 2, 0> { }; -class LPushArgument V8_FINAL : public LTemplateInstruction<0, 1, 0> { +class LPreparePushArguments V8_FINAL : public LTemplateInstruction<0, 0, 0> { public: - explicit LPushArgument(LOperand* value) { - inputs_[0] = value; + explicit LPreparePushArguments(int argc) : argc_(argc) {} + + inline int argc() const { return argc_; } + + DECLARE_CONCRETE_INSTRUCTION(PreparePushArguments, "prepare-push-arguments") + + protected: + int argc_; +}; + + +class LPushArguments V8_FINAL : public LTemplateResultInstruction<0> { + public: + explicit LPushArguments(Zone* zone, + int capacity = kRecommendedMaxPushedArgs) + : zone_(zone), inputs_(capacity, zone) {} + + LOperand* argument(int i) { return inputs_[i]; } + int ArgumentCount() const { return inputs_.length(); } + + void AddArgument(LOperand* arg) { inputs_.Add(arg, zone_); } + + DECLARE_CONCRETE_INSTRUCTION(PushArguments, "push-arguments") + + // It is better to limit the number of arguments pushed simultaneously to + // avoid pressure on the register allocator. + static const int kRecommendedMaxPushedArgs = 4; + bool ShouldSplitPush() const { + return inputs_.length() >= kRecommendedMaxPushedArgs; } - LOperand* value() { return inputs_[0]; } + protected: + Zone* zone_; + ZoneList inputs_; - DECLARE_CONCRETE_INSTRUCTION(PushArgument, "push-argument") + private: + // Iterator support. + virtual int InputCount() V8_FINAL V8_OVERRIDE { return inputs_.length(); } + virtual LOperand* InputAt(int i) V8_FINAL V8_OVERRIDE { return inputs_[i]; } + + virtual int TempCount() V8_FINAL V8_OVERRIDE { return 0; } + virtual LOperand* TempAt(int i) V8_FINAL V8_OVERRIDE { return NULL; } }; diff --git a/src/arm64/lithium-codegen-arm64.cc b/src/arm64/lithium-codegen-arm64.cc index 4ecc434..9161aaf 100644 --- a/src/arm64/lithium-codegen-arm64.cc +++ b/src/arm64/lithium-codegen-arm64.cc @@ -4677,14 +4677,27 @@ void LCodeGen::DoParameter(LParameter* instr) { } -void LCodeGen::DoPushArgument(LPushArgument* instr) { - LOperand* argument = instr->value(); - if (argument->IsDoubleRegister() || argument->IsDoubleStackSlot()) { - Abort(kDoPushArgumentNotImplementedForDoubleType); - } else { - __ Push(ToRegister(argument)); - after_push_argument_ = true; +void LCodeGen::DoPreparePushArguments(LPreparePushArguments* instr) { + __ PushPreamble(instr->argc(), kPointerSize); +} + + +void LCodeGen::DoPushArguments(LPushArguments* instr) { + MacroAssembler::PushPopQueue args(masm()); + + for (int i = 0; i < instr->ArgumentCount(); ++i) { + LOperand* arg = instr->argument(i); + if (arg->IsDoubleRegister() || arg->IsDoubleStackSlot()) { + Abort(kDoPushArgumentNotImplementedForDoubleType); + return; + } + args.Queue(ToRegister(arg)); } + + // The preamble was done by LPreparePushArguments. + args.PushQueued(MacroAssembler::PushPopQueue::SKIP_PREAMBLE); + + after_push_argument_ = true; } diff --git a/src/arm64/macro-assembler-arm64.cc b/src/arm64/macro-assembler-arm64.cc index d961713..f485de4 100644 --- a/src/arm64/macro-assembler-arm64.cc +++ b/src/arm64/macro-assembler-arm64.cc @@ -805,10 +805,13 @@ void MacroAssembler::Pop(const CPURegister& dst0, const CPURegister& dst1, } -void MacroAssembler::PushPopQueue::PushQueued() { +void MacroAssembler::PushPopQueue::PushQueued( + PreambleDirective preamble_directive) { if (queued_.empty()) return; - masm_->PushPreamble(size_); + if (preamble_directive == WITH_PREAMBLE) { + masm_->PushPreamble(size_); + } int count = queued_.size(); int index = 0; diff --git a/src/arm64/macro-assembler-arm64.h b/src/arm64/macro-assembler-arm64.h index 48f2e77..8b761ee 100644 --- a/src/arm64/macro-assembler-arm64.h +++ b/src/arm64/macro-assembler-arm64.h @@ -614,7 +614,11 @@ class MacroAssembler : public Assembler { queued_.push_back(rt); } - void PushQueued(); + enum PreambleDirective { + WITH_PREAMBLE, + SKIP_PREAMBLE + }; + void PushQueued(PreambleDirective preamble_directive = WITH_PREAMBLE); void PopQueued(); private: @@ -2012,6 +2016,15 @@ class MacroAssembler : public Assembler { void JumpIfDictionaryInPrototypeChain(Register object, Register scratch0, Register scratch1, Label* found); + // Perform necessary maintenance operations before a push or after a pop. + // + // Note that size is specified in bytes. + void PushPreamble(Operand total_size); + void PopPostamble(Operand total_size); + + void PushPreamble(int count, int size) { PushPreamble(count * size); } + void PopPostamble(int count, int size) { PopPostamble(count * size); } + private: // Helpers for CopyFields. // These each implement CopyFields in a different way. @@ -2039,15 +2052,6 @@ class MacroAssembler : public Assembler { const CPURegister& dst0, const CPURegister& dst1, const CPURegister& dst2, const CPURegister& dst3); - // Perform necessary maintenance operations before a push or after a pop. - // - // Note that size is specified in bytes. - void PushPreamble(Operand total_size); - void PopPostamble(Operand total_size); - - void PushPreamble(int count, int size) { PushPreamble(count * size); } - void PopPostamble(int count, int size) { PopPostamble(count * size); } - // Call Printf. On a native build, a simple call will be generated, but if the // simulator is being used then a suitable pseudo-instruction is used. The // arguments and stack (csp) must be prepared by the caller as for a normal diff --git a/src/code-stubs-hydrogen.cc b/src/code-stubs-hydrogen.cc index bb50d92..46d8551 100644 --- a/src/code-stubs-hydrogen.cc +++ b/src/code-stubs-hydrogen.cc @@ -298,7 +298,7 @@ HValue* CodeStubGraphBuilder::BuildCodeStub() { // Convert the parameter to number using the builtin. HValue* function = AddLoadJSBuiltin(Builtins::TO_NUMBER); - Add(value); + Add(zone(), value); Push(Add(function, 1)); if_number.End(); diff --git a/src/hydrogen-instructions.cc b/src/hydrogen-instructions.cc index 9bd0464..77179a7 100644 --- a/src/hydrogen-instructions.cc +++ b/src/hydrogen-instructions.cc @@ -868,7 +868,7 @@ bool HInstruction::CanDeoptimize() { case HValue::kMathMinMax: case HValue::kParameter: case HValue::kPhi: - case HValue::kPushArgument: + case HValue::kPushArguments: case HValue::kRegExpLiteral: case HValue::kReturn: case HValue::kRor: diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h index 196f8f0..4f83aaf 100644 --- a/src/hydrogen-instructions.h +++ b/src/hydrogen-instructions.h @@ -127,7 +127,7 @@ class LChunkBuilder; V(OsrEntry) \ V(Parameter) \ V(Power) \ - V(PushArgument) \ + V(PushArguments) \ V(RegExpLiteral) \ V(Return) \ V(Ror) \ @@ -2176,23 +2176,66 @@ class HLeaveInlined V8_FINAL : public HTemplateInstruction<0> { }; -class HPushArgument V8_FINAL : public HUnaryOperation { +class HPushArguments V8_FINAL : public HInstruction { public: - DECLARE_INSTRUCTION_FACTORY_P1(HPushArgument, HValue*); + DECLARE_INSTRUCTION_FACTORY_P1(HPushArguments, Zone*); + DECLARE_INSTRUCTION_FACTORY_P2(HPushArguments, Zone*, HValue*); + DECLARE_INSTRUCTION_FACTORY_P3(HPushArguments, Zone*, HValue*, HValue*); + DECLARE_INSTRUCTION_FACTORY_P4(HPushArguments, Zone*, + HValue*, HValue*, HValue*); + DECLARE_INSTRUCTION_FACTORY_P5(HPushArguments, Zone*, + HValue*, HValue*, HValue*, HValue*); virtual Representation RequiredInputRepresentation(int index) V8_OVERRIDE { return Representation::Tagged(); } - virtual int argument_delta() const V8_OVERRIDE { return 1; } - HValue* argument() { return OperandAt(0); } + virtual int argument_delta() const V8_OVERRIDE { return inputs_.length(); } + HValue* argument(int i) { return OperandAt(i); } - DECLARE_CONCRETE_INSTRUCTION(PushArgument) + virtual int OperandCount() V8_FINAL V8_OVERRIDE { return inputs_.length(); } + virtual HValue* OperandAt(int i) const V8_FINAL V8_OVERRIDE { + return inputs_[i]; + } + + void AddArgument(HValue* arg) { + inputs_.Add(NULL, zone_); + SetOperandAt(inputs_.length() - 1, arg); + } + + DECLARE_CONCRETE_INSTRUCTION(PushArguments) + + protected: + virtual void InternalSetOperandAt(int i, HValue* value) V8_FINAL V8_OVERRIDE { + inputs_[i] = value; + } private: - explicit HPushArgument(HValue* value) : HUnaryOperation(value) { + HPushArguments(Zone* zone, + HValue* arg1 = NULL, HValue* arg2 = NULL, + HValue* arg3 = NULL, HValue* arg4 = NULL) + : HInstruction(HType::Tagged()), zone_(zone), inputs_(4, zone) { set_representation(Representation::Tagged()); + if (arg1) { + inputs_.Add(NULL, zone); + SetOperandAt(0, arg1); + } + if (arg2) { + inputs_.Add(NULL, zone); + SetOperandAt(1, arg2); + } + if (arg3) { + inputs_.Add(NULL, zone); + SetOperandAt(2, arg3); + } + if (arg4) { + inputs_.Add(NULL, zone); + SetOperandAt(3, arg4); + } } + + Zone* zone_; + ZoneList inputs_; }; diff --git a/src/hydrogen.cc b/src/hydrogen.cc index a4c8d01..457bd09 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -1726,7 +1726,7 @@ HValue* HGraphBuilder::BuildNumberToString(HValue* object, Type* type) { if_found.Else(); { // Cache miss, fallback to runtime. - Add(object); + Add(zone(), object); Push(Add( isolate()->factory()->empty_string(), Runtime::FunctionForId(Runtime::kHiddenNumberToStringSkipCache), @@ -2050,8 +2050,7 @@ HValue* HGraphBuilder::BuildUncheckedStringAdd( if_sameencodingandsequential.Else(); { // Fallback to the runtime to add the two strings. - Add(left); - Add(right); + Add(zone(), left, right); Push(Add( isolate()->factory()->empty_string(), Runtime::FunctionForId(Runtime::kHiddenStringAdd), @@ -4188,9 +4187,11 @@ void HOptimizedGraphBuilder::PushArgumentsFromEnvironment(int count) { arguments.Add(Pop(), zone()); } + HPushArguments* push_args = New(zone()); while (!arguments.is_empty()) { - Add(arguments.RemoveLast()); + push_args->AddArgument(arguments.RemoveLast()); } + AddInstruction(push_args); } @@ -5184,10 +5185,11 @@ void HOptimizedGraphBuilder::VisitObjectLiteral(ObjectLiteral* expr) { flags |= expr->has_function() ? ObjectLiteral::kHasFunction : ObjectLiteral::kNoFlags; - Add(Add(closure_literals)); - Add(Add(literal_index)); - Add(Add(constant_properties)); - Add(Add(flags)); + Add(zone(), + Add(closure_literals), + Add(literal_index), + Add(constant_properties), + Add(flags)); // TODO(mvstanton): Add a flag to turn off creation of any // AllocationMementos for this call: we are in crankshaft and should have @@ -5342,10 +5344,11 @@ void HOptimizedGraphBuilder::VisitArrayLiteral(ArrayLiteral* expr) { : ArrayLiteral::kNoFlags; flags |= ArrayLiteral::kDisableMementos; - Add(Add(literals)); - Add(Add(literal_index)); - Add(Add(constants)); - Add(Add(flags)); + Add(zone(), + Add(literals), + Add(literal_index), + Add(constants), + Add(flags)); // TODO(mvstanton): Consider a flag to turn off creation of any // AllocationMementos for this call: we are in crankshaft and should have @@ -6379,7 +6382,7 @@ void HOptimizedGraphBuilder::VisitThrow(Throw* expr) { HValue* value = environment()->Pop(); if (!FLAG_hydrogen_track_positions) SetSourcePosition(expr->position()); - Add(value); + Add(zone(), value); Add(isolate()->factory()->empty_string(), Runtime::FunctionForId(Runtime::kHiddenThrow), 1); Add(expr->id()); @@ -6781,7 +6784,7 @@ void HOptimizedGraphBuilder::EnsureArgumentsArePushedForAccess() { HInstruction* insert_after = entry; for (int i = 0; i < arguments_values->length(); i++) { HValue* argument = arguments_values->at(i); - HInstruction* push_argument = New(argument); + HInstruction* push_argument = New(zone(), argument); push_argument->InsertAfter(insert_after); insert_after = push_argument; } @@ -8076,7 +8079,7 @@ bool HOptimizedGraphBuilder::TryInlineApiCall(Handle function, ASSERT_EQ(NULL, receiver); // Receiver is on expression stack. receiver = Pop(); - Add(receiver); + Add(zone(), receiver); break; case kCallApiSetter: { @@ -8087,8 +8090,7 @@ bool HOptimizedGraphBuilder::TryInlineApiCall(Handle function, // Receiver and value are on expression stack. HValue* value = Pop(); receiver = Pop(); - Add(receiver); - Add(value); + Add(zone(), receiver, value); break; } } @@ -8656,7 +8658,7 @@ void HOptimizedGraphBuilder::VisitCallNew(CallNew* expr) { } // TODO(mstarzinger): For now we remove the previous HAllocate and all - // corresponding instructions and instead add HPushArgument for the + // corresponding instructions and instead add HPushArguments for the // arguments in case inlining failed. What we actually should do is for // inlining to try to build a subgraph without mutating the parent graph. HInstruction* instr = current_block()->last(); @@ -9135,9 +9137,8 @@ void HOptimizedGraphBuilder::VisitDelete(UnaryOperation* expr) { HValue* key = Pop(); HValue* obj = Pop(); HValue* function = AddLoadJSBuiltin(Builtins::DELETE); - Add(obj); - Add(key); - Add(Add(function_strict_mode())); + Add(zone(), + obj, key, Add(function_strict_mode())); // TODO(olivf) InvokeFunction produces a check for the parameter count, // even though we are certain to pass the correct number of arguments here. HInstruction* instr = New(function, 3); @@ -9622,8 +9623,7 @@ HValue* HGraphBuilder::BuildBinaryOperation( } else if (!left_type->Is(Type::String())) { ASSERT(right_type->Is(Type::String())); HValue* function = AddLoadJSBuiltin(Builtins::STRING_ADD_RIGHT); - Add(left); - Add(right); + Add(zone(), left, right); return AddUncasted(function, 2); } @@ -9634,8 +9634,7 @@ HValue* HGraphBuilder::BuildBinaryOperation( } else if (!right_type->Is(Type::String())) { ASSERT(left_type->Is(Type::String())); HValue* function = AddLoadJSBuiltin(Builtins::STRING_ADD_LEFT); - Add(left); - Add(right); + Add(zone(), left, right); return AddUncasted(function, 2); } @@ -9697,8 +9696,7 @@ HValue* HGraphBuilder::BuildBinaryOperation( // operation in optimized code, which is more expensive, than a stub call. if (graph()->info()->IsStub() && is_non_primitive) { HValue* function = AddLoadJSBuiltin(BinaryOpIC::TokenToJSBuiltin(op)); - Add(left); - Add(right); + Add(zone(), left, right); instr = AddUncasted(function, 2); } else { switch (op) { @@ -10062,8 +10060,7 @@ void HOptimizedGraphBuilder::VisitCompareOperation(CompareOperation* expr) { UNREACHABLE(); } else if (op == Token::IN) { HValue* function = AddLoadJSBuiltin(Builtins::IN); - Add(left); - Add(right); + Add(zone(), left, right); // TODO(olivf) InvokeFunction produces a check for the parameter count, // even though we are certain to pass the correct number of arguments here. HInstruction* result = New(function, 2); diff --git a/src/ia32/lithium-ia32.cc b/src/ia32/lithium-ia32.cc index 4e3f561..6c13443 100644 --- a/src/ia32/lithium-ia32.cc +++ b/src/ia32/lithium-ia32.cc @@ -1060,9 +1060,13 @@ LInstruction* LChunkBuilder::DoApplyArguments(HApplyArguments* instr) { } -LInstruction* LChunkBuilder::DoPushArgument(HPushArgument* instr) { - LOperand* argument = UseAny(instr->argument()); - return new(zone()) LPushArgument(argument); +LInstruction* LChunkBuilder::DoPushArguments(HPushArguments* instr) { + int argc = instr->OperandCount(); + for (int i = 0; i < argc; ++i) { + LOperand* argument = UseAny(instr->argument(i)); + AddInstruction(new(zone()) LPushArgument(argument), instr); + } + return NULL; } diff --git a/src/lithium.cc b/src/lithium.cc index 0001e39..25d716d 100644 --- a/src/lithium.cc +++ b/src/lithium.cc @@ -509,7 +509,7 @@ LEnvironment* LChunkBuilderBase::CreateEnvironment( LOperand* op; HValue* value = hydrogen_env->values()->at(i); - CHECK(!value->IsPushArgument()); // Do not deopt outgoing arguments + CHECK(!value->IsPushArguments()); // Do not deopt outgoing arguments if (value->IsArgumentsObject() || value->IsCapturedObject()) { op = LEnvironment::materialization_marker(); } else { @@ -590,7 +590,7 @@ void LChunkBuilderBase::AddObjectToMaterialize(HValue* value, // Insert a hole for nested objects op = LEnvironment::materialization_marker(); } else { - ASSERT(!arg_value->IsPushArgument()); + ASSERT(!arg_value->IsPushArguments()); // For ordinary values, tell the register allocator we need the value // to be alive here op = UseAny(arg_value); diff --git a/src/mips/lithium-mips.cc b/src/mips/lithium-mips.cc index e78dde1..562c88d 100644 --- a/src/mips/lithium-mips.cc +++ b/src/mips/lithium-mips.cc @@ -1008,9 +1008,13 @@ LInstruction* LChunkBuilder::DoApplyArguments(HApplyArguments* instr) { } -LInstruction* LChunkBuilder::DoPushArgument(HPushArgument* instr) { - LOperand* argument = Use(instr->argument()); - return new(zone()) LPushArgument(argument); +LInstruction* LChunkBuilder::DoPushArguments(HPushArguments* instr) { + int argc = instr->OperandCount(); + for (int i = 0; i < argc; ++i) { + LOperand* argument = Use(instr->argument(i)); + AddInstruction(new(zone()) LPushArgument(argument), instr); + } + return NULL; } diff --git a/src/x64/lithium-x64.cc b/src/x64/lithium-x64.cc index 4c1ae26..a3bd9f6 100644 --- a/src/x64/lithium-x64.cc +++ b/src/x64/lithium-x64.cc @@ -1023,9 +1023,13 @@ LInstruction* LChunkBuilder::DoApplyArguments(HApplyArguments* instr) { } -LInstruction* LChunkBuilder::DoPushArgument(HPushArgument* instr) { - LOperand* argument = UseOrConstant(instr->argument()); - return new(zone()) LPushArgument(argument); +LInstruction* LChunkBuilder::DoPushArguments(HPushArguments* instr) { + int argc = instr->OperandCount(); + for (int i = 0; i < argc; ++i) { + LOperand* argument = UseOrConstant(instr->argument(i)); + AddInstruction(new(zone()) LPushArgument(argument), instr); + } + return NULL; } -- 2.7.4