Skip to content

Commit 72b652e

Browse files
joshualittCommit Bot
joshualitt
authored and
Commit Bot
committed
Revert "[ic] Migrate Code-based handlers to use data driven handler."
This reverts commit d46bd85. Reason for revert: I suspect this breaks the 'V8 Linux - predictable' bot. Specifically, 'typedarray-copywithin' has been failing since this landed. I am not exactly sure what is wrong from the tests error message, but see this link for more information: https://logs.chromium.org/logs/v8/buildbucket/cr-buildbucket.appspot.com/8896980452133814304/+/steps/Check_-_d8__flakes_/0/logs/typedarray-copywithin/0 Original change's description: > [ic] Migrate Code-based handlers to use data driven handler. > > All usage of KeyedLoadIC_Slow, HasIC_Slow, StoreInArrayLiteralIC_Slow > and KeyedStoreIC_Slow now use data driven handlers > > Bug: v8:9779 > Change-Id: Idd888c5c10b462a5fe155ba0add36f95169bd76d > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1895988 > Reviewed-by: Toon Verwaest <verwaest@chromium.org> > Reviewed-by: Ross McIlroy <rmcilroy@chromium.org> > Commit-Queue: Suraj Sharma <surshar@microsoft.com> > Cr-Commit-Position: refs/heads/master@{#64918} TBR=rmcilroy@chromium.org,verwaest@chromium.org,surshar@microsoft.com Change-Id: Id7c2b553f85b46048bed2c633b8bd24098f67147 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: v8:9779 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1912092 Reviewed-by: Joshua Litt <joshualitt@chromium.org> Commit-Queue: Joshua Litt <joshualitt@chromium.org> Cr-Commit-Position: refs/heads/master@{#64922}
1 parent c60c2e3 commit 72b652e

11 files changed

Lines changed: 185 additions & 58 deletions

