Unify canonicalization of HAdd/HSub/HMul a bit.

src/hydrogen-instructions.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 1404 matching lines @@
     HValue* result = HBitNot::cast(value())->value();
     ASSERT(result->representation().IsInteger32());
     if (!result->CheckFlag(kUint32)) {
       return result;
     }
   }
   return this;
 }
 
 
-HValue* HAdd::Canonicalize() {
-  if (!representation().IsInteger32()) return this;
-  if (CheckUsesForFlag(kTruncatingToInt32)) ClearFlag(kCanOverflow);
+HValue* HArithmeticBinaryOperation::Canonicalize() {
+  if (representation().IsInteger32() && CheckUsesForFlag(kTruncatingToInt32)) {
+    ClearFlag(kCanOverflow);
+  }
   return this;
 }
 
 
-HValue* HSub::Canonicalize() {
-  if (!representation().IsInteger32()) return this;
-  if (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() &&
       arg2->GetInteger32Constant() == 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;
 }
 
 
 HValue* HChange::Canonicalize() {
   return (from().Equals(to())) ? value() : this;
 }
 
 
 HValue* HWrapReceiver::Canonicalize() {
@@ skipping 2125 matching lines @@
 
 
 void HCheckFunction::Verify() {
   HInstruction::Verify();
   ASSERT(HasNoUses());
 }
 
 #endif
 
 } }  // namespace v8::internal