Reland "[builtins] Load external references from the external-reference-table"

This is a reland of 9afde91b

Original change's description:
> [builtins] Load external references from the external-reference-table
>
> Off-heap code cannot embed external references. With this CL, we load
> from the external reference table (reached through the root pointer)
> instead.
>
> In a follow-up, the table could be stored within the isolate itself,
> removing one more level of indirection.
>
> Bug: v8:6666
> Change-Id: I4c612ad3d4112ec03c3b389f5bfb9cdc3dc8a671
> Reviewed-on: https://chromium-review.googlesource.com/970468
> Commit-Queue: Jakob Gruber <jgruber@chromium.org>
> Reviewed-by: Yang Guo <yangguo@chromium.org>
> Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#52073}

TBR=mstarzinger@chromium.org

Bug: v8:6666, v8:7580
Change-Id: I163cfc15605c1183b79ead77df0e37d71d60b6f7
Reviewed-on: https://chromium-review.googlesource.com/972821
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52118}

src/builtins/constants-table-builder.cc

@@ -46,25 +46,6 @@ uint32_t BuiltinsConstantsTableBuilder::AddObject(Handle<Object> object) {
   }
 }
 
-uint32_t BuiltinsConstantsTableBuilder::AddExternalReference(
-    ExternalReference reference) {
-  // Not yet finalized.
-  DCHECK_EQ(isolate_->heap()->empty_fixed_array(),
-            isolate_->heap()->builtins_constants_table());
-
-  auto maybe_entry = external_reference_map_.find(reference.address());
-  if (maybe_entry == external_reference_map_.end()) {
-    HandleScope handle_scope(isolate_);
-    Handle<Foreign> object =
-        isolate_->factory()->NewForeign(reference.address(), TENURED);
-    uint32_t index = AddObject(object);
-    external_reference_map_.emplace(reference.address(), index);
-    return index;
-  } else {
-    return maybe_entry->second;
-  }
-}
-
 void BuiltinsConstantsTableBuilder::Finalize() {
   HandleScope handle_scope(isolate_);
 

src/builtins/constants-table-builder.h

@@ -5,8 +5,6 @@
 #ifndef V8_BUILTINS_CONSTANTS_TABLE_BUILDER_H_
 #define V8_BUILTINS_CONSTANTS_TABLE_BUILDER_H_
 
-#include <unordered_map>
-
 #include "src/allocation.h"
 #include "src/base/macros.h"
 #include "src/handles.h"
@@ -30,10 +28,6 @@ class BuiltinsConstantsTableBuilder final {
   // object, possibly adding the object to the table. Objects are deduplicated.
   uint32_t AddObject(Handle<Object> object);
 
-  // External references can also be added, and end up as a Foreign object in
-  // the constants table.
-  uint32_t AddExternalReference(ExternalReference reference);
-
   // Should be called after all affected code (e.g. builtins and bytecode
   // handlers) has been generated.
   void Finalize();
@@ -45,10 +39,6 @@ class BuiltinsConstantsTableBuilder final {
   typedef IdentityMap<uint32_t, FreeStoreAllocationPolicy> ConstantsMap;
   ConstantsMap map_;
 
-  // Maps external references to corresponding indices within the constants
-  // list. Note that external references are also in map_ as Foreign objects.
-  std::unordered_map<Address, uint32_t> external_reference_map_;
-
   DISALLOW_COPY_AND_ASSIGN(BuiltinsConstantsTableBuilder)
 };
 

src/compiler/code-assembler.cc

@@ -21,6 +21,7 @@
 #include "src/machine-type.h"
 #include "src/macro-assembler.h"
 #include "src/objects-inl.h"
+#include "src/snapshot/serializer-common.h"
 #include "src/utils.h"
 #include "src/zone/zone.h"
 
@@ -261,37 +262,37 @@ TNode<HeapObject> CodeAssembler::LookupConstant(Handle<HeapObject> object) {
       Load(MachineType::AnyTagged(), builtins_constants_table, offset));
 }
 
-// External references are stored on the builtins constants list, wrapped in
-// Foreign objects.
-// TODO(jgruber,v8:6666): A more efficient solution.
+// External references are stored in the external reference table.
 TNode<ExternalReference> CodeAssembler::LookupExternalReference(
     ExternalReference reference) {
   DCHECK(isolate()->serializer_enabled());
 
-  // Ensure the given object is in the builtins constants table and fetch its
-  // index.
-  BuiltinsConstantsTableBuilder* builder =
-      isolate()->builtins_constants_table_builder();
-  uint32_t index = builder->AddExternalReference(reference);
+  // Encode as an index into the external reference table stored on the isolate.
 
-  // The builtins constants table is loaded through the root register on all
-  // supported platforms. This is checked by the
-  // VerifyBuiltinsIsolateIndependence cctest, which disallows embedded objects
-  // in isolate-independent builtins.
-  DCHECK(isolate()->heap()->RootCanBeTreatedAsConstant(
-      Heap::kBuiltinsConstantsTableRootIndex));
-  TNode<FixedArray> builtins_constants_table = UncheckedCast<FixedArray>(
-      LoadRoot(Heap::kBuiltinsConstantsTableRootIndex));
+  ExternalReferenceEncoder encoder(isolate());
+  ExternalReferenceEncoder::Value v = encoder.Encode(reference.address());
+  CHECK(!v.is_from_api());
+  uint32_t index = v.index();
 
-  // Generate the lookup.
-  const int32_t header_size = FixedArray::kHeaderSize - kHeapObjectTag;
-  TNode<IntPtrT> offset = IntPtrConstant(header_size + kPointerSize * index);
-  TNode<Object> foreign = UncheckedCast<HeapObject>(
-      Load(MachineType::AnyTagged(), builtins_constants_table, offset));
+  // Generate code to load from the external reference table.
+  // TODO(jgruber,v8:6666): If the external reference points within the isolate,
+  // we could return kRootPointer + offset without loading through the table.
+
+  TNode<IntPtrT> roots_ptr = LoadRootsPointer();
+
+  static const intptr_t roots_to_isolate_offset =
+#ifdef V8_TARGET_ARCH_X64
+      Isolate::offset_from_roots_to_external_reference_table() -
+      kRootRegisterBias;
+#else
+      Isolate::offset_from_roots_to_external_reference_table();
+#endif
 
+  Node* external_refs_ptr = Load(MachineType::Pointer(), roots_ptr,
+                                 IntPtrConstant(roots_to_isolate_offset));
   return UncheckedCast<ExternalReference>(
-      Load(MachineType::Pointer(), foreign,
-           IntPtrConstant(Foreign::kForeignAddressOffset - kHeapObjectTag)));
+      Load(MachineType::Pointer(), external_refs_ptr,
+           IntPtrConstant(ExternalReferenceTable::OffsetOfEntry(index))));
 }
 #endif  // V8_EMBEDDED_BUILTINS
 

