Skip to content

Commit b37907f

Browse files
bmeurerCommit bot
bmeurer
authored and
Commit bot
committed
[calls] Consistent call protocol for calls.
The number of actual arguments should always be available, there's no point in trying to optimize away a simple assignment of an immediate to a register before some calls. The main motivation is to have a consistent state at the beginning of every function. Currently the arguments register (i.e. rax or eax) either contains the number of arguments or some random garbage depending on whether the callsite decided that the callee might need the information or not. This causes trouble with runtime implementations of functions that do not set internal_formal_parameter_count to the DontAdaptArguments sentinel (we don't have any of those yet), but also makes it impossible to sanity check the arguments in the callee, because the callee doesn't know whether the caller decided to pass the number of arguments or random garbage. BUG=v8:4413 LOG=n Review URL: https://codereview.chromium.org/1330033002 Cr-Commit-Position: refs/heads/master@{#30648}
1 parent ce95a4d commit b37907f

16 files changed

Lines changed: 43 additions & 81 deletions

‎src/arm/lithium-codegen-arm.cc‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3464,11 +3464,8 @@ void LCodeGen::CallKnownFunction(Handle<JSFunction> function,
34643464
// Change context.
34653465
__ ldr(cp, FieldMemOperand(function_reg, JSFunction::kContextOffset));
34663466

3467-
// Set r0 to arguments count if adaption is not needed. Assumes that r0
3468-
// is available to write to at this point.
3469-
if (dont_adapt_arguments) {
3470-
__ mov(r0, Operand(arity));
3471-
}
3467+
// Always initialize r0 to the number of actual arguments.
3468+
__ mov(r0, Operand(arity));
34723469

34733470
// Invoke function.
34743471
__ ldr(ip, FieldMemOperand(function_reg, JSFunction::kCodeEntryOffset));
@@ -3849,9 +3846,7 @@ void LCodeGen::DoCallJSFunction(LCallJSFunction* instr) {
38493846
DCHECK(ToRegister(instr->function()).is(r1));
38503847
DCHECK(ToRegister(instr->result()).is(r0));
38513848

3852-
if (instr->hydrogen()->pass_argument_count()) {
3853-
__ mov(r0, Operand(instr->arity()));
3854-
}
3849+
__ mov(r0, Operand(instr->arity()));
38553850

38563851
// Change context.
38573852
__ ldr(cp, FieldMemOperand(r1, JSFunction::kContextOffset));

‎src/arm/macro-assembler-arm.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1251,10 +1251,10 @@ void MacroAssembler::InvokePrologue(const ParameterCount& expected,
12511251

12521252
if (expected.is_immediate()) {
12531253
DCHECK(actual.is_immediate());
1254+
mov(r0, Operand(actual.immediate()));
12541255
if (expected.immediate() == actual.immediate()) {
12551256
definitely_matches = true;
12561257
} else {
1257-
mov(r0, Operand(actual.immediate()));
12581258
const int sentinel = SharedFunctionInfo::kDontAdaptArgumentsSentinel;
12591259
if (expected.immediate() == sentinel) {
12601260
// Don't worry about adapting arguments for builtins that
@@ -1269,9 +1269,9 @@ void MacroAssembler::InvokePrologue(const ParameterCount& expected,
12691269
}
12701270
} else {
12711271
if (actual.is_immediate()) {
1272+
mov(r0, Operand(actual.immediate()));
12721273
cmp(expected.reg(), Operand(actual.immediate()));
12731274
b(eq, &regular_invoke);
1274-
mov(r0, Operand(actual.immediate()));
12751275
} else {
12761276
cmp(expected.reg(), Operand(actual.reg()));
12771277
b(eq, &regular_invoke);

‎src/arm64/lithium-codegen-arm64.cc‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1996,11 +1996,8 @@ void LCodeGen::CallKnownFunction(Handle<JSFunction> function,
19961996
// Change context.
19971997
__ Ldr(cp, FieldMemOperand(function_reg, JSFunction::kContextOffset));
19981998

1999-
// Set the arguments count if adaption is not needed. Assumes that x0 is
2000-
// available to write to at this point.
2001-
if (dont_adapt_arguments) {
2002-
__ Mov(arity_reg, arity);
2003-
}
1999+
// Always initialize x0 to the number of actual arguments.
2000+
__ Mov(arity_reg, arity);
20042001

20052002
// Invoke function.
20062003
__ Ldr(x10, FieldMemOperand(function_reg, JSFunction::kCodeEntryOffset));
@@ -2067,9 +2064,7 @@ void LCodeGen::DoCallJSFunction(LCallJSFunction* instr) {
20672064
DCHECK(instr->IsMarkedAsCall());
20682065
DCHECK(ToRegister(instr->function()).is(x1));
20692066

2070-
if (instr->hydrogen()->pass_argument_count()) {
2071-
__ Mov(x0, Operand(instr->arity()));
2072-
}
2067+
__ Mov(x0, Operand(instr->arity()));
20732068

20742069
// Change context.
20752070
__ Ldr(cp, FieldMemOperand(x1, JSFunction::kContextOffset));

‎src/arm64/macro-assembler-arm64.cc‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2571,11 +2571,11 @@ void MacroAssembler::InvokePrologue(const ParameterCount& expected,
25712571

25722572
if (expected.is_immediate()) {
25732573
DCHECK(actual.is_immediate());
2574+
Mov(x0, actual.immediate());
25742575
if (expected.immediate() == actual.immediate()) {
25752576
definitely_matches = true;
25762577

25772578
} else {
2578-
Mov(x0, actual.immediate());
25792579
if (expected.immediate() ==
25802580
SharedFunctionInfo::kDontAdaptArgumentsSentinel) {
25812581
// Don't worry about adapting arguments for builtins that
@@ -2593,11 +2593,10 @@ void MacroAssembler::InvokePrologue(const ParameterCount& expected,
25932593
} else { // expected is a register.
25942594
Operand actual_op = actual.is_immediate() ? Operand(actual.immediate())
25952595
: Operand(actual.reg());
2596+
Mov(x0, actual_op);
25962597
// If actual == expected perform a regular invocation.
25972598
Cmp(expected.reg(), actual_op);
25982599
B(eq, &regular_invoke);
2599-
// Otherwise set up x0 for the argument adaptor.
2600-
Mov(x0, actual_op);
26012600
}
26022601

26032602
// If the argument counts may mismatch, generate a call to the argument

‎src/hydrogen-instructions.cc‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -927,8 +927,7 @@ std::ostream& HCallJSFunction::PrintDataTo(std::ostream& os) const { // NOLINT
927927

928928
HCallJSFunction* HCallJSFunction::New(Isolate* isolate, Zone* zone,
929929
HValue* context, HValue* function,
930-
int argument_count,
931-
bool pass_argument_count) {
930+
int argument_count) {
932931
bool has_stack_check = false;
933932
if (function->IsConstant()) {
934933
HConstant* fun_const = HConstant::cast(function);
@@ -939,9 +938,7 @@ HCallJSFunction* HCallJSFunction::New(Isolate* isolate, Zone* zone,
939938
jsfun->code()->kind() == Code::OPTIMIZED_FUNCTION);
940939
}
941940

942-
return new(zone) HCallJSFunction(
943-
function, argument_count, pass_argument_count,
944-
has_stack_check);
941+
return new (zone) HCallJSFunction(function, argument_count, has_stack_check);
945942
}
946943

947944

‎src/hydrogen-instructions.h‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2228,8 +2228,7 @@ class HBinaryCall : public HCall<2> {
22282228
class HCallJSFunction final : public HCall<1> {
22292229
public:
22302230
static HCallJSFunction* New(Isolate* isolate, Zone* zone, HValue* context,
2231-
HValue* function, int argument_count,
2232-
bool pass_argument_count);
2231+
HValue* function, int argument_count);
22332232

22342233
HValue* function() const { return OperandAt(0); }
22352234

@@ -2240,8 +2239,6 @@ class HCallJSFunction final : public HCall<1> {
22402239
return Representation::Tagged();
22412240
}
22422241

2243-
bool pass_argument_count() const { return pass_argument_count_; }
2244-
22452242
bool HasStackCheck() final { return has_stack_check_; }
22462243

22472244
DECLARE_CONCRETE_INSTRUCTION(CallJSFunction)
@@ -2250,15 +2247,12 @@ class HCallJSFunction final : public HCall<1> {
22502247
// The argument count includes the receiver.
22512248
HCallJSFunction(HValue* function,
22522249
int argument_count,
2253-
bool pass_argument_count,
22542250
bool has_stack_check)
22552251
: HCall<1>(argument_count),
2256-
pass_argument_count_(pass_argument_count),
22572252
has_stack_check_(has_stack_check) {
22582253
SetOperandAt(0, function);
22592254
}
22602255

2261-
bool pass_argument_count_;
22622256
bool has_stack_check_;
22632257
};
22642258

‎src/hydrogen.cc‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7901,9 +7901,9 @@ void HOptimizedGraphBuilder::AddCheckPrototypeMaps(Handle<JSObject> holder,
79017901
}
79027902

79037903

7904-
HInstruction* HOptimizedGraphBuilder::NewPlainFunctionCall(
7905-
HValue* fun, int argument_count, bool pass_argument_count) {
7906-
return New<HCallJSFunction>(fun, argument_count, pass_argument_count);
7904+
HInstruction* HOptimizedGraphBuilder::NewPlainFunctionCall(HValue* fun,
7905+
int argument_count) {
7906+
return New<HCallJSFunction>(fun, argument_count);
79077907
}
79087908

79097909

@@ -7941,7 +7941,7 @@ HInstruction* HOptimizedGraphBuilder::BuildCallConstantFunction(
79417941
if (jsfun.is_identical_to(current_info()->closure())) {
79427942
graph()->MarkRecursive();
79437943
}
7944-
return NewPlainFunctionCall(target, argument_count, dont_adapt_arguments);
7944+
return NewPlainFunctionCall(target, argument_count);
79457945
} else {
79467946
HValue* param_count_value = Add<HConstant>(formal_parameter_count);
79477947
HValue* context = Add<HLoadNamedField>(
@@ -8962,7 +8962,7 @@ bool HOptimizedGraphBuilder::TryInlineBuiltinMethodCall(
89628962
if_inline.Else();
89638963
{
89648964
Add<HPushArguments>(receiver);
8965-
result = Add<HCallJSFunction>(function, 1, true);
8965+
result = Add<HCallJSFunction>(function, 1);
89668966
if (!ast_context()->IsEffect()) Push(result);
89678967
}
89688968
if_inline.End();

‎src/hydrogen.h‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2865,9 +2865,7 @@ class HOptimizedGraphBuilder : public HGraphBuilder, public AstVisitor {
28652865
void AddCheckPrototypeMaps(Handle<JSObject> holder,
28662866
Handle<Map> receiver_map);
28672867

2868-
HInstruction* NewPlainFunctionCall(HValue* fun,
2869-
int argument_count,
2870-
bool pass_argument_count);
2868+
HInstruction* NewPlainFunctionCall(HValue* fun, int argument_count);
28712869

28722870
HInstruction* NewArgumentAdaptorCall(HValue* fun, HValue* context,
28732871
int argument_count,

‎src/ia32/lithium-codegen-ia32.cc‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3340,11 +3340,8 @@ void LCodeGen::CallKnownFunction(Handle<JSFunction> function,
33403340
// Change context.
33413341
__ mov(esi, FieldOperand(function_reg, JSFunction::kContextOffset));
33423342

3343-
// Set eax to arguments count if adaption is not needed. Assumes that eax
3344-
// is available to write to at this point.
3345-
if (dont_adapt_arguments) {
3346-
__ mov(eax, arity);
3347-
}
3343+
// Always initialize eax to the number of actual arguments.
3344+
__ mov(eax, arity);
33483345

33493346
// Invoke function directly.
33503347
if (function.is_identical_to(info()->closure())) {
@@ -3406,9 +3403,7 @@ void LCodeGen::DoCallJSFunction(LCallJSFunction* instr) {
34063403
DCHECK(ToRegister(instr->function()).is(edi));
34073404
DCHECK(ToRegister(instr->result()).is(eax));
34083405

3409-
if (instr->hydrogen()->pass_argument_count()) {
3410-
__ mov(eax, instr->arity());
3411-
}
3406+
__ mov(eax, instr->arity());
34123407

34133408
// Change context.
34143409
__ mov(esi, FieldOperand(edi, JSFunction::kContextOffset));

‎src/ia32/macro-assembler-ia32.cc‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1923,10 +1923,10 @@ void MacroAssembler::InvokePrologue(const ParameterCount& expected,
19231923
Label invoke;
19241924
if (expected.is_immediate()) {
19251925
DCHECK(actual.is_immediate());
1926+
mov(eax, actual.immediate());
19261927
if (expected.immediate() == actual.immediate()) {
19271928
definitely_matches = true;
19281929
} else {
1929-
mov(eax, actual.immediate());
19301930
const int sentinel = SharedFunctionInfo::kDontAdaptArgumentsSentinel;
19311931
if (expected.immediate() == sentinel) {
19321932
// Don't worry about adapting arguments for builtins that
@@ -1944,17 +1944,19 @@ void MacroAssembler::InvokePrologue(const ParameterCount& expected,
19441944
// Expected is in register, actual is immediate. This is the
19451945
// case when we invoke function values without going through the
19461946
// IC mechanism.
1947+
mov(eax, actual.immediate());
19471948
cmp(expected.reg(), actual.immediate());
19481949
j(equal, &invoke);
19491950
DCHECK(expected.reg().is(ebx));
1950-
mov(eax, actual.immediate());
19511951
} else if (!expected.reg().is(actual.reg())) {
19521952
// Both expected and actual are in (different) registers. This
19531953
// is the case when we invoke functions using call and apply.
19541954
cmp(expected.reg(), actual.reg());
19551955
j(equal, &invoke);
19561956
DCHECK(actual.reg().is(eax));
19571957
DCHECK(expected.reg().is(ebx));
1958+
} else {
1959+
Move(eax, actual.reg());
19581960
}
19591961
}
19601962

0 commit comments

Comments
 (0)