Skip to content

Commit 381b543

Browse files
rmcilroyCommit bot
rmcilroy
authored and
Commit bot
committed
Don't call FastNewFunctionContextStub if context is bigger than kMaxRegularHeapObjectSize.
CL https://codereview.chromium.org/2177273002 changed FastNewFunctionContextStub to take a number of slots parameter and in-doing so removed the maximum slot count for FastNewFunctionContextStub. This made it possible to create a closure which is larger than kMaxRegularHeapObjectSize and so can't be allocated by FastNewFunctionContextStub. Reintroduce FastNewFunctionContextStub::kMaxSlots (but make the limit much larger) to ensure we call the runtime for contexts which need to be allocated in the LO space. BUG=chromium:655573 Review-Url: https://codereview.chromium.org/2445703002 Cr-Commit-Position: refs/heads/master@{#40541}
1 parent 55277d9 commit 381b543

22 files changed

Lines changed: 230 additions & 98 deletions

‎src/code-stubs.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -863,6 +863,8 @@ class FastNewClosureStub : public TurboFanCodeStub {
863863

864864
class FastNewFunctionContextStub final : public TurboFanCodeStub {
865865
public:
866+
static const int kMaximumSlots = 0x8000;
867+
866868
explicit FastNewFunctionContextStub(Isolate* isolate)
867869
: TurboFanCodeStub(isolate) {}
868870

@@ -872,6 +874,11 @@ class FastNewFunctionContextStub final : public TurboFanCodeStub {
872874
compiler::Node* context);
873875

874876
private:
877+
// FastNewFunctionContextStub can only allocate closures which fit in the
878+
// new space.
879+
STATIC_ASSERT(((kMaximumSlots + Context::MIN_CONTEXT_SLOTS) * kPointerSize +
880+
FixedArray::kHeaderSize) < kMaxRegularHeapObjectSize);
881+
875882
DEFINE_CALL_INTERFACE_DESCRIPTOR(FastNewFunctionContext);
876883
DEFINE_TURBOFAN_CODE_STUB(FastNewFunctionContext, TurboFanCodeStub);
877884
};

‎src/compiler/js-generic-lowering.cc‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -449,9 +449,13 @@ void JSGenericLowering::LowerJSCreateFunctionContext(Node* node) {
449449
int const slot_count = OpParameter<int>(node->op());
450450
CallDescriptor::Flags flags = FrameStateFlagForCall(node);
451451

452-
Callable callable = CodeFactory::FastNewFunctionContext(isolate());
453-
node->InsertInput(zone(), 1, jsgraph()->Int32Constant(slot_count));
454-
ReplaceWithStubCall(node, callable, flags);
452+
if (slot_count <= FastNewFunctionContextStub::kMaximumSlots) {
453+
Callable callable = CodeFactory::FastNewFunctionContext(isolate());
454+
node->InsertInput(zone(), 1, jsgraph()->Int32Constant(slot_count));
455+
ReplaceWithStubCall(node, callable, flags);
456+
} else {
457+
ReplaceWithRuntimeCall(node, Runtime::kNewFunctionContext);
458+
}
455459
}
456460

457461

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

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -164,11 +164,17 @@ void LCodeGen::DoPrologue(LPrologue* instr) {
164164
__ CallRuntime(Runtime::kNewScriptContext);
165165
deopt_mode = Safepoint::kLazyDeopt;
166166
} else {
167-
FastNewFunctionContextStub stub(isolate());
168-
__ mov(FastNewFunctionContextDescriptor::SlotsRegister(), Operand(slots));
169-
__ CallStub(&stub);
170-
// Result of FastNewFunctionContextStub is always in new space.
171-
need_write_barrier = false;
167+
if (slots <= FastNewFunctionContextStub::kMaximumSlots) {
168+
FastNewFunctionContextStub stub(isolate());
169+
__ mov(FastNewFunctionContextDescriptor::SlotsRegister(),
170+
Operand(slots));
171+
__ CallStub(&stub);
172+
// Result of FastNewFunctionContextStub is always in new space.
173+
need_write_barrier = false;
174+
} else {
175+
__ push(r1);
176+
__ CallRuntime(Runtime::kNewFunctionContext);
177+
}
172178
}
173179
RecordSafepoint(deopt_mode);
174180

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

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -595,11 +595,16 @@ void LCodeGen::DoPrologue(LPrologue* instr) {
595595
__ CallRuntime(Runtime::kNewScriptContext);
596596
deopt_mode = Safepoint::kLazyDeopt;
597597
} else {
598-
FastNewFunctionContextStub stub(isolate());
599-
__ Mov(FastNewFunctionContextDescriptor::SlotsRegister(), slots);
600-
__ CallStub(&stub);
601-
// Result of FastNewFunctionContextStub is always in new space.
602-
need_write_barrier = false;
598+
if (slots <= FastNewFunctionContextStub::kMaximumSlots) {
599+
FastNewFunctionContextStub stub(isolate());
600+
__ Mov(FastNewFunctionContextDescriptor::SlotsRegister(), slots);
601+
__ CallStub(&stub);
602+
// Result of FastNewFunctionContextStub is always in new space.
603+
need_write_barrier = false;
604+
} else {
605+
__ Push(x1);
606+
__ CallRuntime(Runtime::kNewFunctionContext);
607+
}
603608
}
604609
RecordSafepoint(deopt_mode);
605610
// Context is returned in x0. It replaces the context passed to us. It's

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

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -176,12 +176,17 @@ void LCodeGen::DoPrologue(LPrologue* instr) {
176176
__ CallRuntime(Runtime::kNewScriptContext);
177177
deopt_mode = Safepoint::kLazyDeopt;
178178
} else {
179-
FastNewFunctionContextStub stub(isolate());
180-
__ mov(FastNewFunctionContextDescriptor::SlotsRegister(),
181-
Immediate(slots));
182-
__ CallStub(&stub);
183-
// Result of FastNewFunctionContextStub is always in new space.
184-
need_write_barrier = false;
179+
if (slots <= FastNewFunctionContextStub::kMaximumSlots) {
180+
FastNewFunctionContextStub stub(isolate());
181+
__ mov(FastNewFunctionContextDescriptor::SlotsRegister(),
182+
Immediate(slots));
183+
__ CallStub(&stub);
184+
// Result of FastNewFunctionContextStub is always in new space.
185+
need_write_barrier = false;
186+
} else {
187+
__ push(edi);
188+
__ CallRuntime(Runtime::kNewFunctionContext);
189+
}
185190
}
186191
RecordSafepoint(deopt_mode);
187192

‎src/crankshaft/mips/lithium-codegen-mips.cc‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -183,11 +183,17 @@ void LCodeGen::DoPrologue(LPrologue* instr) {
183183
__ CallRuntime(Runtime::kNewScriptContext);
184184
deopt_mode = Safepoint::kLazyDeopt;
185185
} else {
186-
FastNewFunctionContextStub stub(isolate());
187-
__ li(FastNewFunctionContextDescriptor::SlotsRegister(), Operand(slots));
188-
__ CallStub(&stub);
189-
// Result of FastNewFunctionContextStub is always in new space.
190-
need_write_barrier = false;
186+
if (slots <= FastNewFunctionContextStub::kMaximumSlots) {
187+
FastNewFunctionContextStub stub(isolate());
188+
__ li(FastNewFunctionContextDescriptor::SlotsRegister(),
189+
Operand(slots));
190+
__ CallStub(&stub);
191+
// Result of FastNewFunctionContextStub is always in new space.
192+
need_write_barrier = false;
193+
} else {
194+
__ push(a1);
195+
__ CallRuntime(Runtime::kNewFunctionContext);
196+
}
191197
}
192198
RecordSafepoint(deopt_mode);
193199

‎src/crankshaft/mips64/lithium-codegen-mips64.cc‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -159,11 +159,17 @@ void LCodeGen::DoPrologue(LPrologue* instr) {
159159
__ CallRuntime(Runtime::kNewScriptContext);
160160
deopt_mode = Safepoint::kLazyDeopt;
161161
} else {
162-
FastNewFunctionContextStub stub(isolate());
163-
__ li(FastNewFunctionContextDescriptor::SlotsRegister(), Operand(slots));
164-
__ CallStub(&stub);
165-
// Result of FastNewFunctionContextStub is always in new space.
166-
need_write_barrier = false;
162+
if (slots <= FastNewFunctionContextStub::kMaximumSlots) {
163+
FastNewFunctionContextStub stub(isolate());
164+
__ li(FastNewFunctionContextDescriptor::SlotsRegister(),
165+
Operand(slots));
166+
__ CallStub(&stub);
167+
// Result of FastNewFunctionContextStub is always in new space.
168+
need_write_barrier = false;
169+
} else {
170+
__ push(a1);
171+
__ CallRuntime(Runtime::kNewFunctionContext);
172+
}
167173
}
168174
RecordSafepoint(deopt_mode);
169175

‎src/crankshaft/ppc/lithium-codegen-ppc.cc‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -170,11 +170,17 @@ void LCodeGen::DoPrologue(LPrologue* instr) {
170170
__ CallRuntime(Runtime::kNewScriptContext);
171171
deopt_mode = Safepoint::kLazyDeopt;
172172
} else {
173-
FastNewFunctionContextStub stub(isolate());
174-
__ mov(FastNewFunctionContextDescriptor::SlotsRegister(), Operand(slots));
175-
__ CallStub(&stub);
176-
// Result of FastNewFunctionContextStub is always in new space.
177-
need_write_barrier = false;
173+
if (slots <= FastNewFunctionContextStub::kMaximumSlots) {
174+
FastNewFunctionContextStub stub(isolate());
175+
__ mov(FastNewFunctionContextDescriptor::SlotsRegister(),
176+
Operand(slots));
177+
__ CallStub(&stub);
178+
// Result of FastNewFunctionContextStub is always in new space.
179+
need_write_barrier = false;
180+
} else {
181+
__ push(r4);
182+
__ CallRuntime(Runtime::kNewFunctionContext);
183+
}
178184
}
179185
RecordSafepoint(deopt_mode);
180186

‎src/crankshaft/s390/lithium-codegen-s390.cc‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -160,11 +160,17 @@ void LCodeGen::DoPrologue(LPrologue* instr) {
160160
__ CallRuntime(Runtime::kNewScriptContext);
161161
deopt_mode = Safepoint::kLazyDeopt;
162162
} else {
163-
FastNewFunctionContextStub stub(isolate());
164-
__ mov(FastNewFunctionContextDescriptor::SlotsRegister(), Operand(slots));
165-
__ CallStub(&stub);
166-
// Result of FastNewFunctionContextStub is always in new space.
167-
need_write_barrier = false;
163+
if (slots <= FastNewFunctionContextStub::kMaximumSlots) {
164+
FastNewFunctionContextStub stub(isolate());
165+
__ mov(FastNewFunctionContextDescriptor::SlotsRegister(),
166+
Operand(slots));
167+
__ CallStub(&stub);
168+
// Result of FastNewFunctionContextStub is always in new space.
169+
need_write_barrier = false;
170+
} else {
171+
__ push(r3);
172+
__ CallRuntime(Runtime::kNewFunctionContext);
173+
}
168174
}
169175
RecordSafepoint(deopt_mode);
170176

‎src/crankshaft/x64/lithium-codegen-x64.cc‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -179,11 +179,16 @@ void LCodeGen::DoPrologue(LPrologue* instr) {
179179
__ CallRuntime(Runtime::kNewScriptContext);
180180
deopt_mode = Safepoint::kLazyDeopt;
181181
} else {
182-
FastNewFunctionContextStub stub(isolate());
183-
__ Set(FastNewFunctionContextDescriptor::SlotsRegister(), slots);
184-
__ CallStub(&stub);
185-
// Result of FastNewFunctionContextStub is always in new space.
186-
need_write_barrier = false;
182+
if (slots <= FastNewFunctionContextStub::kMaximumSlots) {
183+
FastNewFunctionContextStub stub(isolate());
184+
__ Set(FastNewFunctionContextDescriptor::SlotsRegister(), slots);
185+
__ CallStub(&stub);
186+
// Result of FastNewFunctionContextStub is always in new space.
187+
need_write_barrier = false;
188+
} else {
189+
__ Push(rdi);
190+
__ CallRuntime(Runtime::kNewFunctionContext);
191+
}
187192
}
188193
RecordSafepoint(deopt_mode);
189194

0 commit comments

Comments
 (0)