[interpreter] Do ToObject implicitly as part of CloneObject.

The CloneObject bytecode was only able to handle objects, null and
undefined, and explicit bytecode had to be generated to perform the
ToObject outside the bytecode (unlike the other IC bytecodes that
just perform the ToObject implicitly). That means the simplest possible
object cloning would also generate a sequence of 5 bytecodes (at least):

```
   Mov <register>, a0
   JumpIfNull @1
   JumpIfUndefined @1
   ToObject <register>
1: CloneObject <register>
```

That is quite wasteful and unnecessary, since the core logic in the
runtime already does the ToObject properly anyways. This change
refactors the CloneObjectIC slightly to behave more like the other ICs
and do the ToObject implicitly when necessary.

Bug: v8:7611, v8:9114, v8:9183, v8:9343
Change-Id: I11973e90bf875f154a5a7739287bee17041e4a7a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1649554Reviewed-by: Mythri Alle <mythria@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#62064}

src/ic/accessor-assembler.cc

@@ -3640,7 +3640,7 @@ void AccessorAssembler::GenerateStoreInArrayLiteralIC() {
 
 void AccessorAssembler::GenerateCloneObjectIC_Slow() {
   using Descriptor = CloneObjectWithVectorDescriptor;
-  TNode<HeapObject> source = CAST(Parameter(Descriptor::kSource));
+  TNode<Object> source = CAST(Parameter(Descriptor::kSource));
   TNode<Smi> flags = CAST(Parameter(Descriptor::kFlags));
   TNode<Context> context = CAST(Parameter(Descriptor::kContext));
 
@@ -3672,28 +3672,16 @@ void AccessorAssembler::GenerateCloneObjectIC_Slow() {
   }
 
   ReturnIf(IsNullOrUndefined(source), result);
+  source = ToObject_Inline(context, source);
 
-  CSA_ASSERT(this, IsJSReceiver(source));
-
-  Label call_runtime(this, Label::kDeferred);
-  Label done(this);
-
-  TNode<Map> map = LoadMap(source);
-  TNode<Int32T> type = LoadMapInstanceType(map);
-  {
-    Label cont(this);
-    GotoIf(IsJSObjectInstanceType(type), &cont);
-    GotoIf(InstanceTypeEqual(type, JS_PROXY_TYPE), &call_runtime);
-    GotoIfNot(IsStringInstanceType(type), &done);
-    Branch(SmiEqual(LoadStringLengthAsSmi(CAST(source)), SmiConstant(0)), &done,
-           &call_runtime);
-    BIND(&cont);
-  }
+  Label call_runtime(this, Label::kDeferred), done(this);
 
+  TNode<Map> source_map = LoadMap(CAST(source));
+  GotoIfNot(IsJSObjectMap(source_map), &call_runtime);
   GotoIfNot(IsEmptyFixedArray(LoadElements(CAST(source))), &call_runtime);
 
   ForEachEnumerableOwnProperty(
-      context, map, CAST(source), kPropertyAdditionOrder,
+      context, source_map, CAST(source), kPropertyAdditionOrder,
       [=](TNode<Name> key, TNode<Object> value) {
         SetPropertyInLiteral(context, result, key, value);
       },
@@ -3710,17 +3698,17 @@ void AccessorAssembler::GenerateCloneObjectIC_Slow() {
 
 void AccessorAssembler::GenerateCloneObjectIC() {
   using Descriptor = CloneObjectWithVectorDescriptor;
-  TNode<HeapObject> source = CAST(Parameter(Descriptor::kSource));
-  Node* flags = Parameter(Descriptor::kFlags);
-  Node* slot = Parameter(Descriptor::kSlot);
-  Node* vector = Parameter(Descriptor::kVector);
-  Node* context = Parameter(Descriptor::kContext);
+  TNode<Object> source = CAST(Parameter(Descriptor::kSource));
+  TNode<Smi> flags = CAST(Parameter(Descriptor::kFlags));
+  TNode<Smi> slot = CAST(Parameter(Descriptor::kSlot));
+  TNode<HeapObject> vector = CAST(Parameter(Descriptor::kVector));
+  TNode<Context> context = CAST(Parameter(Descriptor::kContext));
   TVARIABLE(MaybeObject, var_handler);
-  Label if_handler(this, &var_handler);
-  Label miss(this, Label::kDeferred), try_polymorphic(this, Label::kDeferred),
+  Label if_handler(this, &var_handler), miss(this, Label::kDeferred),
+      try_polymorphic(this, Label::kDeferred),
       try_megamorphic(this, Label::kDeferred), slow(this, Label::kDeferred);
 
-  TNode<Map> source_map = LoadMap(UncheckedCast<HeapObject>(source));
+  TNode<Map> source_map = LoadReceiverMap(source);
   GotoIf(IsDeprecatedMap(source_map), &miss);
 
   GotoIf(IsUndefined(vector), &slow);
@@ -3740,6 +3728,7 @@ void AccessorAssembler::GenerateCloneObjectIC() {
 
     Label allocate_object(this);
     GotoIf(IsNullOrUndefined(source), &allocate_object);
+    CSA_SLOW_ASSERT(this, IsJSObjectMap(source_map));
     CSA_SLOW_ASSERT(this, IsJSObjectMap(result_map));
 
     // The IC fast case should only be taken if the result map a compatible
@@ -3753,7 +3742,7 @@ void AccessorAssembler::GenerateCloneObjectIC() {
     // either an Smi, or a PropertyArray.
     // FIXME: Make a CSA macro for this
     TNode<Object> source_properties =
-        LoadObjectField(source, JSObject::kPropertiesOrHashOffset);
+        LoadObjectField(CAST(source), JSObject::kPropertiesOrHashOffset);
     {
       GotoIf(TaggedIsSmi(source_properties), &allocate_object);
       GotoIf(IsEmptyFixedArray(source_properties), &allocate_object);
@@ -3803,7 +3792,7 @@ void AccessorAssembler::GenerateCloneObjectIC() {
               TimesTaggedSize(UncheckedCast<IntPtrT>(field_index));
 
           if (may_use_mutable_heap_numbers) {
-            TNode<Object> field = LoadObjectField(source, field_offset);
+            TNode<Object> field = LoadObjectField(CAST(source), field_offset);
             field = CloneIfMutablePrimitive(field);
             TNode<IntPtrT> result_offset =
                 IntPtrAdd(field_offset, field_offset_difference);
@@ -3811,7 +3800,7 @@ void AccessorAssembler::GenerateCloneObjectIC() {
           } else {
             // Copy fields as raw data.
             TNode<IntPtrT> field =
-                LoadObjectField<IntPtrT>(source, field_offset);
+                LoadObjectField<IntPtrT>(CAST(source), field_offset);
             TNode<IntPtrT> result_offset =
                 IntPtrAdd(field_offset, field_offset_difference);
             StoreObjectFieldNoWriteBarrier(object, result_offset, field);

src/ic/ic.cc

@@ -2583,10 +2583,9 @@ static bool CanFastCloneObject(Handle<Map> map) {
   return true;
 }
 
-static Handle<Map> FastCloneObjectMap(Isolate* isolate,
-                                      Handle<HeapObject> source, int flags) {
-  Handle<Map> source_map(source->map(), isolate);
-  SLOW_DCHECK(source->IsNullOrUndefined() || CanFastCloneObject(source_map));
+static Handle<Map> FastCloneObjectMap(Isolate* isolate, Handle<Map> source_map,
+                                      int flags) {
+  SLOW_DCHECK(CanFastCloneObject(source_map));
   Handle<JSFunction> constructor(isolate->native_context()->object_function(),
                                  isolate);
   DCHECK(constructor->has_initial_map());
@@ -2611,9 +2610,10 @@ static Handle<Map> FastCloneObjectMap(Isolate* isolate,
     Map::SetPrototype(isolate, map, isolate->factory()->null_value());
   }
 
-  if (source->IsNullOrUndefined() || !source_map->NumberOfOwnDescriptors()) {
+  if (source_map->NumberOfOwnDescriptors() == 0) {
     return map;
   }
+  DCHECK(!source_map->IsNullOrUndefinedMap());
 
   if (map.is_identical_to(initial_map)) {
     map = Map::Copy(isolate, map, "InitializeClonedDescriptors");
@@ -2638,7 +2638,7 @@ static Handle<Map> FastCloneObjectMap(Isolate* isolate,
 }
 
 static MaybeHandle<JSObject> CloneObjectSlowPath(Isolate* isolate,
-                                                 Handle<HeapObject> source,
+                                                 Handle<Object> source,
                                                  int flags) {
   Handle<JSObject> new_object;
   if (flags & ObjectLiteral::kHasNullPrototype) {
@@ -2662,35 +2662,31 @@ static MaybeHandle<JSObject> CloneObjectSlowPath(Isolate* isolate,
 RUNTIME_FUNCTION(Runtime_CloneObjectIC_Miss) {
   HandleScope scope(isolate);
   DCHECK_EQ(4, args.length());
-  Handle<HeapObject> source = args.at<HeapObject>(0);
+  Handle<Object> source = args.at<Object>(0);
   int flags = args.smi_at(1);
 
-  MigrateDeprecated(source);
-
-  FeedbackSlot slot = FeedbackVector::ToSlot(args.smi_at(2));
-  Handle<HeapObject> maybe_vector = args.at<HeapObject>(3);
-  if (maybe_vector->IsUndefined()) {
-    RETURN_RESULT_OR_FAILURE(isolate,
-                             CloneObjectSlowPath(isolate, source, flags));
-  }
-
-  DCHECK(maybe_vector->IsFeedbackVector());
-  Handle<FeedbackVector> vector = Handle<FeedbackVector>::cast(maybe_vector);
-
-  FeedbackNexus nexus(vector, slot);
-  Handle<Map> source_map(source->map(), isolate);
+  if (MigrateDeprecated(source)) {
+    FeedbackSlot slot = FeedbackVector::ToSlot(args.smi_at(2));
+    Handle<HeapObject> maybe_vector = args.at<HeapObject>(3);
+    if (maybe_vector->IsFeedbackVector()) {
+      FeedbackNexus nexus(Handle<FeedbackVector>::cast(maybe_vector), slot);
+      if (!source->IsSmi() && !nexus.IsMegamorphic()) {
+        Handle<Map> source_map(Handle<HeapObject>::cast(source)->map(),
+                               isolate);
+        if (CanFastCloneObject(source_map)) {
+          Handle<Map> target_map =
+              FastCloneObjectMap(isolate, source_map, flags);
+          nexus.ConfigureCloneObject(source_map, target_map);
+          return *target_map;
+        }
 
-  if (!CanFastCloneObject(source_map) || nexus.IsMegamorphic()) {
-    // Migrate to slow mode if needed.
-    nexus.ConfigureMegamorphic();
-    RETURN_RESULT_OR_FAILURE(isolate,
-                             CloneObjectSlowPath(isolate, source, flags));
+        nexus.ConfigureMegamorphic();
+      }
+    }
   }
 
-  Handle<Map> result_map = FastCloneObjectMap(isolate, source, flags);
-  nexus.ConfigureCloneObject(source_map, result_map);
-
-  return *result_map;
+  RETURN_RESULT_OR_FAILURE(isolate,
+                           CloneObjectSlowPath(isolate, source, flags));
 }
 
 RUNTIME_FUNCTION(Runtime_StoreCallbackProperty) {

src/interpreter/bytecode-generator.cc

@@ -2366,13 +2366,6 @@ void BytecodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
     RegisterAllocationScope register_scope(this);
     Expression* property = expr->properties()->first()->value();
     Register from_value = VisitForRegisterValue(property);
-
-    BytecodeLabels clone_object(zone());
-    builder()->JumpIfUndefined(clone_object.New());
-    builder()->JumpIfNull(clone_object.New());
-    builder()->ToObject(from_value);
-
-    clone_object.Bind(builder());
     int clone_index = feedback_index(feedback_spec()->AddCloneObjectSlot());
     builder()->CloneObject(from_value, flags, clone_index);
     builder()->StoreAccumulatorInRegister(literal);

test/mjsunit/es9/object-spread-basic.js

@@ -11,6 +11,11 @@ assertEquals({}, y = { ...undefined });
 assertEquals({}, y = { ...null });
 
 assertEquals({}, y = { ...1 });
+assertEquals({}, y = { ...1n });
+assertEquals({}, y = { ...NaN });
+assertEquals({}, y = { ...false });
+assertEquals({}, y = { ...true });
+assertEquals({}, y = { ...Symbol() });
 assertEquals({0: 'f', 1: 'o', 2: 'o'}, y = { ...'foo' });
 assertEquals({0: 0, 1: 1}, y = { ...[0, 1] });
 assertEquals({}, { ...new Proxy({}, {}) });