Apply Kevin's suggestions, make branch-compare generation more robust

runtime/vm/intermediate_language_ia32.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/globals.h"  // Needed here to get TARGET_ARCH_IA32.
 #if defined(TARGET_ARCH_IA32)
 
 #include "vm/intermediate_language.h"
 
 #include "lib/error.h"
@@ skipping 174 matching lines @@
                                 try_index(),
                                 kConditionTypeErrorRuntimeEntry);
   // We should never return here.
   __ int3();
 
   __ Bind(&done);
   ASSERT(obj == result);
 }
 
 
+static Condition TokenKindToSmiCondition(Token::Kind kind) {
+  switch (kind) {
+    case Token::kEQ: return EQUAL;
+    case Token::kNE: return NOT_EQUAL;
+    case Token::kLT: return LESS;
+    case Token::kGT: return GREATER;
+    case Token::kLTE: return LESS_EQUAL;
+    case Token::kGTE: return  GREATER_EQUAL;
+    default:
+      UNREACHABLE();
+      return OVERFLOW;
+  }
+}
+
+
 LocationSummary* EqualityCompareComp::MakeLocationSummary() const {
   const intptr_t kNumInputs = 2;
   if (receiver_class_id() != kObject) {
     ASSERT((receiver_class_id() == kSmi) || (receiver_class_id() == kDouble));
     const intptr_t kNumTemps = 1;
     LocationSummary* locs = new LocationSummary(kNumInputs,
                                                 kNumTemps,
                                                 LocationSummary::kNoCall);
     locs->set_in(0, Location::RequiresRegister());
     locs->set_in(1, Location::RequiresRegister());
@@ skipping 36 matching lines @@
                                         kDeoptSmiCompareSmi,
                                         left,
                                         right);
   __ movl(temp, left);
   __ orl(temp, right);
   __ testl(temp, Immediate(kSmiTagMask));
   __ j(NOT_ZERO, deopt);
   __ cmpl(left, right);
   Register result = comp->locs()->out().reg();
   Label load_true, done;
-  __ j(EQUAL, &load_true, Assembler::kNearJump);
+  __ j(TokenKindToSmiCondition(comp->kind()), &load_true, Assembler::kNearJump);
   __ LoadObject(result, compiler->bool_false());
   __ jmp(&done, Assembler::kNearJump);
   __ Bind(&load_true);
   __ LoadObject(result, compiler->bool_true());
   __ Bind(&done);
 }
 
 
 // TODO(srdjan): Add support for mixed Smi/Double equality
 // (see LoadDoubleOrSmiToXmm).
 static void EmitDoubleEqualityCompare(FlowGraphCompiler* compiler,
                                       EqualityCompareComp* comp) {
   Register left = comp->locs()->in(0).reg();
   Register right = comp->locs()->in(1).reg();
   Register temp = comp->locs()->temp(0).reg();
   Label* deopt = compiler->AddDeoptStub(comp->cid(),
                                         comp->token_pos(),
                                         comp->try_index(),
                                         kDeoptDoubleCompareDouble,
                                         left,
                                         right);
   Label done, is_false, is_true;
   __ CompareClassId(left, kDouble, temp);
   __ j(NOT_EQUAL, deopt);
   __ CompareClassId(right, kDouble, temp);
   __ j(NOT_EQUAL, deopt);
   __ movsd(XMM0, FieldAddress(left, Double::value_offset()));
   __ movsd(XMM1, FieldAddress(right, Double::value_offset()));
-  compiler->EmitDoubleCompareBool(
-      EQUAL, XMM0, XMM1, comp->locs()->out().reg());
+  compiler->EmitDoubleCompareBool(TokenKindToSmiCondition(comp->kind()),
+                                  XMM0, XMM1,
+                                  comp->locs()->out().reg());
 }
 
 
 static void EmitEqualityAsInstanceCall(FlowGraphCompiler* compiler,
                                        EqualityCompareComp* comp) {
   compiler->AddCurrentDescriptor(PcDescriptors::kDeopt,
                                  comp->cid(),
                                  comp->token_pos(),
                                  comp->try_index());
   const String& operator_name = String::ZoneHandle(String::NewSymbol("=="));
   const int kNumberOfArguments = 2;
   const Array& kNoArgumentNames = Array::Handle();
   const int kNumArgumentsChecked = 2;
 
   compiler->GenerateInstanceCall(comp->cid(),
                                  comp->token_pos(),
                                  comp->try_index(),
                                  operator_name,
                                  kNumberOfArguments,
                                  kNoArgumentNames,
                                  kNumArgumentsChecked);
   ASSERT(comp->locs()->out().reg() == EAX);
+  if (comp->kind() == Token::kNE) {
+    Label done, false_label;
+    __ CompareObject(EAX, compiler->bool_true());
+    __ j(EQUAL, &false_label, Assembler::kNearJump);
+    __ LoadObject(EAX, compiler->bool_true());
+    __ jmp(&done, Assembler::kNearJump);
+    __ Bind(&false_label);
+    __ LoadObject(EAX, compiler->bool_false());
+    __ Bind(&done);
+  }
 }
 
 
 static void EmitEqualityAsPolymorphicCall(FlowGraphCompiler* compiler,
                                           const ICData& orig_ic_data,
                                           const LocationSummary& locs,
                                           BranchInstr* branch,
                                           Token::Kind kind,
                                           intptr_t cid,
                                           intptr_t token_pos,
                                           intptr_t try_index) {
+  ASSERT((kind == Token::kEQ) || (kind == Token::kNE));
   const ICData& ic_data = ICData::Handle(orig_ic_data.AsUnaryClassChecks());
   ASSERT(ic_data.NumberOfChecks() > 0);
   ASSERT(ic_data.num_args_tested() == 1);
   Label* deopt = compiler->AddDeoptStub(cid,
                                         token_pos,
                                         try_index,
                                         kDeoptEquality);
   Register left = locs.in(0).reg();
   Register right = locs.in(1).reg();
   __ testl(left, Immediate(kSmiTagMask));
   Register temp = locs.temp(0).reg();
   if (ic_data.GetReceiverClassIdAt(0) == kSmi) {
     Label done, load_class_id;
     __ j(NOT_ZERO, &load_class_id, Assembler::kNearJump);
     __ movl(temp, Immediate(kSmi));
     __ jmp(&done, Assembler::kNearJump);
     __ Bind(&load_class_id);
     __ LoadClassId(temp, left);
     __ Bind(&done);
   } else {
     __ j(ZERO, deopt);  // Smi deopts.
     __ LoadClassId(temp, left);
   }
-  Condition cond = (kind == Token::kEQ) ? EQUAL : NOT_EQUAL;
+  Condition cond = TokenKindToSmiCondition(kind);
   Label done;
   for (intptr_t i = 0; i < ic_data.NumberOfChecks(); i++) {
     ASSERT((ic_data.GetReceiverClassIdAt(i) != kSmi) || (i == 0));
     Label next_test;
     __ cmpl(temp, Immediate(ic_data.GetReceiverClassIdAt(i)));
     __ j(NOT_EQUAL, &next_test, Assembler::kNearJump);
     const Function& target = Function::ZoneHandle(ic_data.GetTargetAt(i));
     ObjectStore* object_store = Isolate::Current()->object_store();
     if (target.owner() == object_store->object_class()) {
       // Object.== is same as ===.
@@ skipping 13 matching lines @@
       }
     } else {
       const int kNumberOfArguments = 2;
       const Array& kNoArgumentNames = Array::Handle();
       compiler->GenerateStaticCall(cid,
                                    token_pos,
                                    try_index,
                                    target,
                                    kNumberOfArguments,
                                    kNoArgumentNames);
-      if (branch != NULL) {
+      if (branch == NULL) {
+        if (kind == Token::kNE) {
+          Label false_label;
+          __ CompareObject(EAX, compiler->bool_true());
+          __ j(EQUAL, &false_label, Assembler::kNearJump);
+          __ LoadObject(EAX, compiler->bool_true());
+          __ jmp(&done, Assembler::kNearJump);
+          __ Bind(&false_label);
+          __ LoadObject(EAX, compiler->bool_false());
+          __ jmp(&done);
+        }
+      } else {
         __ CompareObject(EAX, compiler->bool_true());
         branch->EmitBranchOnCondition(compiler, cond);
       }
     }
     __ jmp(&done);
     __ Bind(&next_test);
   }
   // Fall through leads to deoptimization
   __ jmp(deopt);
   __ Bind(&done);
 }
 
 
 // First test if receiver is NULL, in which case === is applied.
 // If type feedback was provided (lists of <class-id, target>), do a
 // type by type check (either === or static call to the operator.
 static void EmitGenericEqualityCompare(FlowGraphCompiler* compiler,
                                        const LocationSummary& locs,
                                        Token::Kind kind,
                                        BranchInstr* branch,
                                        const ICData& ic_data,
                                        intptr_t cid,
                                        intptr_t token_pos,
                                        intptr_t try_index) {
+  ASSERT((kind == Token::kEQ) || (kind == Token::kNE));
   ASSERT(!ic_data.IsNull() && (ic_data.NumberOfChecks() > 0));
   Register left = locs.in(0).reg();
   Register right = locs.in(1).reg();
   const Immediate raw_null =
       Immediate(reinterpret_cast<intptr_t>(Object::null()));
   Label done, non_null_compare;
   __ cmpl(left, raw_null);
   __ j(NOT_EQUAL, &non_null_compare, Assembler::kNearJump);
   // Comparison with NULL is "===".
   __ cmpl(left, right);
-  Condition cond = (kind == Token::kEQ) ? EQUAL : NOT_EQUAL;
+  Condition cond = TokenKindToSmiCondition(kind);
   if (branch != NULL) {
     branch->EmitBranchOnCondition(compiler, cond);
   } else {
     Register result = locs.out().reg();
     Label load_true;
     __ j(cond, &load_true, Assembler::kNearJump);
     __ LoadObject(result, compiler->bool_false());
     __ jmp(&done);
     __ Bind(&load_true);
     __ LoadObject(result, compiler->bool_true());
@@ skipping 11 matching lines @@
 void EqualityCompareComp::EmitNativeCode(FlowGraphCompiler* compiler) {
   if (receiver_class_id() == kSmi) {
     EmitSmiEqualityCompare(compiler, this);
     return;
   }
   if (receiver_class_id() == kDouble) {
     EmitDoubleEqualityCompare(compiler, this);
     return;
   }
   if (HasICData() && (ic_data()->NumberOfChecks() > 0)) {
-    EmitGenericEqualityCompare(compiler, *locs(), Token::kEQ, NULL,
+    EmitGenericEqualityCompare(compiler, *locs(), kind(), NULL,
                                *ic_data(), cid(), token_pos(), try_index());
   } else {
     Register left = locs()->in(0).reg();
     Register right = locs()->in(1).reg();
     __ pushl(left);
     __ pushl(right);
     EmitEqualityAsInstanceCall(compiler, this);
   }
 }
 
 
 LocationSummary* RelationalOpComp::MakeLocationSummary() const {
   if ((operands_class_id() == kSmi) || (operands_class_id() == kDouble)) {
     const intptr_t kNumInputs = 2;
     const intptr_t kNumTemps = 1;
     LocationSummary* summary = new LocationSummary(kNumInputs,
                                                    kNumTemps,
                                                    LocationSummary::kNoCall);
     summary->set_in(0, Location::RequiresRegister());
     summary->set_in(1, Location::RequiresRegister());
     summary->set_out(Location::RequiresRegister());
     summary->set_temp(0, Location::RequiresRegister());
     return summary;
   }
   ASSERT(operands_class_id() == kObject);
   return MakeCallSummary();
 }
 
 
-static Condition TokenKindToSmiCondition(Token::Kind kind) {
-  switch (kind) {
-    case Token::kEQ: return EQUAL;
-    case Token::kNE: return NOT_EQUAL;
-    case Token::kLT: return LESS;
-    case Token::kGT: return GREATER;
-    case Token::kLTE: return LESS_EQUAL;
-    case Token::kGTE: return  GREATER_EQUAL;
-    default:
-      UNREACHABLE();
-      return OVERFLOW;
-  }
-}
-
-
 static void EmitSmiComparisonOp(FlowGraphCompiler* compiler,
                                 const LocationSummary& locs,
                                 Token::Kind kind,
                                 BranchInstr* branch,
                                 intptr_t cid,
                                 intptr_t token_pos,
                                 intptr_t try_index) {
   Register left = locs.in(0).reg();
   Register right = locs.in(1).reg();
   Register temp = locs.temp(0).reg();
@@ skipping 1647 matching lines @@
   ASSERT(locs()->out().reg() == EAX);
   __ CompareObject(locs()->out().reg(), compiler->bool_true());
   EmitBranchOnCondition(compiler, branch_condition);
 }
 
 }  // namespace dart
 
 #undef __
 
 #endif  // defined TARGET_ARCH_X64