[dict-proto] Allow storing certain PropertyDetails in single byte

This CL adds PropertyDetails::ToByte and ::FromByte. These are not
applicable to all PropertDetails, but only those for dictionary-backed
properties with an (unused) enumeration index with value 0.

The motivation for this is that those dictionare backing stores that
don't store the enumeration order in the PropertyDetails but store it
in the table itself (like OrderedNameDictionary and the upcoming
SwissNameDictionary), can store PropertyDetails in an array of bytes.

Bug: v8:11388
Change-Id: Id346b924cd7c67b2f33cbc7a7807eec31cefbeec
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2672029
Commit-Queue: Frank Emrich <emrich@google.com>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Marja Hölttä <marja@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72528}

src/objects/property-details.h

@@ -211,16 +211,14 @@ static const int kInvalidEnumCacheSentinel =
 // A PropertyCell's property details contains a cell type that is meaningful if
 // the cell is still valid (does not hold the hole).
 enum class PropertyCellType {
+  kMutable,       // Cell will no longer be tracked as constant.
   kUndefined,     // The PREMONOMORPHIC of property cells.
   kConstant,      // Cell has been assigned only once.
   kConstantType,  // Cell has been assigned only one type.
-  kMutable,       // Cell will no longer be tracked as constant.
-  // Arbitrary choice for dictionaries not holding cells.
+  // Value for dictionaries not holding cells, must have value 0:
   kNoCell = kMutable,
 };
 
