Improve code for !identical(a, b):

runtime/vm/intermediate_language.cc

Index: runtime/vm/intermediate_language.cc
===================================================================
--- runtime/vm/intermediate_language.cc	(revision 20450)
+++ runtime/vm/intermediate_language.cc	(working copy)
@@ -1239,9 +1239,13 @@
   // Only handle strict-compares.
   if (comparison()->IsStrictCompare()) {
     Definition* replacement = comparison()->Canonicalize(optimizer);
-    if (replacement == comparison() || replacement == NULL) return this;
+    if ((replacement == comparison()) || (replacement == NULL)) {
+      return this;
+    }
     ComparisonInstr* comp = replacement->AsComparison();
-    if ((comp == NULL) || comp->CanDeoptimize()) return this;
+    if ((comp == NULL) || comp->CanDeoptimize()) {
+      return this;
+    }
 
     // Check that comparison is not serving as a pending deoptimization target
     // for conversions.
@@ -1274,7 +1278,9 @@
 
 
 Definition* StrictCompareInstr::Canonicalize(FlowGraphOptimizer* optimizer) {
-  if (!right()->BindsToConstant()) return this;
+  if (!right()->BindsToConstant()) {
+    return this;
+  }
   const Object& right_constant = right()->BoundConstant();
   Definition* left_defn = left()->definition();
   // TODO(fschneider): Handle other cases: e === false and e !== true/false.
@@ -1285,6 +1291,22 @@
     // Return left subexpression as the replacement for this instruction.
     return left_defn;
   }
+  // x = (a === b);   y = x !== true; -> y = a !== b.

[Kevin Millikin (Google), 2013/03/26 12:48:31]

Nit: extra space character in the comment.

Perhaps mention that we do not need to check left's cid (unlike just above)
because we require it to be a strict equality.

[srdjan, 2013/03/26 22:35:25]

Done.

+  // In order to merge two strict comares, 'left_strict' must have only one use.
+  StrictCompareInstr* left_strict = left_defn->AsStrictCompare();
+  if ((kind() == Token::kNE_STRICT) &&
+      (right_constant.raw() == Bool::True().raw()) &&
+      (left_strict != NULL) &&
+      (left_strict->input_use_list()->next_use() == NULL)) {

[Kevin Millikin (Google), 2013/03/26 12:48:31]

I suppose there should be not environment uses either.  You can write

left_strict->HasOnlyUse(left())

to check that it has only the expected use.

[srdjan, 2013/03/26 22:35:25]

Thanks!

+    Token::Kind negated_kind = Token::NegateComparison(left_strict->kind());
+    StrictCompareInstr* negated_strict =
+        new StrictCompareInstr(negated_kind,

[Kevin Millikin (Google), 2013/03/26 12:48:31]

Do you think it's too unsafe to just replace the token in the existing
StrictCompareInstr?  It seems OK to me since we are removing it and have
verified it has no uses.

[srdjan, 2013/03/26 22:35:25]

I am on the edge on that one (tried both). Adding a kind_ field mutating
operation just for this case seems less safe than this ... on the other hand 
....

Changed as suggested.

+                               left_strict->left()->Copy(),
+                               left_strict->right()->Copy());
+    left_strict->ReplaceWith(negated_strict, optimizer->current_iterator());
+    return negated_strict;
+  }
+
   return this;
 }