[KeyedLoadIC] Support Smi "handlers" for element loads

src/code-stub-assembler.cc

Index: src/code-stub-assembler.cc
diff --git a/src/code-stub-assembler.cc b/src/code-stub-assembler.cc
index 6e3cd99dd5b2083f68d5dc37ab10aed543e705ed..fa927c21cc7337e7bd25e015f10104457b1eb990 100644
--- a/src/code-stub-assembler.cc
+++ b/src/code-stub-assembler.cc
@@ -6,6 +6,7 @@
 #include "src/code-factory.h"
 #include "src/frames-inl.h"
 #include "src/frames.h"
+#include "src/ic/handler-compiler.h"
 #include "src/ic/stub-cache.h"
 
 namespace v8 {
@@ -2977,21 +2978,214 @@ void CodeStubAssembler::TryProbeStubCache(
   }
 }
 
-void CodeStubAssembler::HandleLoadICHandlerCase(const LoadICParameters* p,
-                                                Node* handler, Label* miss) {
+void CodeStubAssembler::HandleLoadICHandlerCase(
+    const LoadICParameters* p, Node* handler, Label* miss,
+    ElementSupport support_elements) {
   Comment("have_handler");
   Label call_handler(this);
   GotoUnless(WordIsSmi(handler), &call_handler);
 
-  // |handler| is a Smi. It encodes a field index as obtained by
-  // FieldIndex.GetLoadByFieldOffset().
-  // TODO(jkummerow): For KeyedLoadICs, extend this scheme to encode
-  // fast *element* loads.
+  // |handler| is a Smi, encoding what to do. The last bit distinguishes
+  // element and property loads.
   {
     Variable var_double_value(this, MachineRepresentation::kFloat64);
     Label rebox_double(this, &var_double_value);
 
     Node* handler_word = SmiUntag(handler);
+    if (support_elements == kSupportElements) {

[Igor Sheludko, 2016/07/28 10:01:11]

I think this new code (and maybe the property case as well) needs a separate
function.

[Jakob Kummerow, 2016/08/03 11:44:11]

Done (did some splitting, though not exactly here). I think this results in
readable chunks. Let me know if you'd prefer further splitting.

+      Variable var_length(this, MachineRepresentation::kTagged);
+      Label property(this), is_array(this), length_loaded(this, &var_length);
+      Node* is_element_bit = WordAnd(handler_word, IntPtrConstant(1));

[Igor Sheludko, 2016/07/28 10:01:11]

It would be nice to have such a SmiHandlerIsProperty bitfield type somewhere and
define FieldOffsetIsInobject and KeyedLoadIsJsArray using
SmiHandlerIsProperty::kNext.

[Jakob Kummerow, 2016/08/03 11:44:09]

Done.

+      GotoUnless(WordEqual(is_element_bit, IntPtrConstant(0)), &property);
+
+      Comment("element_load");
+      GotoUnless(WordIsSmi(p->name), miss);

[Igor Sheludko, 2016/07/28 10:01:11]

Future improvement: support "123"-like unique strings instead of missing.

[Jakob Kummerow, 2016/08/03 11:44:09]

Possibly, yes. That change would have to be made in conjunction with teaching
optimizing compilers about it. As it is, KeyedLoadIC type feedback implies that
optimized code can safely assume that the key was a Smi. If the IC has seen and
converted "123"-like strings, we might want to record that fact somewhere.

+      Node* key = SmiUntag(p->name);
+      Node* elements = LoadElements(p->receiver);
+      Node* is_jsarray = WordAnd(
+          handler_word,
+          IntPtrConstant(ElementHandlerCompiler::KeyedLoadIsJsarray::kMask));
+      GotoUnless(WordEqual(is_jsarray, IntPtrConstant(0)), &is_array);
+      var_length.Bind(SmiUntag(LoadFixedArrayBaseLength(elements)));
+      Goto(&length_loaded);
+      Bind(&is_array);

[Igor Sheludko, 2016/07/28 10:01:11]

Consider wrapping blocks in {} for readability.

[Jakob Kummerow, 2016/08/03 11:44:09]

Done.

+      var_length.Bind(
+          SmiUntag(LoadObjectField(p->receiver, JSArray::kLengthOffset)));
+      Goto(&length_loaded);
+
+      Bind(&length_loaded);

[Igor Sheludko, 2016/07/28 10:01:11]

Same here.

[Jakob Kummerow, 2016/08/03 11:44:12]

Done.

+      // Bounds check.
+      GotoUnless(UintPtrLessThan(key, var_length.value()), miss);
+      Node* convert_hole = WordAnd(

[Igor Sheludko, 2016/07/28 10:01:11]

You can probably move this to if_hole block although TF should be smart enough
to put this code into the right block.

[Jakob Kummerow, 2016/08/03 11:44:11]

Done.

+          handler_word,
+          IntPtrConstant(ElementHandlerCompiler::KeyedLoadConvertHole::kMask));
+      Node* elements_kind =
+          WordShr(handler_word,
+                  IntPtrConstant(
+                      ElementHandlerCompiler::KeyedLoadElementsKind::kShift));
+
+      Label typed_array(this), fast_packed(this), fast_holey(this),

[Igor Sheludko, 2016/07/28 10:01:10]

As far as I understand we have a kind of naming convention in this file that the
names of labels which are targets of various if checks start with if_.

[Jakob Kummerow, 2016/08/03 11:44:11]

Done.

+          fast_double(this), fast_holey_double(this), hole(this);

[Igor Sheludko, 2016/07/28 10:01:11]

STATIC_ASSERT(LAST_ELEMENTS_KIND == LAST_FIXED_TYPED_ARRAY_ELEMENTS_KIND);
I hope it will fail once we enable NO_ELEMENTS kind for real.

[Jakob Kummerow, 2016/08/03 11:44:11]

Done.

+      GotoIf(IntPtrGreaterThanOrEqual(
+                 elements_kind,
+                 IntPtrConstant(FIRST_FIXED_TYPED_ARRAY_ELEMENTS_KIND)),
+             &typed_array);
+#define JUMP_IF_KIND(kind, label) \
+  GotoIf(WordEqual(elements_kind, IntPtrConstant(kind)), label);

[Igor Sheludko, 2016/07/28 10:01:10]

It looks like a switch construct will fit better here.

[Jakob Kummerow, 2016/08/03 11:44:09]

Done.

+
+      JUMP_IF_KIND(FAST_SMI_ELEMENTS, &fast_packed);
+      JUMP_IF_KIND(FAST_ELEMENTS, &fast_packed);
+      JUMP_IF_KIND(FAST_HOLEY_SMI_ELEMENTS, &fast_holey);
+      JUMP_IF_KIND(FAST_HOLEY_ELEMENTS, &fast_holey);
+      JUMP_IF_KIND(FAST_DOUBLE_ELEMENTS, &fast_double);
+      JUMP_IF_KIND(FAST_HOLEY_DOUBLE_ELEMENTS, &fast_holey_double);
+      // All other kinds are unimplemented.
+      DebugBreak();

[Igor Sheludko, 2016/07/28 10:01:11]

?

[Jakob Kummerow, 2016/08/03 11:44:11]

I want to crash if we get here. Think "UNREACHABLE()". Added comment.

+      Goto(miss);
+#undef JUMP_IF_KIND
+
+      Bind(&fast_packed);
+      Comment("fast packed elements");

[Igor Sheludko, 2016/07/28 10:01:11]

Consider wrapping blocks in {} for readability.

[Jakob Kummerow, 2016/08/03 11:44:11]

Done.

+      // TODO(jkummerow): The Load*Element helpers add movsxlq instructions
+      // on x64 which we don't need here, because |key| is an IntPtr already.
+      // Do something about that.

[Igor Sheludko, 2016/07/28 10:01:10]

We just need to introduce INTPTR_PARAMETERS mode.

[Jakob Kummerow, 2016/08/03 11:44:09]

Yes, that's what I was thinking too. Follow-up CL.

+      Return(LoadFixedArrayElement(elements, key));
+
+      Bind(&fast_holey);
+      {
+        Comment("fast holey elements");
+        Node* element = LoadFixedArrayElement(elements, key);
+        GotoIf(WordEqual(element, TheHoleConstant()), &hole);
+        Return(element);
+      }
+
+      Bind(&fast_double);
+      Comment("packed double elements");

[Igor Sheludko, 2016/07/28 10:01:11]

Same here.

[Jakob Kummerow, 2016/08/03 11:44:09]

Done.

+      var_double_value.Bind(
+          LoadFixedDoubleArrayElement(elements, key, MachineType::Float64()));
+      Goto(&rebox_double);
+
+      Bind(&fast_holey_double);
+      {
+        Comment("holey double elements");
+        if (kPointerSize == kDoubleSize) {
+          Node* raw_element =
+              LoadFixedDoubleArrayElement(elements, key, MachineType::Uint64());
+          Node* the_hole = Int64Constant(kHoleNanInt64);
+          GotoIf(Word64Equal(raw_element, the_hole), &hole);
+        } else {
+          Node* element_upper =
+              LoadFixedDoubleArrayElement(elements, key, MachineType::Uint32(),
+                                          kIeeeDoubleExponentWordOffset);
+          GotoIf(Word32Equal(element_upper, Int32Constant(kHoleNanUpper32)),
+                 &hole);
+        }
+        var_double_value.Bind(
+            LoadFixedDoubleArrayElement(elements, key, MachineType::Float64()));
+        Goto(&rebox_double);
+      }
+
+      Bind(&hole);
+      {
+        Comment("convert hole");
+        GotoIf(WordEqual(convert_hole, IntPtrConstant(0)), miss);
+        Node* protector_cell = LoadRoot(Heap::kArrayProtectorRootIndex);
+        GotoUnless(
+            WordEqual(LoadObjectField(protector_cell, Cell::kValueOffset),
+                      SmiConstant(Smi::FromInt(Isolate::kArrayProtectorValid))),
+            miss);
+        Return(UndefinedConstant());
+      }
+
+      Bind(&typed_array);

[Igor Sheludko, 2016/07/28 10:01:11]

And here.

[Jakob Kummerow, 2016/08/03 11:44:09]

Done.

+      Comment("typed elements");
+      // Check if buffer has been neutered.
+      Node* buffer =
+          LoadObjectField(p->receiver, JSArrayBufferView::kBufferOffset);
+      Node* bitfield = LoadObjectField(buffer, JSArrayBuffer::kBitFieldOffset,
+                                       MachineType::Uint32());
+      Node* neutered_bit = Word32And(
+          bitfield, Int32Constant(1 << JSArrayBuffer::WasNeutered::kShift));

[Igor Sheludko, 2016/07/28 10:01:11]

JSArrayBuffer::WasNeutered::kMask?

[Jakob Kummerow, 2016/08/03 11:44:09]

Done.

+      GotoUnless(Word32Equal(neutered_bit, Int32Constant(0)), miss);
+      // Backing store = external_pointer + base_pointer.
+      Node* external_pointer =
+          LoadObjectField(elements, FixedTypedArrayBase::kExternalPointerOffset,
+                          MachineType::Pointer());
+      Node* base_pointer =
+          LoadObjectField(elements, FixedTypedArrayBase::kBasePointerOffset);
+      Node* backing_store = IntPtrAdd(external_pointer, base_pointer);
+
+      const int kTypedElementsKindCount =
+          LAST_FIXED_TYPED_ARRAY_ELEMENTS_KIND -
+          FIRST_FIXED_TYPED_ARRAY_ELEMENTS_KIND + 1;
+      Label* elements_kind_labels[kTypedElementsKindCount];
+      int32_t elements_kinds[kTypedElementsKindCount];
+      for (int i = 0; i < kTypedElementsKindCount; i++) {
+        elements_kinds[i] = i + FIRST_FIXED_TYPED_ARRAY_ELEMENTS_KIND;
+        elements_kind_labels[i] = new Label(this);
+      }
+      Switch(elements_kind, miss, elements_kinds, elements_kind_labels,
+             static_cast<size_t>(kTypedElementsKindCount));
+
+      for (int i = 0; i < kTypedElementsKindCount; i++) {
+        ElementsKind kind = static_cast<ElementsKind>(elements_kinds[i]);
+        Bind(elements_kind_labels[i]);
+        Comment(ElementsKindToString(kind));
+        switch (kind) {
+          case UINT8_ELEMENTS:
+          case UINT8_CLAMPED_ELEMENTS:
+            Return(SmiTag(Load(MachineType::Uint8(), backing_store, key)));
+            break;
+          case INT8_ELEMENTS:
+            Return(SmiTag(Load(MachineType::Int8(), backing_store, key)));
+            break;
+          case UINT16_ELEMENTS: {
+            Node* index = WordShl(key, IntPtrConstant(1));
+            Return(SmiTag(Load(MachineType::Uint16(), backing_store, index)));
+            break;
+          }
+          case INT16_ELEMENTS: {
+            Node* index = WordShl(key, IntPtrConstant(1));
+            Return(SmiTag(Load(MachineType::Int16(), backing_store, index)));
+            break;
+          }
+          case UINT32_ELEMENTS: {
+            Node* index = WordShl(key, IntPtrConstant(2));
+            Node* element = Load(MachineType::Uint32(), backing_store, index);
+            Return(ChangeUint32ToTagged(element));
+            break;
+          }
+          case INT32_ELEMENTS: {
+            Node* index = WordShl(key, IntPtrConstant(2));
+            Node* element = Load(MachineType::Int32(), backing_store, index);
+            Return(ChangeInt32ToTagged(element));
+            break;
+          }
+          case FLOAT32_ELEMENTS: {
+            Node* index = WordShl(key, IntPtrConstant(2));
+            Node* element = Load(MachineType::Float32(), backing_store, index);
+            var_double_value.Bind(ChangeFloat32ToFloat64(element));
+            Goto(&rebox_double);
+            break;
+          }
+          case FLOAT64_ELEMENTS: {
+            Node* index = WordShl(key, IntPtrConstant(3));
+            Node* element = Load(MachineType::Float64(), backing_store, index);
+            var_double_value.Bind(element);
+            Goto(&rebox_double);
+            break;
+          }
+          default:
+            UNREACHABLE();
+        }
+        // Don't forget to clean up.
+        delete elements_kind_labels[i];
+      }
+
+      Bind(&property);
+      Comment("property_load");
+    }
+
     // |handler_word| is a field index as obtained by
     // FieldIndex.GetLoadByFieldOffset():
     Label inobject_double(this), out_of_object(this),
@@ -3102,7 +3296,7 @@ void CodeStubAssembler::KeyedLoadIC(const LoadICParameters* p) {
                                       &var_handler, &try_polymorphic);
   Bind(&if_handler);
   {
-    HandleLoadICHandlerCase(p, var_handler.value(), &miss);
+    HandleLoadICHandlerCase(p, var_handler.value(), &miss, kSupportElements);
   }
 
   Bind(&try_polymorphic);