Nuke CompareToNull AST node.

The Great Master Plan is to move the recognition of special cases for
comparisons further down the compilation pipeline where more information is
available. This is a first step into this direction: The special handling of
equality comparisons involving null is pushed from the parser to the code
generators, removing the need for a special AST node. (There are rumors from
usually well-informed sources that this node type is actually a relic of ancient
crankshaft days...)

The next steps will be the unification of null/undefined handling and pushing
the special case handling in crankshaft even further down the pipeline, enabling
the recognition of cases like "var foo=null; if (foo === bar) ...", but these
will be in separate CLs.
Review URL: http://codereview.chromium.org/7887037

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@9293 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/arm/full-codegen-arm.cc

@@ -4085,20 +4085,16 @@ void FullCodeGenerator::VisitCompareOperation(CompareOperation* expr) {
 }
 
 
-void FullCodeGenerator::VisitCompareToNull(CompareToNull* expr) {
-  Comment cmnt(masm_, "[ CompareToNull");
-  Label materialize_true, materialize_false;
-  Label* if_true = NULL;
-  Label* if_false = NULL;
-  Label* fall_through = NULL;
-  context()->PrepareTest(&materialize_true, &materialize_false,
-                         &if_true, &if_false, &fall_through);
-
-  VisitForAccumulatorValue(expr->expression());
+void FullCodeGenerator::EmitLiteralCompareNull(Expression* expr,
+                                               bool is_strict,
+                                               Label* if_true,
+                                               Label* if_false,
+                                               Label* fall_through) {
+  VisitForAccumulatorValue(expr);
   PrepareForBailoutBeforeSplit(TOS_REG, true, if_true, if_false);
   __ LoadRoot(r1, Heap::kNullValueRootIndex);
   __ cmp(r0, r1);
-  if (expr->is_strict()) {
+  if (is_strict) {
     Split(eq, if_true, if_false, fall_through);
   } else {
     __ b(eq, if_true);
@@ -4113,7 +4109,6 @@ void FullCodeGenerator::VisitCompareToNull(CompareToNull* expr) {
     __ cmp(r1, Operand(1 << Map::kIsUndetectable));
     Split(eq, if_true, if_false, fall_through);
   }
-  context()->Plug(if_true, if_false);
 }
 
 

src/ast.cc

@@ -380,6 +380,27 @@ bool CompareOperation::IsLiteralCompareUndefined(Expression** expr) {
 }
 
 
+bool CompareOperation::IsLiteralCompareNull(Expression** expr) {
+  if (op_ != Token::EQ && op_ != Token::EQ_STRICT) return false;
+
+  // Check for the pattern: <expression> equals null.
+  Literal* right_literal = right_->AsLiteral();
+  if (right_literal != NULL && right_literal->handle()->IsNull()) {
+    *expr = left_;
+    return true;
+  }
+
+  // Check for the pattern: null equals <expression>.
+  Literal* left_literal = left_->AsLiteral();
+  if (left_literal != NULL && left_literal->handle()->IsNull()) {
+    *expr = right_;
+    return true;
+  }
+
+  return false;
+}
+
+
 // ----------------------------------------------------------------------------
 // Inlining support
 
@@ -598,11 +619,6 @@ bool CompareOperation::IsInlineable() const {
 }
 
 
-bool CompareToNull::IsInlineable() const {
-  return expression()->IsInlineable();
-}
-
-
 bool CountOperation::IsInlineable() const {
   return expression()->IsInlineable();
 }

src/ast.h

@@ -90,7 +90,6 @@ namespace internal {
   V(CountOperation)                             \
   V(BinaryOperation)                            \
   V(CompareOperation)                           \
-  V(CompareToNull)                              \
   V(ThisFunction)
 
 #define AST_NODE_LIST(V)                        \
@@ -1465,6 +1464,7 @@ class CompareOperation: public Expression {
   // Match special cases.
   bool IsLiteralCompareTypeof(Expression** expr, Handle<String>* check);
   bool IsLiteralCompareUndefined(Expression** expr);
+  bool IsLiteralCompareNull(Expression** expr);
 
  private:
   Token::Value op_;
@@ -1477,25 +1477,6 @@ class CompareOperation: public Expression {
 };
 
 
-class CompareToNull: public Expression {
- public:
-  CompareToNull(Isolate* isolate, bool is_strict, Expression* expression)
-      : Expression(isolate), is_strict_(is_strict), expression_(expression) { }
-
-  DECLARE_NODE_TYPE(CompareToNull)
-
-  virtual bool IsInlineable() const;
-
-  bool is_strict() const { return is_strict_; }
-  Token::Value op() const { return is_strict_ ? Token::EQ_STRICT : Token::EQ; }
-  Expression* expression() const { return expression_; }
-
- private:
-  bool is_strict_;
-  Expression* expression_;
-};
-
-
 class Conditional: public Expression {
  public:
   Conditional(Isolate* isolate,

src/full-codegen.cc

@@ -244,11 +244,6 @@ void BreakableStatementChecker::VisitBinaryOperation(BinaryOperation* expr) {
 }
 
 
-void BreakableStatementChecker::VisitCompareToNull(CompareToNull* expr) {
-  Visit(expr->expression());
-}
-
-
 void BreakableStatementChecker::VisitCompareOperation(CompareOperation* expr) {
   Visit(expr->left());
   Visit(expr->right());
@@ -1325,6 +1320,7 @@ bool FullCodeGenerator::TryLiteralCompare(CompareOperation* compare,
                                           Label* if_true,
                                           Label* if_false,
                                           Label* fall_through) {
+  bool is_strict = compare->op() == Token::EQ_STRICT;
   Expression *expr;
   Handle<String> check;
   if (compare->IsLiteralCompareTypeof(&expr, &check)) {
@@ -1337,6 +1333,11 @@ bool FullCodeGenerator::TryLiteralCompare(CompareOperation* compare,
     return true;
   }
 
+  if (compare->IsLiteralCompareNull(&expr)) {
+    EmitLiteralCompareNull(expr, is_strict, if_true, if_false, fall_through);
+    return true;
+  }
+
   return false;
 }
 

src/full-codegen.h

@@ -411,6 +411,12 @@ class FullCodeGenerator: public AstVisitor {
                                    Label* if_false,
                                    Label* fall_through);
 
+  // Platform-specific code for equality comparison with the null value.
+  void EmitLiteralCompareNull(Expression* expr,
+                              bool is_strict,
+                              Label* if_true,
+                              Label* if_false,
+                              Label* fall_through);
   // Bailout support.
   void PrepareForBailout(Expression* node, State state);
   void PrepareForBailoutForId(int id, State state);

src/hydrogen.cc

@@ -5721,6 +5721,11 @@ void HGraphBuilder::VisitCompareOperation(CompareOperation* expr) {
     return;
   }
 
+  if (expr->IsLiteralCompareNull(&sub_expr)) {
+    HandleLiteralCompareNull(expr, sub_expr);
+    return;
+  }
+
   TypeInfo type_info = oracle()->CompareType(expr);
   // Check if this expression was ever executed according to type feedback.
   if (type_info.IsUninitialized()) {
@@ -5824,15 +5829,16 @@ void HGraphBuilder::VisitCompareOperation(CompareOperation* expr) {
 }
 
 
-void HGraphBuilder::VisitCompareToNull(CompareToNull* expr) {
+void HGraphBuilder::HandleLiteralCompareNull(CompareOperation* compare_expr,
+                                             Expression* expr) {
   ASSERT(!HasStackOverflow());
   ASSERT(current_block() != NULL);
   ASSERT(current_block()->HasPredecessor());
-  CHECK_ALIVE(VisitForValue(expr->expression()));
+  CHECK_ALIVE(VisitForValue(expr));
   HValue* value = Pop();
-  HIsNullAndBranch* instr =
-      new(zone()) HIsNullAndBranch(value, expr->is_strict());
-  return ast_context()->ReturnControl(instr, expr->id());
+  bool is_strict = compare_expr->op() == Token::EQ_STRICT;
+  HIsNullAndBranch* instr = new(zone()) HIsNullAndBranch(value, is_strict);
+  return ast_context()->ReturnControl(instr, compare_expr->id());
 }
 
 

src/hydrogen.h

@@ -915,6 +915,8 @@ class HGraphBuilder: public AstVisitor {
                                   Handle<String> check);
   void HandleLiteralCompareUndefined(CompareOperation* compare_expr,
                                      Expression* expr);
+  void HandleLiteralCompareNull(CompareOperation* compare_expr,
+                                Expression* expr);
 
   HStringCharCodeAt* BuildStringCharCodeAt(HValue* context,
                                            HValue* string,

src/ia32/full-codegen-ia32.cc

@@ -4146,19 +4146,15 @@ void FullCodeGenerator::VisitCompareOperation(CompareOperation* expr) {
 }
 
 
-void FullCodeGenerator::VisitCompareToNull(CompareToNull* expr) {
-  Label materialize_true, materialize_false;
-  Label* if_true = NULL;
-  Label* if_false = NULL;
-  Label* fall_through = NULL;
-  context()->PrepareTest(&materialize_true, &materialize_false,
-                         &if_true, &if_false, &fall_through);
-
-  VisitForAccumulatorValue(expr->expression());
+void FullCodeGenerator::EmitLiteralCompareNull(Expression* expr,
+                                               bool is_strict,
+                                               Label* if_true,
+                                               Label* if_false,
+                                               Label* fall_through) {
+  VisitForAccumulatorValue(expr);
   PrepareForBailoutBeforeSplit(TOS_REG, true, if_true, if_false);
-
   __ cmp(eax, isolate()->factory()->null_value());
-  if (expr->is_strict()) {
+  if (is_strict) {
     Split(equal, if_true, if_false, fall_through);
   } else {
     __ j(equal, if_true);
@@ -4171,7 +4167,6 @@ void FullCodeGenerator::VisitCompareToNull(CompareToNull* expr) {
     __ test(edx, Immediate(1 << Map::kIsUndetectable));
     Split(not_zero, if_true, if_false, fall_through);
   }
-  context()->Plug(if_true, if_false);
 }
 
 

src/mips/full-codegen-mips.cc

@@ -4109,20 +4109,16 @@ void FullCodeGenerator::VisitCompareOperation(CompareOperation* expr) {
 }
 
 
-void FullCodeGenerator::VisitCompareToNull(CompareToNull* expr) {
-  Comment cmnt(masm_, "[ CompareToNull");
-  Label materialize_true, materialize_false;
-  Label* if_true = NULL;
-  Label* if_false = NULL;
-  Label* fall_through = NULL;
-  context()->PrepareTest(&materialize_true, &materialize_false,
-                         &if_true, &if_false, &fall_through);
-
-  VisitForAccumulatorValue(expr->expression());
+void FullCodeGenerator::EmitLiteralCompareNull(Expression* expr,
+                                               bool is_strict,
+                                               Label* if_true,
+                                               Label* if_false,
+                                               Label* fall_through) {
+  VisitForAccumulatorValue(expr);
   PrepareForBailoutBeforeSplit(TOS_REG, true, if_true, if_false);
   __ mov(a0, result_register());
   __ LoadRoot(a1, Heap::kNullValueRootIndex);
-  if (expr->is_strict()) {
+  if (is_strict) {
     Split(eq, a0, Operand(a1), if_true, if_false, fall_through);
   } else {
     __ Branch(if_true, eq, a0, Operand(a1));
@@ -4136,7 +4132,6 @@ void FullCodeGenerator::VisitCompareToNull(CompareToNull* expr) {
     __ And(a1, a1, Operand(1 << Map::kIsUndetectable));
     Split(ne, a1, Operand(zero_reg), if_true, if_false, fall_through);
   }
-  context()->Plug(if_true, if_false);
 }
 
 

src/parser.cc

@@ -2614,7 +2614,7 @@ Expression* Parser::ParseBinaryExpression(int prec, bool accept_IN, bool* ok) {
           case Token::NE_STRICT: cmp = Token::EQ_STRICT; break;
           default: break;
         }
-        x = NewCompareNode(cmp, x, y, position);
+        x = new(zone()) CompareOperation(isolate(), cmp, x, y, position);
         if (cmp != op) {
           // The comparison was negated - add a NOT.
           x = new(zone()) UnaryOperation(isolate(), Token::NOT, x, position);
@@ -2630,27 +2630,6 @@ Expression* Parser::ParseBinaryExpression(int prec, bool accept_IN, bool* ok) {
 }
 
 
-Expression* Parser::NewCompareNode(Token::Value op,
-                                   Expression* x,
-                                   Expression* y,
-                                   int position) {
-  ASSERT(op != Token::NE && op != Token::NE_STRICT);
-  if (op == Token::EQ || op == Token::EQ_STRICT) {
-    bool is_strict = (op == Token::EQ_STRICT);
-    Literal* x_literal = x->AsLiteral();
-    if (x_literal != NULL && x_literal->IsNull()) {
-      return new(zone()) CompareToNull(isolate(), is_strict, y);
-    }
-
-    Literal* y_literal = y->AsLiteral();
-    if (y_literal != NULL && y_literal->IsNull()) {
-      return new(zone()) CompareToNull(isolate(), is_strict, x);
-    }
-  }
-  return new(zone()) CompareOperation(isolate(), op, x, y, position);
-}
-
-
 Expression* Parser::ParseUnaryExpression(bool* ok) {
   // UnaryExpression ::
   //   PostfixExpression

src/parser.h

@@ -533,11 +533,6 @@ class Parser {
   ObjectLiteral::Property* ParseObjectLiteralGetSet(bool is_getter, bool* ok);
   Expression* ParseRegExpLiteral(bool seen_equal, bool* ok);
 
-  Expression* NewCompareNode(Token::Value op,
-                             Expression* x,
-                             Expression* y,
-                             int position);
-
   // Populate the constant properties fixed array for a materialized object
   // literal.
   void BuildObjectLiteralConstantProperties(

src/prettyprinter.cc

@@ -372,13 +372,6 @@ void PrettyPrinter::VisitCompareOperation(CompareOperation* node) {
 }
 
 
-void PrettyPrinter::VisitCompareToNull(CompareToNull* node) {
-  Print("(");
-  Visit(node->expression());
-  Print("%s null)", Token::String(node->op()));
-}
-
-
 void PrettyPrinter::VisitThisFunction(ThisFunction* node) {
   Print("<this-function>");
 }
@@ -1020,15 +1013,6 @@ void AstPrinter::VisitCompareOperation(CompareOperation* node) {
 }
 
 
-void AstPrinter::VisitCompareToNull(CompareToNull* node) {
-  const char* name = node->is_strict()
-      ? "COMPARE-TO-NULL-STRICT"
-      : "COMPARE-TO-NULL";
-  IndentedScope indent(this, name, node);
-  Visit(node->expression());
-}
-
-
 void AstPrinter::VisitThisFunction(ThisFunction* node) {
   IndentedScope indent(this, "THIS-FUNCTION");
 }
@@ -1404,16 +1388,6 @@ void JsonAstBuilder::VisitCompareOperation(CompareOperation* expr) {
 }
 
 
-void JsonAstBuilder::VisitCompareToNull(CompareToNull* expr) {
-  TagScope tag(this, "CompareToNull");
-  {
-    AttributesScope attributes(this);
-    AddAttribute("is_strict", expr->is_strict());
-  }
-  Visit(expr->expression());
-}
-
-
 void JsonAstBuilder::VisitThisFunction(ThisFunction* expr) {
   TagScope tag(this, "ThisFunction");
 }

src/x64/full-codegen-x64.cc

@@ -4018,19 +4018,15 @@ void FullCodeGenerator::VisitCompareOperation(CompareOperation* expr) {
 }
 
 
-void FullCodeGenerator::VisitCompareToNull(CompareToNull* expr) {
-  Comment cmnt(masm_, "[ CompareToNull");
-  Label materialize_true, materialize_false;
-  Label* if_true = NULL;
-  Label* if_false = NULL;
-  Label* fall_through = NULL;
-  context()->PrepareTest(&materialize_true, &materialize_false,
-                         &if_true, &if_false, &fall_through);
-
-  VisitForAccumulatorValue(expr->expression());
+void FullCodeGenerator::EmitLiteralCompareNull(Expression* expr,
+                                               bool is_strict,
+                                               Label* if_true,
+                                               Label* if_false,
+                                               Label* fall_through) {
+  VisitForAccumulatorValue(expr);
   PrepareForBailoutBeforeSplit(TOS_REG, true, if_true, if_false);
   __ CompareRoot(rax, Heap::kNullValueRootIndex);
-  if (expr->is_strict()) {
+  if (is_strict) {
     Split(equal, if_true, if_false, fall_through);
   } else {
     __ j(equal, if_true);
@@ -4043,7 +4039,6 @@ void FullCodeGenerator::VisitCompareToNull(CompareToNull* expr) {
              Immediate(1 << Map::kIsUndetectable));
     Split(not_zero, if_true, if_false, fall_through);
   }
-  context()->Plug(if_true, if_false);
 }