Change == into an instance call to allow polymorphic inlining of ==.

runtime/vm/flow_graph_builder.cc

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/flow_graph_builder.h"
 
 #include "lib/invocation_mirror.h"
 #include "vm/ast_printer.h"
 #include "vm/bit_vector.h"
 #include "vm/code_descriptors.h"
@@ skipping 652 matching lines @@
 void TestGraphVisitor::ReturnValue(Value* value) {
   if (FLAG_enable_type_checks) {
     value = Bind(new AssertBooleanInstr(condition_token_pos(), value));
   }
   Value* constant_true = Bind(new ConstantInstr(Bool::True()));
   StrictCompareInstr* comp =
       new StrictCompareInstr(condition_token_pos(),
                              Token::kEQ_STRICT,
                              value,
                              constant_true);
+  comp->set_needs_number_check(false);
   BranchInstr* branch = new BranchInstr(comp);
   AddInstruction(branch);
   CloseFragment();
 
   true_successor_addresses_.Add(branch->true_successor_address());
   false_successor_addresses_.Add(branch->false_successor_address());
 }
 
 
 void TestGraphVisitor::MergeBranchWithComparison(ComparisonInstr* comp) {
   BranchInstr* branch;
   if (Token::IsStrictEqualityOperator(comp->kind())) {
-    branch = new BranchInstr(new StrictCompareInstr(comp->token_pos(),
-                                                    comp->kind(),
-                                                    comp->left(),
-                                                    comp->right()));
+    ASSERT(comp->IsStrictCompare());
+    branch = new BranchInstr(comp);
   } else if (Token::IsEqualityOperator(comp->kind()) &&
              (comp->left()->BindsToConstantNull() ||
               comp->right()->BindsToConstantNull())) {
     branch = new BranchInstr(new StrictCompareInstr(
         comp->token_pos(),
         (comp->kind() == Token::kEQ) ? Token::kEQ_STRICT : Token::kNE_STRICT,
         comp->left(),
         comp->right()));
   } else {
     branch = new BranchInstr(comp, FLAG_enable_type_checks);
   }
   AddInstruction(branch);
   CloseFragment();
   true_successor_addresses_.Add(branch->true_successor_address());
   false_successor_addresses_.Add(branch->false_successor_address());
 }
 
 
 void TestGraphVisitor::MergeBranchWithNegate(BooleanNegateInstr* neg) {
   ASSERT(!FLAG_enable_type_checks);
   Value* constant_true = Bind(new ConstantInstr(Bool::True()));
-  BranchInstr* branch = new BranchInstr(
+  StrictCompareInstr* comp =
       new StrictCompareInstr(condition_token_pos(),
                              Token::kNE_STRICT,
                              neg->value(),
-                             constant_true));
+                             constant_true);
+  comp->set_needs_number_check(false);
+  BranchInstr* branch = new BranchInstr(comp);
   AddInstruction(branch);
   CloseFragment();
   true_successor_addresses_.Add(branch->true_successor_address());
   false_successor_addresses_.Add(branch->false_successor_address());
 }
 
 
 void TestGraphVisitor::ReturnDefinition(Definition* definition) {
   ComparisonInstr* comp = definition->AsComparison();
   if (comp != NULL) {
@@ skipping 627 matching lines @@
       Library::PrivateCoreLibName(Symbols::_as()),
       node->kind(),
       arguments,
       Object::null_array(),  // No argument names.
       kNumArgsChecked,
       owner()->ic_data_array());
   ReturnDefinition(call);
 }
 
 
