Recognize special comparisons via pattern matching on the hydrogen graph.

src/hydrogen.cc

Index: src/hydrogen.cc
===================================================================
--- src/hydrogen.cc	(revision 9587)
+++ src/hydrogen.cc	(working copy)
@@ -5791,38 +5791,69 @@
 
 
 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);
-    return true;
-  }
 
-  if (expr->IsLiteralCompareNull(&sub_expr)) {
-    HandleLiteralCompareNil(expr, sub_expr, kNullValue);
+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;
 }
 
 
+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);
@@ -5840,11 +5871,9 @@
     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();
@@ -5859,6 +5888,20 @@
   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
@@ -5947,13 +5990,11 @@
 
 
 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);