Inline loading of immutable properties

src/hydrogen.cc

Index: src/hydrogen.cc
diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index 6587ce97199ca944f8daaa8938bcbf73b23f6414..36ceb32b43c0b09cafc5708f1f88ba60417b8d4a 100644
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -6716,6 +6716,31 @@ void HOptimizedGraphBuilder::BuildLoad(Property* expr,
     Handle<String> name = expr->key()->AsLiteral()->AsPropertyName();
     HValue* object = Pop();
 
+    // See if the property to be loaded is non-configurable and non-writable
+    if (object->IsConstant() &&
+        HConstant::cast(object)->handle(isolate())->IsJSObject()) {
+      Handle<JSObject> js_object =
+          Handle<JSObject>::cast(HConstant::cast(object)->handle(isolate()));
+
+      if (!js_object->map()->is_observed()) {
+        LookupResult lookup(isolate());
+        js_object->LocalLookup(*name, &lookup, false);
+
+        if (lookup.IsFound() && lookup.IsCacheable() &&
+            (js_object->map()->is_frozen() ||

[Toon Verwaest, 2014/04/02 14:51:47]

You don't need to special-case is_frozen(), such properties will be read-only
non-deleteable.

+            (lookup.IsReadOnly() && lookup.IsDontDelete()))) {
+          Handle<Object> value = handle(lookup.GetLazyValue(), isolate());
+
+          if (!value.is_null() &&
+              !value->IsTheHole() &&
+              !value->IsCallable()) {

[Toon Verwaest, 2014/04/02 14:51:47]

Why these cases? Can you introduce abstractions that make it clear what these
cases mean?

[p.antonov, 2014/04/02 15:53:58]

The null check should actually be unnecessary (since the lookup is found?).

About the hole check: I took a look at the different lookup types and it looked
like Normal, Field and Constant were the ones that can be applied in this
situation. I didn't find a method that checks for all of those but I noticed
GetLazyValue returns TheHole if the lookup type is not a normal, a field or a
constant. Should I make a method that checks of the lookup type is one of these
three types?

The callable check is there because if the value is a function, then inlining
its load and calling it causes crash. I think this is because of some assumption
is being violated somewhere but I don't know at this time what it is. Should I
add a fixme comment?

+            instr = New<HConstant>(value);
+            return ast_context()->ReturnInstruction(instr, ast_id);
+          }
+        }
+      }
+    }
+
     instr = BuildNamedAccess(LOAD, ast_id, expr->LoadId(), expr,
                              object, name, NULL, expr->IsUninitialized());
     if (instr == NULL) return;