[turbofan] Use the heap broker in JSContextSpecialization.

No longer access the heap directly, as policed by Disallow*
scopes in JSContextSpecialization::Reduce.

Bug: v8:7790
Change-Id: I40f1c500b04b96152421fd5de631747ba386bca1
Reviewed-on: https://chromium-review.googlesource.com/1101322
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53759}

src/compiler/js-context-specialization.cc

@@ -17,6 +17,11 @@ namespace internal {
 namespace compiler {
 
 Reduction JSContextSpecialization::Reduce(Node* node) {
+  DisallowHeapAllocation no_heap_allocation;
+  DisallowHandleAllocation no_handle_allocation;
+  DisallowHandleDereference no_handle_dereference;
+  DisallowCodeDependencyChange no_dependency_change;
+
   switch (node->opcode()) {
     case IrOpcode::kParameter:
       return ReduceParameter(node);
@@ -99,12 +104,14 @@ bool IsContextParameter(Node* node) {
 // context (which we want to read from or store to), try to return a
 // specialization context.  If successful, update {distance} to whatever
 // distance remains from the specialization context.
-MaybeHandle<Context> GetSpecializationContext(Node* node, size_t* distance,
-                                              Maybe<OuterContext> maybe_outer) {
+base::Optional<ContextHeapReference> GetSpecializationContext(
+    const JSHeapBroker* broker, Node* node, size_t* distance,
+    Maybe<OuterContext> maybe_outer) {
   switch (node->opcode()) {
     case IrOpcode::kHeapConstant: {
-      Handle<Object> object = HeapConstantOf(node->op());
-      if (object->IsContext()) return Handle<Context>::cast(object);
+      HeapReference object =
+          broker->HeapReferenceForObject(HeapConstantOf(node->op()));
+      if (object.IsContext()) return object.AsContext();
       break;
     }
     case IrOpcode::kParameter: {
@@ -112,14 +119,14 @@ MaybeHandle<Context> GetSpecializationContext(Node* node, size_t* distance,
       if (maybe_outer.To(&outer) && IsContextParameter(node) &&
           *distance >= outer.distance) {
         *distance -= outer.distance;
-        return outer.context;
+        return broker->HeapReferenceForObject(outer.context).AsContext();
       }
       break;
     }
     default:
       break;
   }
-  return MaybeHandle<Context>();
+  return base::Optional<ContextHeapReference>();
 }
 
 }  // anonymous namespace
@@ -133,39 +140,57 @@ Reduction JSContextSpecialization::ReduceJSLoadContext(Node* node) {
   // First walk up the context chain in the graph as far as possible.
   Node* context = NodeProperties::GetOuterContext(node, &depth);
 
-  Handle<Context> concrete;
-  if (!GetSpecializationContext(context, &depth, outer()).ToHandle(&concrete)) {
+  base::Optional<ContextHeapReference> maybe_concrete =
+      GetSpecializationContext(js_heap_broker(), context, &depth, outer());
+  if (!maybe_concrete.has_value()) {
     // We do not have a concrete context object, so we can only partially reduce
     // the load by folding-in the outer context node.
     return SimplifyJSLoadContext(node, context, depth);
   }
 
   // Now walk up the concrete context chain for the remaining depth.
+  ContextHeapReference concrete = maybe_concrete.value();
   for (; depth > 0; --depth) {
-    concrete = handle(concrete->previous(), isolate());
+    concrete = concrete.previous(js_heap_broker()).value();
   }
 
   if (!access.immutable()) {
     // We found the requested context object but since the context slot is
     // mutable we can only partially reduce the load.
-    return SimplifyJSLoadContext(node, jsgraph()->Constant(concrete), depth);
+    return SimplifyJSLoadContext(node, jsgraph()->Constant(concrete.object()),
+                                 depth);
   }
 
-  // Even though the context slot is immutable, the context might have escaped
-  // before the function to which it belongs has initialized the slot.
-  // We must be conservative and check if the value in the slot is currently
-  // the hole or undefined. Only if it is neither of these, can we be sure that
-  // it won't change anymore.
-  Handle<Object> value(concrete->get(static_cast<int>(access.index())),
-                       isolate());
-  if (value->IsUndefined(isolate()) || value->IsTheHole(isolate())) {
-    return SimplifyJSLoadContext(node, jsgraph()->Constant(concrete), depth);
+  // This will hold the final value, if we can figure it out.
+  base::Optional<ObjectReference> maybe_value;
+
+  maybe_value =
+      concrete.get(js_heap_broker(), static_cast<int>(access.index()));
+  if (maybe_value.has_value() && !maybe_value->IsSmi()) {
+    // Even though the context slot is immutable, the context might have escaped
+    // before the function to which it belongs has initialized the slot.
+    // We must be conservative and check if the value in the slot is currently
+    // the hole or undefined. Only if it is neither of these, can we be sure
+    // that it won't change anymore.
+    HeapReferenceType type =
+        maybe_value->AsHeapReference().type(js_heap_broker());
+    if (type.oddball_type() == HeapReferenceType::kAny ||
+        type.oddball_type() == HeapReferenceType::kUndefined ||
+        type.oddball_type() == HeapReferenceType::kHole) {
+      maybe_value.reset();
+    }
+  }
+
+  if (!maybe_value.has_value()) {
+    return SimplifyJSLoadContext(node, jsgraph()->Constant(concrete.object()),
+                                 depth);
   }
 
   // Success. The context load can be replaced with the constant.
-  // TODO(titzer): record the specialization for sharing code across multiple
-  // contexts that have the same value in the corresponding context slot.
-  Node* constant = jsgraph_->Constant(value);
+  // TODO(titzer): record the specialization for sharing code across
+  // multiple contexts that have the same value in the corresponding context
+  // slot.
+  Node* constant = jsgraph_->Constant(maybe_value->object());
   ReplaceWithValue(node, constant);
   return Replace(constant);
 }
@@ -181,19 +206,22 @@ Reduction JSContextSpecialization::ReduceJSStoreContext(Node* node) {
   // or hit a node that does not have a CreateXYZContext operator.
   Node* context = NodeProperties::GetOuterContext(node, &depth);
 
-  Handle<Context> concrete;
-  if (!GetSpecializationContext(context, &depth, outer()).ToHandle(&concrete)) {
+  base::Optional<ContextHeapReference> maybe_concrete =
+      GetSpecializationContext(js_heap_broker(), context, &depth, outer());
+  if (!maybe_concrete.has_value()) {
     // We do not have a concrete context object, so we can only partially reduce
     // the load by folding-in the outer context node.
     return SimplifyJSStoreContext(node, context, depth);
   }
 
   // Now walk up the concrete context chain for the remaining depth.
+  ContextHeapReference concrete = maybe_concrete.value();
   for (; depth > 0; --depth) {
-    concrete = handle(concrete->previous(), isolate());
+    concrete = concrete.previous(js_heap_broker()).value();
   }
 
-  return SimplifyJSStoreContext(node, jsgraph()->Constant(concrete), depth);
+  return SimplifyJSStoreContext(node, jsgraph()->Constant(concrete.object()),
+                                depth);
 }
 
 

src/compiler/js-context-specialization.h

@@ -33,12 +33,14 @@ struct OuterContext {
 class JSContextSpecialization final : public AdvancedReducer {
  public:
   JSContextSpecialization(Editor* editor, JSGraph* jsgraph,
+                          JSHeapBroker* js_heap_broker,
                           Maybe<OuterContext> outer,
                           MaybeHandle<JSFunction> closure)
       : AdvancedReducer(editor),
         jsgraph_(jsgraph),
         outer_(outer),
-        closure_(closure) {}
+        closure_(closure),
+        js_heap_broker_(js_heap_broker) {}
 
   const char* reducer_name() const override {
     return "JSContextSpecialization";
@@ -60,10 +62,12 @@ class JSContextSpecialization final : public AdvancedReducer {
   JSGraph* jsgraph() const { return jsgraph_; }
   Maybe<OuterContext> outer() const { return outer_; }
   MaybeHandle<JSFunction> closure() const { return closure_; }
+  const JSHeapBroker* js_heap_broker() const { return js_heap_broker_; }
 
   JSGraph* const jsgraph_;
   Maybe<OuterContext> outer_;
   MaybeHandle<JSFunction> closure_;
+  const JSHeapBroker* const js_heap_broker_;
 
   DISALLOW_COPY_AND_ASSIGN(JSContextSpecialization);
 };

src/compiler/js-graph.cc

@@ -47,6 +47,7 @@ Node* JSGraph::CEntryStubConstant(int result_size, SaveFPRegsMode save_doubles,
 }
 
 Node* JSGraph::Constant(Handle<Object> value) {
+  AllowHandleDereference handle_dereference;
   // Dereference the handle to determine if a number constant or other
   // canonicalized node can be used.
   if (value->IsNumber()) {

src/compiler/js-heap-broker.cc

@@ -9,12 +9,46 @@ namespace v8 {
 namespace internal {
 namespace compiler {
 
+bool ObjectReference::IsSmi() const {
+  AllowHandleDereference allow_handle_dereference;
+  return object_->IsSmi();
+}
+
+int ObjectReference::AsSmi() const {
+  AllowHandleDereference allow_handle_dereference;
+  return Smi::cast(*object_)->value();
+}
+
+HeapReference ObjectReference::AsHeapReference() const {
+  AllowHandleDereference allow_handle_dereference;
+  return HeapReference(Handle<HeapObject>::cast(object_));
+}
+
+base::Optional<ContextHeapReference> ContextHeapReference::previous(
+    const JSHeapBroker* broker) const {
+  AllowHandleAllocation handle_allocation;
+  AllowHandleDereference handle_dereference;
+  Context* previous = Handle<Context>::cast(object())->previous();
+  if (previous == nullptr) return base::Optional<ContextHeapReference>();
+  return broker->HeapReferenceForObject(handle(previous, broker->isolate()))
+      .AsContext();
+}
+
+base::Optional<ObjectReference> ContextHeapReference::get(
+    const JSHeapBroker* broker, int index) const {
+  AllowHandleAllocation handle_allocation;
+  AllowHandleDereference handle_dereference;
+  Handle<Object> value(Handle<Context>::cast(object())->get(index),
+                       broker->isolate());
+  return ObjectReference(value);
+}
+
 JSHeapBroker::JSHeapBroker(Isolate* isolate) : isolate_(isolate) {}
 
 HeapReferenceType JSHeapBroker::HeapReferenceTypeFromMap(Map* map) const {
   AllowHandleDereference allow_handle_dereference;
   Heap* heap = isolate_->heap();
-  HeapReferenceType::OddballType oddball_type = HeapReferenceType::kUnknown;
+  HeapReferenceType::OddballType oddball_type = HeapReferenceType::kNone;
   if (map->instance_type() == ODDBALL_TYPE) {
     if (map == heap->undefined_map()) {
       oddball_type = HeapReferenceType::kUndefined;
@@ -25,6 +59,7 @@ HeapReferenceType JSHeapBroker::HeapReferenceTypeFromMap(Map* map) const {
     } else if (map == heap->the_hole_map()) {
       oddball_type = HeapReferenceType::kHole;
     } else {
+      oddball_type = HeapReferenceType::kOther;
       DCHECK(map == heap->uninitialized_map() ||
              map == heap->termination_exception_map() ||
              map == heap->arguments_marker_map() ||

src/compiler/js-heap-broker.h

@@ -16,16 +16,26 @@ namespace compiler {
 
 class HeapReferenceType {
  public:
-  enum OddballType : uint8_t { kUnknown, kBoolean, kUndefined, kNull, kHole };
+  enum OddballType : uint8_t {
+    kNone,     // Not an Oddball.
+    kBoolean,  // True or False.
+    kUndefined,
+    kNull,
+    kHole,
+    kOther,  // Oddball, but none of the above.
+    kAny     // Any Oddball.
+  };
   enum Flag : uint8_t { kUndetectable = 1 << 0, kCallable = 1 << 1 };
 
   typedef base::Flags<Flag> Flags;
 
   HeapReferenceType(InstanceType instance_type, Flags flags,
-                    OddballType oddball_type = kUnknown)
+                    OddballType oddball_type)
       : instance_type_(instance_type),
         oddball_type_(oddball_type),
-        flags_(flags) {}
+        flags_(flags) {
+    DCHECK_EQ(instance_type == ODDBALL_TYPE, oddball_type != kNone);
+  }
 
   OddballType oddball_type() const { return oddball_type_; }
   InstanceType instance_type() const { return instance_type_; }
@@ -41,6 +51,7 @@ class HeapReferenceType {
 };
 
 #define HEAP_BROKER_DATA_LIST(V) \
+  V(Context)                     \
   V(JSFunction)                  \
   V(Number)
 
@@ -54,6 +65,20 @@ HEAP_BROKER_DATA_LIST(FORWARD_DECL)
 #undef FORWARD_DECL
 
 class JSHeapBroker;
+class HeapReference;
+
+class ObjectReference {
+ public:
+  explicit ObjectReference(Handle<Object> object) : object_(object) {}
+
+  Handle<Object> object() const { return object_; }
+  bool IsSmi() const;
+  int AsSmi() const;
+  HeapReference AsHeapReference() const;
+
+ private:
+  Handle<Object> object_;
+};
 
 class HeapReference {
  public:
@@ -72,7 +97,7 @@ class HeapReference {
 
  private:
   friend class JSHeapBroker;
-  Handle<HeapObject> const object_;
+  Handle<HeapObject> object_;
 };
 
 class V8_EXPORT_PRIVATE JSHeapBroker : public NON_EXPORTED_BASE(ZoneObject) {
@@ -87,12 +112,13 @@ class V8_EXPORT_PRIVATE JSHeapBroker : public NON_EXPORTED_BASE(ZoneObject) {
 
   static base::Optional<int> TryGetSmi(Handle<Object> object);
 
+  Isolate* isolate() const { return isolate_; }
+
  private:
   friend class HeapReference;
   HeapReferenceType HeapReferenceTypeFromMap(Map* map) const;
 
   Isolate* const isolate_;
-  Isolate* isolate() const { return isolate_; }
 };
 
 class JSFunctionHeapReference : public HeapReference {
@@ -110,6 +136,16 @@ class NumberHeapReference : public HeapReference {
   double value() const;
 };
 
+class ContextHeapReference : public HeapReference {
+ public:
+  explicit ContextHeapReference(Handle<HeapObject> object)
+      : HeapReference(object) {}
+  base::Optional<ContextHeapReference> previous(
+      const JSHeapBroker* broker) const;
+  base::Optional<ObjectReference> get(const JSHeapBroker* broker,
+                                      int index) const;
+};
+
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8

src/compiler/pipeline.cc

@@ -1163,7 +1163,7 @@ struct InliningPhase {
             : JSCallReducer::kNoFlags,
         data->native_context(), data->info()->dependencies());
     JSContextSpecialization context_specialization(
-        &graph_reducer, data->jsgraph(),
+        &graph_reducer, data->jsgraph(), data->js_heap_broker(),
         ChooseSpecializationContext(isolate, data->info()),
         data->info()->is_function_context_specializing()
             ? data->info()->closure()

src/compiler/types.cc

@@ -162,7 +162,7 @@ Type::bitset BitsetType::Lub(HeapReferenceType const& type) {
       return kSymbol;
     case BIGINT_TYPE:
       return kBigInt;
-    case ODDBALL_TYPE: {
+    case ODDBALL_TYPE:
       switch (type.oddball_type()) {
         case HeapReferenceType::kHole:
           return kHole;
@@ -172,11 +172,14 @@ Type::bitset BitsetType::Lub(HeapReferenceType const& type) {
           return kNull;
         case HeapReferenceType::kUndefined:
           return kUndefined;
-        case HeapReferenceType::kUnknown:
+        case HeapReferenceType::kOther:
+          // TODO(neis): We should add a kOtherOddball type.
           return kOtherInternal;
+        case HeapReferenceType::kAny:
+          return kOddball | kOtherInternal;
+        default:
+          UNREACHABLE();
       }
-      UNREACHABLE();
-    }
     case HEAP_NUMBER_TYPE:
       return kNumber;
     case JS_OBJECT_TYPE:

test/cctest/compiler/test-js-context-specialization.cc

@@ -30,7 +30,9 @@ class ContextSpecializationTester : public HandleAndZoneScope {
         jsgraph_(main_isolate(), graph(), common(), &javascript_, &simplified_,
                  &machine_),
         reducer_(main_zone(), graph()),
-        spec_(&reducer_, jsgraph(), context, MaybeHandle<JSFunction>()) {}
+        js_heap_broker_(main_isolate()),
+        spec_(&reducer_, jsgraph(), &js_heap_broker_, context,
+              MaybeHandle<JSFunction>()) {}
 
   JSContextSpecialization* spec() { return &spec_; }
   Factory* factory() { return main_isolate()->factory(); }
@@ -55,6 +57,7 @@ class ContextSpecializationTester : public HandleAndZoneScope {
   SimplifiedOperatorBuilder simplified_;
   JSGraph jsgraph_;
   GraphReducer reducer_;
+  JSHeapBroker js_heap_broker_;
   JSContextSpecialization spec_;
 };