‎src/builtins/builtins-definitions.h‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,31 +215,43 @@ namespace internal {
215215
\
216216
/* Handlers */ \
217217
TFH(KeyedLoadIC_PolymorphicName, LoadWithVector) \
218+
TFH(KeyedLoadIC_Slow, LoadWithVector) \
218219
TFH(KeyedStoreIC_Megamorphic, Store) \
220+
TFH(KeyedStoreIC_Slow, StoreWithVector) \
219221
TFH(LoadGlobalIC_NoFeedback, LoadGlobalNoFeedback) \
220222
TFH(LoadIC_FunctionPrototype, LoadWithVector) \
221223
TFH(LoadIC_StringLength, LoadWithVector) \
222224
TFH(LoadIC_StringWrapperLength, LoadWithVector) \
223225
TFH(LoadIC_NoFeedback, LoadNoFeedback) \
224226
TFH(StoreGlobalIC_Slow, StoreWithVector) \
225227
TFH(StoreIC_NoFeedback, Store) \
228+
TFH(StoreInArrayLiteralIC_Slow, StoreWithVector) \
226229
TFH(KeyedLoadIC_SloppyArguments, LoadWithVector) \
227230
TFH(LoadIndexedInterceptorIC, LoadWithVector) \
228231
TFH(KeyedStoreIC_SloppyArguments_Standard, StoreWithVector) \
229232
TFH(KeyedStoreIC_SloppyArguments_GrowNoTransitionHandleCOW, StoreWithVector) \
230233
TFH(KeyedStoreIC_SloppyArguments_NoTransitionIgnoreOOB, StoreWithVector) \
231234
TFH(KeyedStoreIC_SloppyArguments_NoTransitionHandleCOW, StoreWithVector) \
235+
TFH(StoreInArrayLiteralIC_Slow_Standard, StoreWithVector) \
232236
TFH(StoreFastElementIC_Standard, StoreWithVector) \
233237
TFH(StoreFastElementIC_GrowNoTransitionHandleCOW, StoreWithVector) \
234238
TFH(StoreFastElementIC_NoTransitionIgnoreOOB, StoreWithVector) \
235239
TFH(StoreFastElementIC_NoTransitionHandleCOW, StoreWithVector) \
240+
TFH(StoreInArrayLiteralIC_Slow_GrowNoTransitionHandleCOW, StoreWithVector) \
241+
TFH(StoreInArrayLiteralIC_Slow_NoTransitionIgnoreOOB, StoreWithVector) \
242+
TFH(StoreInArrayLiteralIC_Slow_NoTransitionHandleCOW, StoreWithVector) \
243+
TFH(KeyedStoreIC_Slow_Standard, StoreWithVector) \
244+
TFH(KeyedStoreIC_Slow_GrowNoTransitionHandleCOW, StoreWithVector) \
245+
TFH(KeyedStoreIC_Slow_NoTransitionIgnoreOOB, StoreWithVector) \
246+
TFH(KeyedStoreIC_Slow_NoTransitionHandleCOW, StoreWithVector) \
236247
TFH(ElementsTransitionAndStore_Standard, StoreTransition) \
237248
TFH(ElementsTransitionAndStore_GrowNoTransitionHandleCOW, StoreTransition) \
238249
TFH(ElementsTransitionAndStore_NoTransitionIgnoreOOB, StoreTransition) \
239250
TFH(ElementsTransitionAndStore_NoTransitionHandleCOW, StoreTransition) \
240251
TFH(KeyedHasIC_PolymorphicName, LoadWithVector) \
241252
TFH(KeyedHasIC_SloppyArguments, LoadWithVector) \
242253
TFH(HasIndexedInterceptorIC, LoadWithVector) \
254+
TFH(HasIC_Slow, LoadWithVector) \
243255
\
244256
/* Microtask helpers */ \
245257
TFS(EnqueueMicrotask, kMicrotask) \

‎src/builtins/builtins-handler-gen.cc‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ class HandlerBuiltinsAssembler : public CodeStubAssembler {
2020

2121
protected:
2222
void Generate_KeyedStoreIC_SloppyArguments();
23+
void Generate_KeyedStoreIC_Slow();
24+
void Generate_StoreInArrayLiteralIC_Slow();
2325

2426
// Essentially turns runtime elements kinds (TNode<Int32T>) into
2527
// compile-time types (int) by dispatching over the runtime type and
@@ -200,6 +202,14 @@ TF_BUILTIN(LoadIC_StringWrapperLength, CodeStubAssembler) {
200202
Return(LoadStringLengthAsSmi(string));
201203
}
202204

205+
TF_BUILTIN(KeyedLoadIC_Slow, CodeStubAssembler) {
206+
TNode<Object> receiver = CAST(Parameter(Descriptor::kReceiver));
207+
TNode<Object> name = CAST(Parameter(Descriptor::kName));
208+
TNode<Context> context = CAST(Parameter(Descriptor::kContext));
209+
210+
TailCallRuntime(Runtime::kGetProperty, context, receiver, name);
211+
}
212+
203213
void Builtins::Generate_KeyedStoreIC_Megamorphic(
204214
compiler::CodeAssemblerState* state) {
205215
KeyedStoreGenericGenerator::Generate(state);
@@ -210,6 +220,74 @@ void Builtins::Generate_StoreIC_NoFeedback(
210220
StoreICNoFeedbackGenerator::Generate(state);
211221
}
212222

223+
// TODO(mythria): Create a Descriptor without feedback vector and slot
224+
// parameters.
225+
void HandlerBuiltinsAssembler::Generate_KeyedStoreIC_Slow() {
226+
using Descriptor = StoreWithVectorDescriptor;
227+
TNode<Object> receiver = CAST(Parameter(Descriptor::kReceiver));
228+
TNode<Object> name = CAST(Parameter(Descriptor::kName));
229+
TNode<Object> value = CAST(Parameter(Descriptor::kValue));
230+
TNode<Context> context = CAST(Parameter(Descriptor::kContext));
231+
232+
// The slow case calls into the runtime to complete the store without causing
233+
// an IC miss that would otherwise cause a transition to the generic stub.
234+
TailCallRuntime(Runtime::kKeyedStoreIC_Slow, context, value, receiver, name);
235+
}
236+
237+
TF_BUILTIN(KeyedStoreIC_Slow, HandlerBuiltinsAssembler) {
238+
Generate_KeyedStoreIC_Slow();
239+
}
240+
241+
TF_BUILTIN(KeyedStoreIC_Slow_Standard, HandlerBuiltinsAssembler) {
242+
Generate_KeyedStoreIC_Slow();
243+
}
244+
245+
TF_BUILTIN(KeyedStoreIC_Slow_GrowNoTransitionHandleCOW,
246+
HandlerBuiltinsAssembler) {
247+
Generate_KeyedStoreIC_Slow();
248+
}
249+
250+
TF_BUILTIN(KeyedStoreIC_Slow_NoTransitionIgnoreOOB, HandlerBuiltinsAssembler) {
251+
Generate_KeyedStoreIC_Slow();
252+
}
253+
254+
TF_BUILTIN(KeyedStoreIC_Slow_NoTransitionHandleCOW, HandlerBuiltinsAssembler) {
255+
Generate_KeyedStoreIC_Slow();
256+
}
257+
258+
void HandlerBuiltinsAssembler::Generate_StoreInArrayLiteralIC_Slow() {
259+
using Descriptor = StoreWithVectorDescriptor;
260+
TNode<JSArray> array = CAST(Parameter(Descriptor::kReceiver));
261+
TNode<Object> index = CAST(Parameter(Descriptor::kName));
262+
TNode<Object> value = CAST(Parameter(Descriptor::kValue));
263+
TNode<Context> context = CAST(Parameter(Descriptor::kContext));
264+
TailCallRuntime(Runtime::kStoreInArrayLiteralIC_Slow, context, value, array,
265+
index);
266+
}
267+
268+
TF_BUILTIN(StoreInArrayLiteralIC_Slow, HandlerBuiltinsAssembler) {
269+
Generate_StoreInArrayLiteralIC_Slow();
270+
}
271+
272+
TF_BUILTIN(StoreInArrayLiteralIC_Slow_Standard, HandlerBuiltinsAssembler) {
273+
Generate_StoreInArrayLiteralIC_Slow();
274+
}
275+
276+
TF_BUILTIN(StoreInArrayLiteralIC_Slow_GrowNoTransitionHandleCOW,
277+
HandlerBuiltinsAssembler) {
278+
Generate_StoreInArrayLiteralIC_Slow();
279+
}
280+
281+
TF_BUILTIN(StoreInArrayLiteralIC_Slow_NoTransitionIgnoreOOB,
282+
HandlerBuiltinsAssembler) {
283+
Generate_StoreInArrayLiteralIC_Slow();
284+
}
285+
286+
TF_BUILTIN(StoreInArrayLiteralIC_Slow_NoTransitionHandleCOW,
287+
HandlerBuiltinsAssembler) {
288+
Generate_StoreInArrayLiteralIC_Slow();
289+
}
290+
213291
// All possible fast-to-fast transitions. Transitions to dictionary mode are not
214292
// handled by ElementsTransitionAndStore.
215293
#define ELEMENTS_KIND_TRANSITIONS(V) \
@@ -598,5 +676,13 @@ TF_BUILTIN(HasIndexedInterceptorIC, CodeStubAssembler) {
598676
vector);
599677
}
600678

679+
TF_BUILTIN(HasIC_Slow, CodeStubAssembler) {
680+
TNode<Object> receiver = CAST(Parameter(Descriptor::kReceiver));
681+
TNode<Object> name = CAST(Parameter(Descriptor::kName));
682+
TNode<Context> context = CAST(Parameter(Descriptor::kContext));
683+
684+
TailCallRuntime(Runtime::kHasProperty, context, receiver, name);
685+
}
686+
601687
} // namespace internal
602688
} // namespace v8

