Unify canonicalization of HAdd/HSub/HMul a bit.

src/hydrogen-instructions.cc

Index: src/hydrogen-instructions.cc
diff --git a/src/hydrogen-instructions.cc b/src/hydrogen-instructions.cc
index 0aabfbc43229198f181ec9f76d4aaf4724228f4f..89b739fee61e14cd28fa2724de49f193ad4537fa 100644
--- a/src/hydrogen-instructions.cc
+++ b/src/hydrogen-instructions.cc
@@ -1422,21 +1422,14 @@ HValue* HBitNot::Canonicalize() {
 }
 
 
-HValue* HAdd::Canonicalize() {
-  if (!representation().IsInteger32()) return this;
-  if (CheckUsesForFlag(kTruncatingToInt32)) ClearFlag(kCanOverflow);
-  return this;
-}
-
-
-HValue* HSub::Canonicalize() {
-  if (!representation().IsInteger32()) return this;
-  if (CheckUsesForFlag(kTruncatingToInt32)) ClearFlag(kCanOverflow);
+HValue* HArithmeticBinaryOperation::Canonicalize() {
+  if (representation().IsInteger32() && CheckUsesForFlag(kTruncatingToInt32)) {
+    ClearFlag(kCanOverflow);
+  }
   return this;
 }
 
 
-// TODO(svenpanne) Use this in other Canonicalize() functions.
 static bool IsIdentityOperation(HValue* arg1, HValue* arg2, int32_t identity) {
   return arg1->representation().IsSpecialization() &&
       arg2->IsInteger32Constant() &&
@@ -1444,9 +1437,25 @@ static bool IsIdentityOperation(HValue* arg1, HValue* arg2, int32_t identity) {
 }
 
 
+HValue* HAdd::Canonicalize() {
+  if (IsIdentityOperation(left(), right(), 0)) return left();
+  if (IsIdentityOperation(right(), left(), 0)) return right();
+  HArithmeticBinaryOperation::Canonicalize();
+  return this;

[Jakob Kummerow, 2013/04/18 09:11:06]

Instead of:
    HArithmeticBinaryOperation::Canonicalize();
    return this;
I would prefer:
    return HArithmeticBinaryOperation::Canonicalize();
Otherwise that could lead to surprising behavior when someone puts more logic
into the superclass function and doesn't notice that callers ignore the return
value.

[Sven Panne, 2013/04/18 09:19:18]

Good point, I even think I had something similar in previous versions. :-/
Changed here and in HSub/HMul.

+}
+
+
+HValue* HSub::Canonicalize() {
+  if (IsIdentityOperation(left(), right(), 0)) return left();
+  HArithmeticBinaryOperation::Canonicalize();
+  return this;
+}
+
+
 HValue* HMul::Canonicalize() {
   if (IsIdentityOperation(left(), right(), 1)) return left();
   if (IsIdentityOperation(right(), left(), 1)) return right();
+  HArithmeticBinaryOperation::Canonicalize();
   return this;
 }