Reviewers: Sven Panne, Toon Verwaest,

Message:
@sven, this CL is a workaround fix for
https://code.google.com/p/v8/issues/detail?id=2898

@Toon, I move "HasNonSmiUse()" before observed_output_representation_ in
HBinaryOperation::InferRepresentation. The reason is in
IgnoreObservedOutputRepresentation, representation and related truncated will be
checked. We need to adjust Smi to Int32 before that.

I also fix "(MinInt/-1) ^ 1" which would better not to check overflow because of
its truncated int32 use.

Below is the performance data on my X86 PC.

        opt     orig
ss      218.2   218.3
kraken  1788    1785
Octane  15974   15804



Description:
Special handle for mul/div minus one when kAllUsesTruncatingToInt32

BUG=

Please review this at https://codereview.chromium.org/24521002/

SVN Base: git://github.com/v8/v8.git@master

Affected files (+54, -26 lines):
  M src/hydrogen-instructions.h
  M src/hydrogen-instructions.cc
  A + test/mjsunit/div-mul-minus-one.js


Index: src/hydrogen-instructions.cc
diff --git a/src/hydrogen-instructions.cc b/src/hydrogen-instructions.cc
index a685198ba6288e54e0913695f6e6f4e5cc797f09..ca5c02497fd04e4969027d364d4c1ca8951a92d7 100644
--- a/src/hydrogen-instructions.cc
+++ b/src/hydrogen-instructions.cc
@@ -1237,6 +1237,19 @@ HValue* HMul::Canonicalize() {
 }


+bool HMul::MulMinusOne() {
+  ASSERT(left().IsSmiOrInteger32());
+  ASSERT(right().IsSmiOrInteger32());
+
+  if (left()->EqualsInteger32Constant(-1) ||
+      right()->EqualsInteger32Constant(-1)) {
+    return true;
+  }
+
+  return false;
+}
+
+
 HValue* HMod::Canonicalize() {
   return this;
 }