‎src/codegen/code-factory.cc‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,53 @@ Callable CodeFactory::KeyedStoreIC_SloppyArguments(Isolate* isolate,
122122
return isolate->builtins()->CallableFor(isolate, builtin_index);
123123
}
124124

125+
Callable CodeFactory::KeyedStoreIC_Slow(Isolate* isolate,
126+
KeyedAccessStoreMode mode) {
127+
Builtins::Name builtin_index;
128+
switch (mode) {
129+
case STANDARD_STORE:
130+
builtin_index = Builtins::kKeyedStoreIC_Slow_Standard;
131+
break;
132+
case STORE_AND_GROW_HANDLE_COW:
133+
builtin_index = Builtins::kKeyedStoreIC_Slow_GrowNoTransitionHandleCOW;
134+
break;
135+
case STORE_IGNORE_OUT_OF_BOUNDS:
136+
builtin_index = Builtins::kKeyedStoreIC_Slow_NoTransitionIgnoreOOB;
137+
break;
138+
case STORE_HANDLE_COW:
139+
builtin_index = Builtins::kKeyedStoreIC_Slow_NoTransitionHandleCOW;
140+
break;
141+
default:
142+
UNREACHABLE();
143+
}
144+
return isolate->builtins()->CallableFor(isolate, builtin_index);
145+
}
146+
147+
Callable CodeFactory::StoreInArrayLiteralIC_Slow(Isolate* isolate,
148+
KeyedAccessStoreMode mode) {
149+
Builtins::Name builtin_index;
150+
switch (mode) {
151+
case STANDARD_STORE:
152+
builtin_index = Builtins::kStoreInArrayLiteralIC_Slow_Standard;
153+
break;
154+
case STORE_AND_GROW_HANDLE_COW:
155+
builtin_index =
156+
Builtins::kStoreInArrayLiteralIC_Slow_GrowNoTransitionHandleCOW;
157+
break;
158+
case STORE_IGNORE_OUT_OF_BOUNDS:
159+
builtin_index =
160+
Builtins::kStoreInArrayLiteralIC_Slow_NoTransitionIgnoreOOB;
161+
break;
162+
case STORE_HANDLE_COW:
163+
builtin_index =
164+
Builtins::kStoreInArrayLiteralIC_Slow_NoTransitionHandleCOW;
165+
break;
166+
default:
167+
UNREACHABLE();
168+
}
169+
return isolate->builtins()->CallableFor(isolate, builtin_index);
170+
}
171+
125172
Callable CodeFactory::ElementsTransitionAndStore(Isolate* isolate,
126173
KeyedAccessStoreMode mode) {
127174
Builtins::Name builtin_index;

‎src/codegen/code-factory.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@ class V8_EXPORT_PRIVATE CodeFactory final {
4242

4343
static Callable KeyedStoreIC_SloppyArguments(Isolate* isolate,
4444
KeyedAccessStoreMode mode);
45+
static Callable KeyedStoreIC_Slow(Isolate* isolate,
46+
KeyedAccessStoreMode mode);
47+
static Callable StoreInArrayLiteralIC_Slow(Isolate* isolate,
48+
KeyedAccessStoreMode mode);
4549
static Callable ElementsTransitionAndStore(Isolate* isolate,
4650
KeyedAccessStoreMode mode);
4751
static Callable StoreFastElementIC(Isolate* isolate,

‎src/ic/accessor-assembler.cc‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1564,9 +1564,6 @@ void AccessorAssembler::HandleStoreICProtoHandler(
15641564
GotoIf(Word32Equal(handler_kind, Int32Constant(StoreHandler::kNormal)),
15651565
&if_add_normal);
15661566

1567-
GotoIf(Word32Equal(handler_kind, Int32Constant(StoreHandler::kSlow)),
1568-
&if_slow);
1569-
15701567
TNode<MaybeObject> maybe_holder = LoadHandlerDataField(handler, 1);
15711568
CSA_ASSERT(this, IsWeakOrCleared(maybe_holder));
15721569
TNode<HeapObject> holder = GetHeapObjectAssumeWeak(maybe_holder, miss);
@@ -1584,6 +1581,9 @@ void AccessorAssembler::HandleStoreICProtoHandler(
15841581
GotoIf(Word32Equal(handler_kind, Int32Constant(StoreHandler::kApiSetter)),
15851582
&if_api_setter);
15861583

1584+
GotoIf(Word32Equal(handler_kind, Int32Constant(StoreHandler::kSlow)),
1585+
&if_slow);
1586+
15871587
GotoIf(Word32Equal(handler_kind, Int32Constant(StoreHandler::kInterceptor)),
15881588
&if_interceptor);
15891589

‎src/ic/handler-configuration-inl.h‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -137,10 +137,8 @@ Handle<Smi> StoreHandler::StoreInterceptor(Isolate* isolate) {
137137
return handle(Smi::FromInt(config), isolate);
138138
}
139139

140-
Handle<Smi> StoreHandler::StoreSlow(Isolate* isolate,
141-
KeyedAccessStoreMode store_mode) {
142-
int config =
143-
KindBits::encode(kSlow) | KeyedAccessStoreModeBits::encode(store_mode);
140+
Handle<Smi> StoreHandler::StoreSlow(Isolate* isolate) {
141+
int config = KindBits::encode(kSlow);
144142
return handle(Smi::FromInt(config), isolate);
145143
}
146144

‎src/ic/handler-configuration.cc‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -176,19 +176,6 @@ KeyedAccessLoadMode LoadHandler::GetKeyedAccessLoadMode(MaybeObject handler) {
176176
return STANDARD_LOAD;
177177
}
178178

179-
// static
180-
KeyedAccessStoreMode StoreHandler::GetKeyedAccessStoreMode(
181-
MaybeObject handler) {
182-
DisallowHeapAllocation no_gc;
183-
if (handler->IsSmi()) {
184-
int const raw_handler = handler.ToSmi().value();
185-
KeyedAccessStoreMode store_mode =
186-
KeyedAccessStoreModeBits::decode(raw_handler);
187-
return store_mode;
188-
}
189-
return STANDARD_STORE;
190-
}
191-
192179
// static
193180
Handle<Object> StoreHandler::StoreElementTransition(
194181
Isolate* isolate, Handle<Map> receiver_map, Handle<Map> transition,

‎src/ic/handler-configuration.h‎

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -224,13 +224,6 @@ class StoreHandler final : public DataHandler {
224224
// Index of a value entry in the descriptor array.
225225
using DescriptorBits =
226226
LookupOnReceiverBits::Next<unsigned, kDescriptorIndexBitCount>;
227-
228-
//
229-
// Encodes the bits when StoreSlow contains KeyedAccessStoreMode.
230-
//
231-
using KeyedAccessStoreModeBits =
232-
DescriptorBits::Next<KeyedAccessStoreMode, 2>;
233-
234227
//
235228
// Encoding when KindBits contains kTransitionToConstant.
236229
//
@@ -298,15 +291,11 @@ class StoreHandler final : public DataHandler {
298291
static inline Handle<Smi> StoreInterceptor(Isolate* isolate);
299292

300293
// Creates a Smi-handler for storing a property.
301-
static inline Handle<Smi> StoreSlow(
302-
Isolate* isolate, KeyedAccessStoreMode store_mode = STANDARD_STORE);
294+
static inline Handle<Smi> StoreSlow(Isolate* isolate);
303295

304296
// Creates a Smi-handler for storing a property on a proxy.
305297
static inline Handle<Smi> StoreProxy(Isolate* isolate);
306298

307-
// Decodes the KeyedAccessStoreMode from a {handler}.
308-
static KeyedAccessStoreMode GetKeyedAccessStoreMode(MaybeObject handler);
309-
310299
private:
311300
static inline Handle<Smi> StoreField(Isolate* isolate, Kind kind,
312301
int descriptor, FieldIndex field_index,

‎src/ic/ic.cc‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1133,12 +1133,13 @@ Handle<Object> KeyedLoadIC::LoadElementHandler(Handle<Map> receiver_map,
11331133
InstanceType instance_type = receiver_map->instance_type();
11341134
if (instance_type < FIRST_NONSTRING_TYPE) {
11351135
TRACE_HANDLER_STATS(isolate(), KeyedLoadIC_LoadIndexedStringDH);
1136-
if (IsAnyHas()) return LoadHandler::LoadSlow(isolate());
1136+
if (IsAnyHas()) return BUILTIN_CODE(isolate(), HasIC_Slow);
11371137
return LoadHandler::LoadIndexedString(isolate(), load_mode);
11381138
}
11391139
if (instance_type < FIRST_JS_RECEIVER_TYPE) {
11401140
TRACE_HANDLER_STATS(isolate(), KeyedLoadIC_SlowStub);
1141-
return LoadHandler::LoadSlow(isolate());
1141+
return IsAnyHas() ? BUILTIN_CODE(isolate(), HasIC_Slow)
1142+
: BUILTIN_CODE(isolate(), KeyedLoadIC_Slow);
11421143
}
11431144
if (instance_type == JS_PROXY_TYPE) {
11441145
return LoadHandler::LoadProxy(isolate());
@@ -1870,7 +1871,7 @@ Handle<Object> KeyedStoreIC::StoreElementHandler(
18701871
}
18711872

18721873
// TODO(ishell): move to StoreHandler::StoreElement().
1873-
Handle<Object> code;
1874+
Handle<Code> code;
18741875
if (receiver_map->has_sloppy_arguments_elements()) {
18751876
// TODO(jgruber): Update counter name.
18761877
TRACE_HANDLER_STATS(isolate(), KeyedStoreIC_KeyedStoreSloppyArgumentsStub);
@@ -1886,16 +1887,18 @@ Handle<Object> KeyedStoreIC::StoreElementHandler(
18861887
} else if (IsStoreInArrayLiteralICKind(kind())) {
18871888
// TODO(jgruber): Update counter name.
18881889
TRACE_HANDLER_STATS(isolate(), StoreInArrayLiteralIC_SlowStub);
1889-
return StoreHandler::StoreSlow(isolate(), store_mode);
1890+
code =
1891+
CodeFactory::StoreInArrayLiteralIC_Slow(isolate(), store_mode).code();
18901892
} else {
18911893
// TODO(jgruber): Update counter name.
18921894
TRACE_HANDLER_STATS(isolate(), KeyedStoreIC_StoreElementStub);
18931895
DCHECK(DICTIONARY_ELEMENTS == receiver_map->elements_kind() ||
18941896
receiver_map->has_frozen_elements());
1895-
code = StoreHandler::StoreSlow(isolate(), store_mode);
1897+
code = CodeFactory::KeyedStoreIC_Slow(isolate(), store_mode).code();
18961898
}
18971899

18981900
if (IsStoreInArrayLiteralICKind(kind())) return code;
1901+
18991902
Handle<Object> validity_cell =
19001903
Map::GetOrCreatePrototypeChainValidityCell(receiver_map, isolate());
19011904
if (validity_cell->IsSmi()) {

0 commit comments

Comments
 (0)