[turbofan] Implement the call protocol properly for direct calls.

The callees are expected to properly set the number of actual
arguments passed to the callee, which is now represented correctly
in the TurboFan graphs by a new Parameter right before the context
Parameter.  Currently this is only being used for outgoing calls.

Note that this requires disabling two of the TF code stub tests,
because of the JavaScript graphs are not automagically compatible
with abitrary (incoming) code stub interface descriptors.  If we
want to support JS code stubs at all, then we need to find a sane
way to feed in this information.

Drive-by-fix: Don't insert a direct call to a classConstructor.

R=mstarzinger@chromium.org
BUG=v8:4413, v8:4428
LOG=n

Review URL: https://codereview.chromium.org/1410633006

Cr-Commit-Position: refs/heads/master@{#31789}
This commit is contained in:
bmeurer 2015-11-04 06:08:46 -08:00 committed by Commit bot
parent e7154a0b78
commit 30aca03ad1
15 changed files with 61 additions and 15 deletions

View File

@ -23,6 +23,7 @@ const Register kInterpreterRegisterFileRegister = {Register::kCode_r4};
const Register kInterpreterBytecodeOffsetRegister = {Register::kCode_r5};
const Register kInterpreterBytecodeArrayRegister = {Register::kCode_r6};
const Register kInterpreterDispatchTableRegister = {Register::kCode_r8};
const Register kJavaScriptCallArgCountRegister = {Register::kCode_r0};
const Register kRuntimeCallFunctionRegister = {Register::kCode_r1};
const Register kRuntimeCallArgCountRegister = {Register::kCode_r0};

View File

@ -44,6 +44,7 @@ namespace internal {
#define kInterpreterBytecodeOffsetRegister x19
#define kInterpreterBytecodeArrayRegister x20
#define kInterpreterDispatchTableRegister x21
#define kJavaScriptCallArgCountRegister x0
#define kRuntimeCallFunctionRegister x1
#define kRuntimeCallArgCountRegister x0

View File

@ -483,9 +483,9 @@ Node* AstGraphBuilder::GetFunctionClosure() {
Node* AstGraphBuilder::GetFunctionContext() {
if (!function_context_.is_set()) {
// Parameter (arity + 1) is special for the outer context of the function
// Parameter (arity + 2) is special for the outer context of the function
const Operator* op = common()->Parameter(
info()->num_parameters_including_this(), "%context");
info()->num_parameters_including_this() + 1, "%context");
Node* node = NewNode(op, graph()->start());
function_context_.set(node);
}
@ -498,8 +498,9 @@ bool AstGraphBuilder::CreateGraph(bool stack_check) {
DCHECK(graph() != NULL);
// Set up the basic structure of the graph. Outputs for {Start} are the formal
// parameters (including the receiver) plus context and closure.
int actual_parameter_count = info()->num_parameters_including_this() + 2;
// parameters (including the receiver) plus number of arguments, context and
// closure.
int actual_parameter_count = info()->num_parameters_including_this() + 3;
graph()->SetStart(graph()->NewNode(common()->Start(actual_parameter_count)));
// Initialize the top-level environment.

View File

@ -134,7 +134,9 @@ Reduction JSInliner::InlineCall(Node* call, Node* context, Node* frame_state,
Node* control = NodeProperties::GetControlInput(call);
Node* effect = NodeProperties::GetEffectInput(call);
// Context is last argument.
int const inlinee_arity_index =
static_cast<int>(start->op()->ValueOutputCount()) - 2;
// Context is last parameter.
int const inlinee_context_index =
static_cast<int>(start->op()->ValueOutputCount()) - 1;
@ -148,10 +150,13 @@ Reduction JSInliner::InlineCall(Node* call, Node* context, Node* frame_state,
case IrOpcode::kParameter: {
int index = 1 + ParameterIndexOf(use->op());
DCHECK_LE(index, inlinee_context_index);
if (index < inliner_inputs && index < inlinee_context_index) {
if (index < inliner_inputs && index < inlinee_arity_index) {
// There is an input from the call, and the index is a value
// projection but not the context, so rewire the input.
Replace(use, call->InputAt(index));
} else if (index == inlinee_arity_index) {
// The projection is requesting the number of arguments.
Replace(use, jsgraph_->Int32Constant(inliner_inputs - 2));
} else if (index == inlinee_context_index) {
// The projection is requesting the inlinee function context.
Replace(use, context);
@ -274,6 +279,15 @@ Reduction JSInliner::ReduceJSCallFunction(Node* node,
return NoChange();
}
// Class constructors are callable, but [[Call]] will raise an exception.
// See ES6 section 9.2.1 [[Call]] ( thisArgument, argumentsList ).
if (IsClassConstructor(function->shared()->kind())) {
TRACE("Not inlining %s into %s because callee is classConstructor\n",
function->shared()->DebugName()->ToCString().get(),
info_->shared_info()->DebugName()->ToCString().get());
return NoChange();
}
if (function->shared()->HasDebugInfo()) {
// Function contains break points.
TRACE("Not inlining %s into %s because callee may contain break points\n",
@ -441,10 +455,10 @@ Reduction JSInliner::ReduceJSCallFunction(Node* node,
}
// Insert argument adaptor frame if required. The callees formal parameter
// count (i.e. value outputs of start node minus target, receiver & context)
// have to match the number of arguments passed to the call.
// count (i.e. value outputs of start node minus target, receiver, num args
// and context) have to match the number of arguments passed to the call.
DCHECK_EQ(static_cast<int>(parameter_count),
start->op()->ValueOutputCount() - 3);
start->op()->ValueOutputCount() - 4);
if (call.formal_arguments() != parameter_count) {
frame_state = CreateArgumentsAdaptorFrameState(&call, info.shared_info());
}

View File

@ -1590,6 +1590,10 @@ Reduction JSTypedLowering::ReduceJSCallFunction(Node* node) {
Handle<JSFunction>::cast(target_type->AsConstant()->Value());
Handle<SharedFunctionInfo> shared(function->shared(), isolate());
// Class constructors are callable, but [[Call]] will raise an exception.
// See ES6 section 9.2.1 [[Call]] ( thisArgument, argumentsList ).
if (IsClassConstructor(shared->kind())) return NoChange();
// Grab the context from the {function}.
Node* context = jsgraph()->Constant(handle(function->context(), isolate()));
NodeProperties::ReplaceContextInput(node, context);
@ -1611,12 +1615,17 @@ Reduction JSTypedLowering::ReduceJSCallFunction(Node* node) {
CallDescriptor::Flags flags = CallDescriptor::kNeedsFrameState;
if (p.AllowTailCalls()) flags |= CallDescriptor::kSupportsTailCalls;
if (shared->internal_formal_parameter_count() == arity) {
if (shared->internal_formal_parameter_count() == arity ||
shared->internal_formal_parameter_count() ==
SharedFunctionInfo::kDontAdaptArgumentsSentinel) {
// Patch {node} to a direct call.
node->InsertInput(graph()->zone(), arity + 2,
jsgraph()->Int32Constant(arity));
NodeProperties::ChangeOp(node,
common()->Call(Linkage::GetJSCallDescriptor(
graph()->zone(), false, 1 + arity, flags)));
} else {
// Patch {node} to an indirect call via the ArgumentsAdaptorTrampoline.
Callable callable = CodeFactory::ArgumentAdaptor(isolate());
node->InsertInput(graph()->zone(), 0,
jsgraph()->HeapConstant(callable.code()));

View File

@ -383,7 +383,9 @@ CallDescriptor* Linkage::GetJSCallDescriptor(Zone* zone, bool is_osr,
CallDescriptor::Flags flags) {
const size_t return_count = 1;
const size_t context_count = 1;
const size_t parameter_count = js_parameter_count + context_count;
const size_t num_args_count = 1;
const size_t parameter_count =
js_parameter_count + num_args_count + context_count;
LocationSignature::Builder locations(zone, return_count, parameter_count);
MachineSignature::Builder types(zone, return_count, parameter_count);
@ -398,6 +400,11 @@ CallDescriptor* Linkage::GetJSCallDescriptor(Zone* zone, bool is_osr,
locations.AddParam(LinkageLocation::ForCallerFrameSlot(spill_slot_index));
types.AddParam(kMachAnyTagged);
}
// Add JavaScript call argument count.
locations.AddParam(regloc(kJavaScriptCallArgCountRegister));
types.AddParam(kMachInt32);
// Add context.
locations.AddParam(regloc(kContextRegister));
types.AddParam(kMachAnyTagged);
@ -544,8 +551,9 @@ LinkageLocation Linkage::GetOsrValueLocation(int index) const {
if (index == kOsrContextSpillSlotIndex) {
// Context. Use the parameter location of the context spill slot.
// Parameter (arity + 1) is special for the context of the function frame.
int context_index = 1 + 1 + parameter_count; // target + receiver + params
// Parameter (arity + 2) is special for the context of the function frame.
int context_index =
1 + 1 + 1 + parameter_count; // target + receiver + params + #args
return incoming_->GetInputLocation(context_index);
} else if (index >= first_stack_slot) {
// Local variable stored in this (callee) stack.

View File

@ -258,7 +258,7 @@ std::ostream& operator<<(std::ostream& os, const CallDescriptor::Kind& k);
//
// #0 #1 #2 #3 [...] #n
// Call[CodeStub] code, arg 1, arg 2, arg 3, [...], context
// Call[JSFunction] function, rcvr, arg 1, arg 2, [...], context
// Call[JSFunction] function, rcvr, arg 1, arg 2, [...], #arg, context
// Call[Runtime] CEntryStub, arg 1, arg 2, arg 3, [...], fun, #arg, context
class Linkage : public ZoneObject {
public:

View File

@ -22,6 +22,7 @@ const Register kInterpreterAccumulatorRegister = {Register::kCode_eax};
const Register kInterpreterRegisterFileRegister = {Register::kCode_edx};
const Register kInterpreterBytecodeOffsetRegister = {Register::kCode_ecx};
const Register kInterpreterBytecodeArrayRegister = {Register::kCode_edi};
const Register kJavaScriptCallArgCountRegister = {Register::kCode_eax};
const Register kRuntimeCallFunctionRegister = {Register::kCode_ebx};
const Register kRuntimeCallArgCountRegister = {Register::kCode_eax};

View File

@ -22,6 +22,7 @@ const Register kInterpreterRegisterFileRegister = {Register::kCode_t3};
const Register kInterpreterBytecodeOffsetRegister = {Register::kCode_t4};
const Register kInterpreterBytecodeArrayRegister = {Register::kCode_t5};
const Register kInterpreterDispatchTableRegister = {Register::kCode_t6};
const Register kJavaScriptCallArgCountRegister = {Register::kCode_a0};
const Register kRuntimeCallFunctionRegister = {Register::kCode_a1};
const Register kRuntimeCallArgCountRegister = {Register::kCode_a0};

View File

@ -22,6 +22,7 @@ const Register kInterpreterRegisterFileRegister = {Register::kCode_a7};
const Register kInterpreterBytecodeOffsetRegister = {Register::kCode_t0};
const Register kInterpreterBytecodeArrayRegister = {Register::kCode_t1};
const Register kInterpreterDispatchTableRegister = {Register::kCode_t2};
const Register kJavaScriptCallArgCountRegister = {Register::kCode_a0};
const Register kRuntimeCallFunctionRegister = {Register::kCode_a1};
const Register kRuntimeCallArgCountRegister = {Register::kCode_a0};

View File

@ -23,6 +23,7 @@ const Register kInterpreterRegisterFileRegister = {Register::kCode_r14};
const Register kInterpreterBytecodeOffsetRegister = {Register::kCode_r15};
const Register kInterpreterBytecodeArrayRegister = {Register::kCode_r16};
const Register kInterpreterDispatchTableRegister = {Register::kCode_r17};
const Register kJavaScriptCallArgCountRegister = {Register::kCode_r3};
const Register kRuntimeCallFunctionRegister = {Register::kCode_r4};
const Register kRuntimeCallArgCountRegister = {Register::kCode_r3};

View File

@ -25,6 +25,7 @@ const Register kInterpreterRegisterFileRegister = {Register::kCode_r11};
const Register kInterpreterBytecodeOffsetRegister = {Register::kCode_r12};
const Register kInterpreterBytecodeArrayRegister = {Register::kCode_r14};
const Register kInterpreterDispatchTableRegister = {Register::kCode_r15};
const Register kJavaScriptCallArgCountRegister = {Register::kCode_rax};
const Register kRuntimeCallFunctionRegister = {Register::kCode_rbx};
const Register kRuntimeCallArgCountRegister = {Register::kCode_rax};

View File

@ -22,6 +22,7 @@ const Register kInterpreterAccumulatorRegister = {Register::kCode_eax};
const Register kInterpreterRegisterFileRegister = {Register::kCode_edx};
const Register kInterpreterBytecodeOffsetRegister = {Register::kCode_ecx};
const Register kInterpreterBytecodeArrayRegister = {Register::kCode_edi};
const Register kJavaScriptCallArgCountRegister = {Register::kCode_eax};
const Register kRuntimeCallFunctionRegister = {Register::kCode_ebx};
const Register kRuntimeCallArgCountRegister = {Register::kCode_eax};

View File

@ -33,6 +33,12 @@
##############################################################################
# TODO(danno): These tests fail because the incoming descriptor for JavaScript
# calls has nothing to do with the interface descriptors of some code stubs,
# and they cannot be used interchangably.
'test-run-stubs/RunOptimizedMathFloorStub': [SKIP],
'test-run-stubs/RunStringAddTFStub': [SKIP],
# BUG(382): Weird test. Can't guarantee that it never times out.
'test-api/ApplyInterruption': [PASS, TIMEOUT],

View File

@ -382,7 +382,7 @@ TARGET_TEST_F(InstructionSelectorTest, CallJSFunctionWithDeopt) {
m.GetFrameStateFunctionInfo(1, 0)),
parameters, locals, stack, context_dummy, function_node,
m.UndefinedConstant());
Node* args[] = {receiver, context};
Node* args[] = {receiver, m.Int32Constant(1), context};
Node* call =
m.CallNWithFrameState(descriptor, function_node, args, state_node);
m.Return(call);