@@ -1622,10 +1635,13 @@ Range* HMul::InferRange(Zone* zone) {
     Range* a = left()->range();
     Range* b = right()->range();
     Range* res = a->Copy(zone);
-    if (!res->MulAndCheckOverflow(r, b)) {
-      // Clearing the kCanOverflow flag when kAllUsesAreTruncatingToInt32
-      // would be wrong, because truncated integer multiplication is too
- // precise and therefore not the same as converting to Double and back.
+    if (!res->MulAndCheckOverflow(r, b) ||
+        (((r.IsInteger32() && CheckFlag(kAllUsesTruncatingToInt32)) ||
+         (r.IsSmi() && CheckFlag(kAllUsesTruncatingToSmi))) &&
+         MulMinusOne())) {
+      // Truncated int multiplication is too precise and therefore not the
+      // same as converting to Double and back.
+      // Special handle for truncated int multiplication with minus one.
       ClearFlag(kCanOverflow);
     }
     res->set_can_be_minus_zero(!CheckFlag(kAllUsesTruncatingToSmi) &&
@@ -1647,7 +1663,10 @@ Range* HDiv::InferRange(Zone* zone) {
     result->set_can_be_minus_zero(!CheckFlag(kAllUsesTruncatingToInt32) &&
                                   (a->CanBeMinusZero() ||
(a->CanBeZero() && b->CanBeNegative())));
-    if (!a->Includes(kMinInt) || !b->Includes(-1)) {
+    if (!a->Includes(kMinInt) ||
+        !b->Includes(-1) ||
+        CheckFlag(kAllUsesTruncatingToInt32)) {
+      // It is safe to clear kCanOverflow when kAllUsesTruncatingToInt32.
       ClearFlag(HValue::kCanOverflow);
     }

@@ -2620,6 +2639,12 @@ void HBinaryOperation::InferRepresentation(HInferRepresentationPhase* h_infer) {
   ASSERT(CheckFlag(kFlexibleRepresentation));
   Representation new_rep = RepresentationFromInputs();
   UpdateRepresentation(new_rep, h_infer, "inputs");
+
+  if (representation().IsSmi() && HasNonSmiUse()) {
+    UpdateRepresentation(
+        Representation::Integer32(), h_infer, "use requirements");
+  }
+
   if (observed_output_representation_.IsNone()) {
     new_rep = RepresentationFromUses();
     UpdateRepresentation(new_rep, h_infer, "uses");
@@ -2627,11 +2652,6 @@ void HBinaryOperation::InferRepresentation(HInferRepresentationPhase* h_infer) {
     new_rep = RepresentationFromOutput();
     UpdateRepresentation(new_rep, h_infer, "output");
   }
-
-  if (representation().IsSmi() && HasNonSmiUse()) {
-    UpdateRepresentation(
-        Representation::Integer32(), h_infer, "use requirements");
-  }
 }


@@ -2657,8 +2677,9 @@ bool HBinaryOperation::IgnoreObservedOutputRepresentation(
     Representation current_rep) {
return ((current_rep.IsInteger32() && CheckUsesForFlag(kTruncatingToInt32)) ||
           (current_rep.IsSmi() && CheckUsesForFlag(kTruncatingToSmi))) &&
-         // Mul in Integer32 mode would be too precise.
-         !this->IsMul();
+          // Mul in Integer32 mode would be too precise.
+          (!this->IsMul() ||
+           HMul::cast(this)->MulMinusOne());
 }


Index: src/hydrogen-instructions.h
diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h
index 8cb2f591742aebec491d57d4ab9305b51cc1eeb4..3f88a61c7783fb0ee5d4d433e93e5dfb8d6d50cb 100644
--- a/src/hydrogen-instructions.h
+++ b/src/hydrogen-instructions.h
@@ -4625,6 +4625,8 @@ class HMul V8_FINAL : public HArithmeticBinaryOperation { HArithmeticBinaryOperation::UpdateRepresentation(new_rep, h_infer, reason);
   }

+  bool MulMinusOne();
+
   DECLARE_CONCRETE_INSTRUCTION(Mul)

  protected:
Index: test/mjsunit/div-mul-minus-one.js
diff --git a/test/mjsunit/regress/regress-2132.js b/test/mjsunit/div-mul-minus-one.js
similarity index 79%
copy from test/mjsunit/regress/regress-2132.js
copy to test/mjsunit/div-mul-minus-one.js
index 9eb2dc5b073e6e3c2ce46362903d07d727de3d27..f05bf0f54c68faf8f5e5cb5f35d7a8dcbc3fb02f 100644
--- a/test/mjsunit/regress/regress-2132.js
+++ b/test/mjsunit/div-mul-minus-one.js
@@ -27,22 +27,27 @@

 // Flags: --allow-natives-syntax

-function mul(x, y) {
-  return (x * y) | 0;
+function div(g) {
+  return (g/-1) ^ 1
 }

-mul(0, 0);
-mul(0, 0);
-%OptimizeFunctionOnNextCall(mul);
-assertEquals(0, mul(0, -1));
-assertOptimized(mul);
+var kMinInt = 1 << 31;
+var expected_MinInt = div(kMinInt);
+var expected_minus_zero = div(0);
+%OptimizeFunctionOnNextCall(div);
+assertEquals(expected_MinInt, div(kMinInt));
+assertOptimized(div);
+assertEquals(expected_minus_zero , div(0));
+assertOptimized(div);

-function div(x, y) {
-  return (x / y) | 0;
+function mul(g) {
+  return (g * -1) ^ 1
 }

-div(4, 2);
-div(4, 2);
-%OptimizeFunctionOnNextCall(div);
-assertEquals(1, div(5, 3));
-assertOptimized(div);
+expected_MinInt = mul(kMinInt);
+expected_minus_zero = mul(0);
+%OptimizeFunctionOnNextCall(mul);
+assertEquals(expected_MinInt, mul(kMinInt));
+assertOptimized(mul);
+assertEquals(expected_minus_zero , mul(0));
+assertOptimized(mul);


--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.

Reply via email to