-std::ostream& operator<<(std::ostream& os, PropertyCellType type);
-
 // PropertyDetails captures type and attributes for a property.
 // They are used both in property dictionaries and instance descriptors.
 class PropertyDetails {
@@ -343,6 +341,8 @@ class PropertyDetails {
     return PropertyCellTypeField::decode(value_);
   }
 
+  bool operator==(const PropertyDetails& b) const { return value_ == b.value_; }
+
   // Bit fields in value_ (type, shift, size). Must be public so the
   // constants can be embedded in generated code.
   using KindField = base::BitField<PropertyKind, 0, 1>;
@@ -356,7 +356,7 @@ class PropertyDetails {
   static const int kAttributesDontEnumMask =
       (DONT_ENUM << AttributesField::kShift);
 
-  // Bit fields for normalized objects.
+  // Bit fields for normalized/dictionary mode objects.
   using PropertyCellTypeField = AttributesField::Next<PropertyCellType, 2>;
   using DictionaryStorageField = PropertyCellTypeField::Next<uint32_t, 23>;
 
@@ -371,6 +371,17 @@ class PropertyDetails {
   STATIC_ASSERT(DictionaryStorageField::kLastUsedBit < 31);
   STATIC_ASSERT(FieldIndexField::kLastUsedBit < 31);
 
+  // DictionaryStorageField must be the last field, so that overflowing it
+  // doesn't overwrite other fields.
+  STATIC_ASSERT(DictionaryStorageField::kLastUsedBit == 30);
+
+  // All bits for non-global dictionary mode objects except enumeration index
+  // must fit in a byte.
+  STATIC_ASSERT(KindField::kLastUsedBit < 8);
+  STATIC_ASSERT(ConstnessField::kLastUsedBit < 8);
+  STATIC_ASSERT(AttributesField::kLastUsedBit < 8);
+  STATIC_ASSERT(LocationField::kLastUsedBit < 8);
+
   static const int kInitialIndex = 1;
 
   static constexpr PropertyConstness kConstIfDictConstnessTracking =
@@ -395,6 +406,42 @@ class PropertyDetails {
   void PrintAsSlowTo(std::ostream& out, bool print_dict_index);
   void PrintAsFastTo(std::ostream& out, PrintMode mode = kPrintFull);
 
+  // Encodes those property details for non-global dictionary properties
+  // with an enumeration index of 0 as a single byte.
+  uint8_t ToByte() {
+    // We only care about the value of KindField, ConstnessField, and
+    // AttributesField. LocationField is also stored, but it will always be
+    // kField. We've statically asserted earlier that all those fields fit into
+    // a byte together.
+
+    // PropertyCellTypeField comes next, its value must be kNoCell == 0 for
+    // dictionary mode PropertyDetails anyway.
+    DCHECK_EQ(PropertyCellType::kNoCell, cell_type());
+    STATIC_ASSERT(static_cast<int>(PropertyCellType::kNoCell) == 0);
+
+    // Only to be used when the enum index isn't actually maintained
+    // by the PropertyDetails:
+    DCHECK_EQ(0, dictionary_index());
+
+    return value_;
+  }
+
+  // Only to be used for bytes obtained by ToByte. In particular, only used for
+  // non-global dictionary properties.
+  static PropertyDetails FromByte(uint8_t encoded_details) {
+    // The 0-extension to 32bit sets PropertyCellType to kNoCell and
+    // enumeration index to 0, as intended. Everything else is obtained from
+    // |encoded_details|.
+
+    PropertyDetails details(encoded_details);
+
+    DCHECK_EQ(0, details.dictionary_index());
+    DCHECK_EQ(PropertyLocation::kField, details.location());
+    DCHECK_EQ(PropertyCellType::kNoCell, details.cell_type());
+
+    return details;
+  }
+
  private:
   PropertyDetails(int value, int pointer) {
     value_ = DescriptorPointer::update(value, pointer);
@@ -410,6 +457,8 @@ class PropertyDetails {
     value_ = AttributesField::update(value, attributes);
   }
 
+  explicit PropertyDetails(uint32_t value) : value_{value} {}
+
   uint32_t value_;
 };
 
@@ -434,6 +483,8 @@ V8_EXPORT_PRIVATE std::ostream& operator<<(
     std::ostream& os, const PropertyAttributes& attributes);
 V8_EXPORT_PRIVATE std::ostream& operator<<(std::ostream& os,
                                            PropertyConstness constness);
+V8_EXPORT_PRIVATE std::ostream& operator<<(std::ostream& os,
+                                           PropertyCellType type);
 }  // namespace internal
 }  // namespace v8
 

test/cctest/BUILD.gn

@@ -264,6 +264,7 @@ v8_source_set("cctest_sources") {
     "test-persistent-handles.cc",
     "test-platform.cc",
     "test-profile-generator.cc",
+    "test-property-details.cc",
     "test-random-number-generator.cc",
     "test-regexp.cc",
     "test-representation.cc",

test/cctest/test-property-details.cc

@@ -0,0 +1,72 @@
+// Copyright 2021 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <limits>
+
+#include "src/objects/property-details.h"
+#include "test/cctest/cctest.h"
+
+namespace v8 {
+namespace internal {
+
+namespace {
+
+std::vector<PropertyDetails> make_details() {
+  std::vector<PropertyDetails> result;
+  for (PropertyKind kind : {PropertyKind::kData, PropertyKind::kAccessor}) {
+    for (PropertyConstness constness :
+         {PropertyConstness::kConst, PropertyConstness::kMutable}) {
+      for (PropertyCellType cell_type :
+           {PropertyCellType::kConstant, PropertyCellType::kConstantType,
+            PropertyCellType::kMutable, PropertyCellType::kUndefined,
+            PropertyCellType::kNoCell}) {
+        for (int attrs = 0; attrs < 8; ++attrs) {
+          PropertyAttributes attributes =
+              static_cast<PropertyAttributes>(attrs);
+          PropertyDetails details(kind, attributes, cell_type);
+          details = details.CopyWithConstness(constness);
+          result.push_back(details);
+        }
+      }
+    }
+  }
+  return result;
+}
+
+}  // namespace
+
+#ifndef DEBUG
+// This test will trigger a DCHECK failure in debug mode. We must ensure that in
+// release mode, the enum index doesn't interfere with other fields once it
+// becomes too large.
+TEST(ExceedMaxEnumerationIndex) {
+  int too_large_enum_index = std::numeric_limits<int>::max();
+
+  for (PropertyDetails d : make_details()) {
+    PropertyDetails copy(d);
+
+    d = d.set_index(too_large_enum_index);
+    CHECK_EQ(copy.kind(), d.kind());
+    CHECK_EQ(copy.location(), d.location());
+    CHECK_EQ(copy.attributes(), d.attributes());
+    CHECK_EQ(copy.cell_type(), d.cell_type());
+    CHECK_EQ(PropertyDetails::DictionaryStorageField::kMax,
+             d.dictionary_index());
+  }
+}
+#endif
+
+TEST(AsByte) {
+  for (PropertyDetails original : make_details()) {
+    if (original.cell_type() != PropertyCellType::kNoCell) continue;
+
+    uint8_t as_byte = original.ToByte();
+    PropertyDetails from_byte = PropertyDetails::FromByte(as_byte);
+
+    CHECK_EQ(original, from_byte);
+  }
+}
+
+}  // namespace internal
+}  // namespace v8