Revert "[cleanup] Remove Heap::kOldSpaceRoots constant"

This reverts commit e3a42cfd.

Reason for revert: Breaking compilation on "V8 Linux - noi18n - debug" bot, see https://ci.chromium.org/p/v8/builders/luci.v8.ci/V8%20Linux%20-%20noi18n%20-%20debug/23170

Original change's description:
> [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}

TBR=ulan@chromium.org,yangguo@chromium.org,ishell@chromium.org

Change-Id: I3a160716c9d558f4ab89b45a7257a461733f7273
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:8238
Reviewed-on: https://chromium-review.googlesource.com/c/1270588Reviewed-by: Maya Lekova <mslekova@chromium.org>
Commit-Queue: Maya Lekova <mslekova@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56475}

src/heap/heap.h

@@ -243,6 +243,9 @@ 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 constexpr bool operator<(RootIndex lhs, RootIndex rhs) {
+V8_INLINE bool operator<(RootIndex lhs, RootIndex rhs) {
   typedef typename std::underlying_type<RootIndex>::type type;
   return static_cast<type>(lhs) < static_cast<type>(rhs);
 }
@@ -70,6 +70,25 @@ 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,6 +60,9 @@ 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)                                             \
@@ -365,17 +368,6 @@ 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
 };
@@ -416,11 +408,9 @@ class RootsTable {
 
  private:
   Object** read_only_roots_begin() {
-    return &roots_[static_cast<size_t>(RootIndex::kFirstReadOnlyRoot)];
-  }
-  inline Object** read_only_roots_end() {
-    return &roots_[static_cast<size_t>(RootIndex::kLastReadOnlyRoot) + 1];
+    return &roots_[static_cast<size_t>(RootIndex::kFirstStrongRoot)];
   }
+  inline Object** read_only_roots_end();
 
   Object** strong_roots_begin() {
     return &roots_[static_cast<size_t>(RootIndex::kFirstStrongRoot)];

src/snapshot/deserializer.cc

@@ -659,12 +659,7 @@ bool Deserializer<AllocatorT>::ReadData(MaybeObject** current,
         break;
       }
 
-      // 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 == Heap::kOldSpaceRoots);
       STATIC_ASSERT(kNumberOfRootArrayConstants == 32);
       SIXTEEN_CASES(kRootArrayConstantsWithSkip)
       SIXTEEN_CASES(kRootArrayConstantsWithSkip + 16) {

test/cctest/test-roots.cc

@@ -27,8 +27,6 @@ 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)
 }