src/external-reference-table.h

@@ -51,6 +51,12 @@ class ExternalReferenceTable {
 
   static const char* ResolveSymbol(void* address);
 
+  static uint32_t OffsetOfEntry(uint32_t i) {
+    // Used in CodeAssembler::LookupExternalReference.
+    STATIC_ASSERT(offsetof(ExternalReferenceEntry, address) == 0);
+    return i * sizeof(ExternalReferenceEntry);
+  }
+
  private:
   struct ExternalReferenceEntry {
     Address address;

src/isolate.cc

@@ -2586,8 +2586,6 @@ void Isolate::GlobalTearDown() {
 
 
 void Isolate::ClearSerializerData() {
-  delete external_reference_table_;
-  external_reference_table_ = nullptr;
   delete external_reference_map_;
   external_reference_map_ = nullptr;
 }
@@ -2675,6 +2673,9 @@ void Isolate::Deinit() {
   delete root_index_map_;
   root_index_map_ = nullptr;
 
+  delete external_reference_table_;
+  external_reference_table_ = nullptr;
+
   ClearSerializerData();
 }
 
@@ -2889,6 +2890,14 @@ void Isolate::PrepareEmbeddedBlobForSerialization() {
 
   CreateOffHeapTrampolines(this);
 }
+
+// static
+int32_t Isolate::offset_from_roots_to_external_reference_table() {
+  int32_t isolate_to_roots = OFFSET_OF(Isolate, heap_.roots_);
+  int32_t isolate_to_external_reference_table =
+      OFFSET_OF(Isolate, external_reference_table_);
+  return isolate_to_external_reference_table - isolate_to_roots;
+}
 #endif  // V8_EMBEDDED_BUILTINS
 
 bool Isolate::Init(StartupDeserializer* des) {
@@ -3026,6 +3035,9 @@ bool Isolate::Init(StartupDeserializer* des) {
 
   InitializeThreadLocal();
 
+  // Ensure that the external reference table has been allocated.
+  ExternalReferenceTable::instance(this);
+
   bootstrapper_->Initialize(create_heap_objects);
   setup_delegate_->SetupBuiltins(this);
   if (create_heap_objects) heap_.CreateFixedStubs();

src/isolate.h

@@ -1261,6 +1261,8 @@ class Isolate {
     return builtins_constants_table_builder_;
   }
 
+  static int32_t offset_from_roots_to_external_reference_table();
+
   const uint8_t* embedded_blob() const;
   uint32_t embedded_blob_size() const;
 #endif