Recognize special comparisons via pattern matching on the hydrogen graph.

src/hydrogen.cc

 // Copyright 2011 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 5772 matching lines @@
 
 Representation HGraphBuilder::ToRepresentation(TypeInfo info) {
   if (info.IsSmi()) return Representation::Integer32();
   if (info.IsInteger32()) return Representation::Integer32();
   if (info.IsDouble()) return Representation::Double();
   if (info.IsNumber()) return Representation::Double();
   return Representation::Tagged();
 }
 
 
 void HGraphBuilder::HandleLiteralCompareTypeof(CompareOperation* expr,

[Kevin Millikin (Chromium), 2011/10/13 13:11:59]

I'd prefer to use more specific types here.  The casts below will always succeed
in this change, but it needs nonlocal reasoning to see that (and relies on the
assertion to preserve it in the face of changes to the helper functions).

void HGraphBuilder::HandleLiteralCompareTypeof(CompareOperation* expr,
                                               HTypeof* typeof_expr,
                                               Handle<String> blah) { ...

[Sven Panne, 2011/10/13 14:21:38]

Done.

-                                               Expression* sub_expr,
-                                               Handle<String> check) {
-  CHECK_ALIVE(VisitForTypeOf(sub_expr));
-  HValue* value = Pop();
-  HTypeofIsAndBranch* instr = new(zone()) HTypeofIsAndBranch(value, check);
+                                               HValue* typeof_expr,
+                                               HValue* check) {
+  HValue* value = HTypeof::cast(typeof_expr)->value();
+  Handle<String> blah = Handle<String>::cast(HConstant::cast(check)->handle());

[Kevin Millikin (Chromium), 2011/10/13 13:11:59]

There must be a better name.

[Sven Panne, 2011/10/13 14:21:38]

hurz? (http://www.youtube.com/watch?v=RAx0P-8n5K4)
gnlpfth? (http://www.youtube.com/watch?v=olGad2Uhnwc) ;-)
Luckily, we don't need this variable anymore.

+  HTypeofIsAndBranch* instr = new(zone()) HTypeofIsAndBranch(value, blah);
   instr->set_position(expr->position());
-  return ast_context()->ReturnControl(instr, expr->id());
+  ast_context()->ReturnControl(instr, expr->id());
+  typeof_expr->DeleteAndReplaceWith(instr);

[Kevin Millikin (Chromium), 2011/10/13 13:11:59]

I don't think this is right.  instr is a control instruction, and I think we're
relying on having no uses for those.  It should be the case that there is no use
of typeof_expr, so you can replace it with NULL.

An alternative that I like a bit better, because it covers more cases, is to
make HTypeof::Canonicalize() return NULL if there are no uses (provided we
convince ourselves that typeof cannot have side effects).

[Sven Panne, 2011/10/13 14:21:38]

I'd like to go for the first alternative (replacing with NULL) for now, because
it is a more local change of the CL. I'll have a look at the Canonicalize()
alternative later and submit a different CL if it makes things clearer.

 }
 
 
-bool HGraphBuilder::TryLiteralCompare(CompareOperation* expr) {
-  Expression *sub_expr;
-  Handle<String> check;
-  if (expr->IsLiteralCompareTypeof(&sub_expr, &check)) {
-    HandleLiteralCompareTypeof(expr, sub_expr, check);
+static bool MatchLiteralCompareNil(HValue* left,
+                                   Token::Value op,
+                                   HValue* right,
+                                   Handle<Object> nil,
+                                   HValue** expr) {
+  if (left->IsConstant() &&
+      HConstant::cast(left)->handle().is_identical_to(nil) &&
+      Token::IsEqualityOp(op)) {
+    *expr = right;
     return true;
   }
+  return false;
+}
 
-  if (expr->IsLiteralCompareUndefined(&sub_expr)) {
-    HandleLiteralCompareNil(expr, sub_expr, kUndefinedValue);
+
+static bool MatchLiteralCompareTypeof(HValue* left,
+                                      Token::Value op,
+                                      HValue* right,
+                                      HValue** typeof_expr,
+                                      HValue** check) {
+  if (left->IsTypeof() &&
+      Token::IsEqualityOp(op) &&
+      right->IsConstant() &&
+      HConstant::cast(right)->HasStringValue()) {
+    *typeof_expr = left;

[Kevin Millikin (Chromium), 2011/10/13 13:11:59]

If you take my suggestion above, this is where you'd cast:

*typeof_expr = HTypeof::cast(left);
*check = Handle<String>::cast(HConstant::cast(right)->handle());

[Sven Panne, 2011/10/13 14:21:38]

Done.

+    *check = right;
     return true;
   }
+  return false;
+}
 
-  if (expr->IsLiteralCompareNull(&sub_expr)) {
-    HandleLiteralCompareNil(expr, sub_expr, kNullValue);
-    return true;
-  }
 
-  return false;
+static bool IsLiteralCompareTypeof(HValue* left,
+                                   Token::Value op,
+                                   HValue* right,
+                                   HValue** typeof_expr,
+                                   HValue** check) {
+  return MatchLiteralCompareTypeof(left, op, right, typeof_expr, check) ||
+      MatchLiteralCompareTypeof(right, op, left, typeof_expr, check);
+}
+
+
+static bool IsLiteralCompareNil(HValue* left,
+                                Token::Value op,
+                                HValue* right,
+                                Handle<Object> nil,
+                                HValue** expr) {
+  return MatchLiteralCompareNil(left, op, right, nil, expr) ||
+      MatchLiteralCompareNil(right, op, left, nil, expr);
 }
 
 
 void HGraphBuilder::VisitCompareOperation(CompareOperation* expr) {
   ASSERT(!HasStackOverflow());
   ASSERT(current_block() != NULL);
   ASSERT(current_block()->HasPredecessor());
   if (IsClassOfTest(expr)) {
     CallRuntime* call = expr->left()->AsCallRuntime();
     ASSERT(call->arguments()->length() == 1);
     CHECK_ALIVE(VisitForValue(call->arguments()->at(0)));
     HValue* value = Pop();
     Literal* literal = expr->right()->AsLiteral();
     Handle<String> rhs = Handle<String>::cast(literal->handle());
     HClassOfTestAndBranch* instr =
         new(zone()) HClassOfTestAndBranch(value, rhs);
     instr->set_position(expr->position());
     return ast_context()->ReturnControl(instr, expr->id());
   }
 
-  // Check for special cases that compare against literals.
-  if (TryLiteralCompare(expr)) return;
-
   TypeInfo type_info = oracle()->CompareType(expr);
   // Check if this expression was ever executed according to type feedback.
+  // Note that for the special typeof/null/undefined cases we get unknown here.
   if (type_info.IsUninitialized()) {
     AddInstruction(new(zone()) HSoftDeoptimize);
     current_block()->MarkAsDeoptimizing();
     type_info = TypeInfo::Unknown();
   }
 
   CHECK_ALIVE(VisitForValue(expr->left()));
   CHECK_ALIVE(VisitForValue(expr->right()));
 
   HValue* context = environment()->LookupContext();
   HValue* right = Pop();
   HValue* left = Pop();
   Token::Value op = expr->op();
 
+  HValue* typeof_expr = NULL;

[Kevin Millikin (Chromium), 2011/10/13 13:11:59]

And

HTypeof* typeof_expr = NULL;
Handle<String> check;

[Sven Panne, 2011/10/13 14:21:38]

Done.

+  HValue* check = NULL;
+  if (IsLiteralCompareTypeof(left, op, right, &typeof_expr, &check)) {
+    return HandleLiteralCompareTypeof(expr, typeof_expr, check);
+  }
+  HValue* sub_expr = NULL;
+  Factory* f = graph()->isolate()->factory();
+  if (IsLiteralCompareNil(left, op, right, f->undefined_value(), &sub_expr)) {
+    return HandleLiteralCompareNil(expr, sub_expr, kUndefinedValue);
+  }
+  if (IsLiteralCompareNil(left, op, right, f->null_value(), &sub_expr)) {
+    return HandleLiteralCompareNil(expr, sub_expr, kNullValue);
+  }
+
   if (op == Token::INSTANCEOF) {
     // Check to see if the rhs of the instanceof is a global function not
     // residing in new space. If it is we assume that the function will stay the
     // same.
     Handle<JSFunction> target = Handle<JSFunction>::null();
     VariableProxy* proxy = expr->right()->AsVariableProxy();
     bool global_function = (proxy != NULL) && proxy->var()->IsUnallocated();
     if (global_function &&
         info()->has_global_object() &&
         !info()->global_object()->IsAccessCheckNeeded()) {
@@ skipping 68 matching lines @@
           new(zone()) HCompareIDAndBranch(left, right, op);
       result->set_position(expr->position());
       result->SetInputRepresentation(r);
       return ast_context()->ReturnControl(result, expr->id());
     }
   }
 }
 
 
 void HGraphBuilder::HandleLiteralCompareNil(CompareOperation* expr,
-                                            Expression* sub_expr,
+                                            HValue* value,
                                             NilValue nil) {
   ASSERT(!HasStackOverflow());
   ASSERT(current_block() != NULL);
   ASSERT(current_block()->HasPredecessor());
-  CHECK_ALIVE(VisitForValue(sub_expr));
-  HValue* value = Pop();
   EqualityKind kind =
       expr->op() == Token::EQ_STRICT ? kStrictEquality : kNonStrictEquality;
   HIsNilAndBranch* instr = new(zone()) HIsNilAndBranch(value, kind, nil);
   instr->set_position(expr->position());
   return ast_context()->ReturnControl(instr, expr->id());
 }
 
 
 void HGraphBuilder::VisitThisFunction(ThisFunction* expr) {
   ASSERT(!HasStackOverflow());
@@ skipping 972 matching lines @@
     }
   }
 
 #ifdef DEBUG
   if (graph_ != NULL) graph_->Verify(false);  // No full verify.
   if (allocator_ != NULL) allocator_->Verify();
 #endif
 }
 
 } }  // namespace v8::internal