+StrictCompareInstr* EffectGraphVisitor::BuildStrictCompare(AstNode* left,
+                                                           AstNode* right,
+                                                           Token::Kind kind,
+                                                           intptr_t token_pos) {

[srdjan, 2013/10/28 21:02:25]

Should you assert that kind is in valid range?

[Florian Schneider, 2013/11/04 10:39:54]

I could do it here, but the constructor of StrictCompareInstr already asserts
that.

+  ValueGraphVisitor for_left_value(owner(), temp_index());
+  left->Visit(&for_left_value);
+  Append(for_left_value);
+  ValueGraphVisitor for_right_value(owner(), temp_index());
+  right->Visit(&for_right_value);
+  Append(for_right_value);
+  StrictCompareInstr* comp = new StrictCompareInstr(token_pos,
+                                                    kind,
+                                                    for_left_value.value(),
+                                                    for_right_value.value());
+  return comp;
+}
+
+
 // <Expression> :: Comparison { kind:  Token::Kind
 //                              left:  <Expression>
 //                              right: <Expression> }
 // TODO(srdjan): Implement new equality.
 void EffectGraphVisitor::VisitComparisonNode(ComparisonNode* node) {
   if (Token::IsTypeTestOperator(node->kind())) {
     BuildTypeTest(node);
     return;
   }
   if (Token::IsTypeCastOperator(node->kind())) {
     BuildTypeCast(node);
     return;
   }
+
   if ((node->kind() == Token::kEQ_STRICT) ||
       (node->kind() == Token::kNE_STRICT)) {
-    ValueGraphVisitor for_left_value(owner(), temp_index());
-    node->left()->Visit(&for_left_value);
-    Append(for_left_value);
-    ValueGraphVisitor for_right_value(owner(), temp_index());
-    node->right()->Visit(&for_right_value);
-    Append(for_right_value);
-    StrictCompareInstr* comp = new StrictCompareInstr(node->token_pos(),
-                                                      node->kind(),
-                                                      for_left_value.value(),
-                                                      for_right_value.value());
-    ReturnDefinition(comp);
+    ReturnDefinition(BuildStrictCompare(node->left(), node->right(),
+                                        node->kind(), node->token_pos()));
     return;
   }
 
   if ((node->kind() == Token::kEQ) || (node->kind() == Token::kNE)) {
+    // Eagerly fold null-comparisons.
+    LiteralNode* left_lit = node->left()->AsLiteralNode();
+    LiteralNode* right_lit = node->right()->AsLiteralNode();
+    if (((left_lit != NULL) && left_lit->literal().IsNull()) ||
+        ((right_lit != NULL) && right_lit->literal().IsNull())) {
+      Token::Kind kind =
+          (node->kind() == Token::kEQ) ? Token::kEQ_STRICT : Token::kNE_STRICT;
+      StrictCompareInstr* compare =
+          BuildStrictCompare(node->left(), node->right(),
+                             kind, node->token_pos());
+      ReturnDefinition(compare);
+      return;
+    }
+
+    ZoneGrowableArray<PushArgumentInstr*>* arguments =
+        new ZoneGrowableArray<PushArgumentInstr*>(2);
+
     ValueGraphVisitor for_left_value(owner(), temp_index());
     node->left()->Visit(&for_left_value);
     Append(for_left_value);
+    PushArgumentInstr* push_left = PushArgument(for_left_value.value());
+    arguments->Add(push_left);
+
     ValueGraphVisitor for_right_value(owner(), temp_index());
     node->right()->Visit(&for_right_value);
     Append(for_right_value);
+    PushArgumentInstr* push_right = PushArgument(for_right_value.value());
+    arguments->Add(push_right);
+
+    Definition* result =
+        new InstanceCallInstr(node->token_pos(),
+                              Symbols::EqualOperator(),
+                              Token::kEQ,  // Result is negated later for kNE.
+                              arguments,
+                              Object::null_array(),
+                              2,
+                              owner()->ic_data_array());
     if (FLAG_enable_type_checks) {
-      EqualityCompareInstr* comp = new EqualityCompareInstr(
-          node->token_pos(),
-          Token::kEQ,
-          for_left_value.value(),
-          for_right_value.value(),
-          owner()->ic_data_array());
-      if (node->kind() == Token::kEQ) {
-        ReturnDefinition(comp);
-      } else {
-        Value* eq_result = Bind(comp);
-        eq_result = Bind(new AssertBooleanInstr(node->token_pos(), eq_result));
-        ReturnDefinition(new BooleanNegateInstr(eq_result));
-      }
-    } else {
-      EqualityCompareInstr* comp = new EqualityCompareInstr(
-          node->token_pos(),
-          node->kind(),
-          for_left_value.value(),
-          for_right_value.value(),
-          owner()->ic_data_array());
-      ReturnDefinition(comp);
+      Value* value = Bind(result);
+      result = new AssertBooleanInstr(node->token_pos(), value);
     }
+
+    if (node->kind() == Token::kNE) {
+      Value* value = Bind(result);
+      result = new BooleanNegateInstr(value);
+    }
+    ReturnDefinition(result);
     return;
   }
 
   ZoneGrowableArray<PushArgumentInstr*>* arguments =
       new ZoneGrowableArray<PushArgumentInstr*>(2);
 
   ValueGraphVisitor for_left_value(owner(), temp_index());
   node->left()->Visit(&for_left_value);
   Append(for_left_value);
   PushArgumentInstr* push_left = PushArgument(for_left_value.value());
@@ skipping 1479 matching lines @@
                                                       true);  // Test only.
   return new LoadLocalInstr(*receiver_var);
 }
 
 
 void EffectGraphVisitor::VisitNativeBodyNode(NativeBodyNode* node) {
   const Function& function = owner()->parsed_function()->function();
   if (!function.IsClosureFunction()) {
     MethodRecognizer::Kind kind = MethodRecognizer::RecognizeKind(function);
     switch (kind) {
+      case MethodRecognizer::kObjectEquals: {
+        Value* receiver = Bind(BuildLoadThisVar(node->scope()));
+        LocalVariable* other_var =
+            node->scope()->LookupVariable(Symbols::Other(),
+                                          true);  // Test only.
+        Value* other = Bind(new LoadLocalInstr(*other_var));
+        StrictCompareInstr* compare =
+            new StrictCompareInstr(node->token_pos(),
+                                   Token::kEQ_STRICT,
+                                   receiver,
+                                   other);
+        // Receiver is not a number because numbers override equality.

[Florian Schneider, 2013/10/28 13:34:19]

I moved the implementation of Object.== here so that the number check can be
eliminated right away (for both unoptimized, and optimized code)

+        compare->set_needs_number_check(false);
+        return ReturnDefinition(compare);
+      }
       case MethodRecognizer::kStringBaseLength:
       case MethodRecognizer::kStringBaseIsEmpty: {
         Value* receiver = Bind(BuildLoadThisVar(node->scope()));
         // Treat length loads as mutable (i.e. affected by side effects) to
         // avoid hoisting them since we can't hoist the preceding class-check.
         // This is because of externalization of strings that affects their
         // class-id.
         const bool is_immutable = false;
         LoadFieldInstr* load = new LoadFieldInstr(
             receiver,
@@ skipping 919 matching lines @@
   intptr_t len = OS::SNPrint(NULL, 0, kFormat, function_name, reason) + 1;
   char* chars = Isolate::Current()->current_zone()->Alloc<char>(len);
   OS::SNPrint(chars, len, kFormat, function_name, reason);
   const Error& error = Error::Handle(
       LanguageError::New(String::Handle(String::New(chars))));
   Isolate::Current()->long_jump_base()->Jump(1, error);
 }
 
 
 }  // namespace dart