[cleanup] Remove Heap::kOldSpaceRoots constant

... in favour of RootsTable::IsImmortalImmovable().

Bug: v8:8238
Change-Id: Ic8434a1658b9ba982a93dd268dbfe52a6cc5c6a2
Reviewed-on: https://chromium-review.googlesource.com/c/1270582
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56472}

src/heap/heap.h

@@ -243,9 +243,6 @@ class Heap {
   static const int kNoGCFlags = 0;
   static const int kReduceMemoryFootprintMask = 1;
 
-  // The roots that have an index less than this are always in old space.
-  static const int kOldSpaceRoots = 0x20;
-
   // The minimum size of a HeapObject on the heap.
   static const int kMinObjectSizeInWords = 2;
 

src/roots-inl.h

@@ -12,7 +12,7 @@
 namespace v8 {
 namespace internal {
 
-V8_INLINE bool operator<(RootIndex lhs, RootIndex rhs) {
+V8_INLINE constexpr bool operator<(RootIndex lhs, RootIndex rhs) {
   typedef typename std::underlying_type<RootIndex>::type type;
   return static_cast<type>(lhs) < static_cast<type>(rhs);
 }
@@ -70,25 +70,6 @@ bool RootsTable::IsImmortalImmovable(RootIndex root_index) {
   }
 }
 
-Object** RootsTable::read_only_roots_end() {
-// Enumerate the read-only roots into an expression of the form:
-// (root_1, root_2, root_3, ..., root_n)
-// This evaluates to root_n, but Clang warns that the other values in the list
-// are unused so suppress that warning.
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wunused-value"
-#endif
-#define ROOT(type, name, CamelName) , RootIndex::k##CamelName
-  constexpr RootIndex kLastReadOnlyRoot =
-      (RootIndex::kFirstRoot READ_ONLY_ROOT_LIST(ROOT));
-#undef ROOT
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
-  return &roots_[static_cast<size_t>(kLastReadOnlyRoot) + 1];
-}
-
 }  // namespace internal
 }  // namespace v8
 

src/roots.h

@@ -60,9 +60,6 @@ class Symbol;
   V(FixedArray, empty_fixed_array, EmptyFixedArray)                            \
   V(DescriptorArray, empty_descriptor_array, EmptyDescriptorArray)             \
   /* Entries beyond the first 32                                            */ \
-  /* The roots above this line should be boring from a GC point of view.    */ \
-  /* This means they are never in new space and never on a page that is     */ \
-  /* being compacted.*/                                                        \
   /* Oddballs */                                                               \
   V(Oddball, arguments_marker, ArgumentsMarker)                                \
   V(Oddball, exception, Exception)                                             \
@@ -368,6 +365,17 @@ enum class RootIndex : uint16_t {
   kFirstStrongRoot = kFirstRoot,
   kLastStrongRoot = kStringTable - 1,
 
+#define ROOT(...) +1
+  kReadOnlyRootsCount = 0 READ_ONLY_ROOT_LIST(ROOT),
+#undef ROOT
+  kFirstReadOnlyRoot = kFirstRoot,
+  kLastReadOnlyRoot = kFirstReadOnlyRoot + kReadOnlyRootsCount - 1,
+
+  kFirstImmortalImmovableRoot = kFirstReadOnlyRoot,
+  // TODO(ishell): reorder the STRONG_MUTABLE_ROOT_LIST and set this constant to
+  // correct value.
+  kLastImmortalImmovableRoot = kLastReadOnlyRoot,
+
   kFirstSmiRoot = kStringTable + 1,
   kLastSmiRoot = kLastRoot
 };
@@ -408,9 +416,11 @@ class RootsTable {
 
  private:
   Object** read_only_roots_begin() {
-    return &roots_[static_cast<size_t>(RootIndex::kFirstStrongRoot)];
+    return &roots_[static_cast<size_t>(RootIndex::kFirstReadOnlyRoot)];
+  }
+  inline Object** read_only_roots_end() {
+    return &roots_[static_cast<size_t>(RootIndex::kLastReadOnlyRoot) + 1];
   }
-  inline Object** read_only_roots_end();
 
   Object** strong_roots_begin() {
     return &roots_[static_cast<size_t>(RootIndex::kFirstStrongRoot)];

src/snapshot/deserializer.cc

@@ -659,7 +659,12 @@ bool Deserializer<AllocatorT>::ReadData(MaybeObject** current,
         break;
       }
 
-      STATIC_ASSERT(kNumberOfRootArrayConstants == Heap::kOldSpaceRoots);
+      // First kNumberOfRootArrayConstants roots are guaranteed to be in
+      // the old space.
+      STATIC_ASSERT(
+          static_cast<int>(RootIndex::kFirstImmortalImmovableRoot) == 0);
+      STATIC_ASSERT(kNumberOfRootArrayConstants <=
+                    static_cast<int>(RootIndex::kLastImmortalImmovableRoot));
       STATIC_ASSERT(kNumberOfRootArrayConstants == 32);
       SIXTEEN_CASES(kRootArrayConstantsWithSkip)
       SIXTEEN_CASES(kRootArrayConstantsWithSkip + 16) {

test/cctest/test-roots.cc

@@ -27,6 +27,8 @@ AllocationSpace GetSpaceFromObject(Object* object) {
 TEST(TestReadOnlyRoots) {
   ReadOnlyRoots roots(CcTest::i_isolate());
 
+  STATIC_ASSERT(static_cast<int>(RootIndex::kFirstReadOnlyRoot) == 0);
+  STATIC_ASSERT(static_cast<int>(RootIndex::kReadOnlyRootsCount) == 501);
   READ_ONLY_ROOT_LIST(CHECK_IN_RO_SPACE)
 }