Skip some conditional deopts for Div/Mul when all uses are truncating.

- set "can be minus zero" flag properly so minus-zero checks are skipped
- skip "integer result?" check in division code when uses are truncating
- drive-by cleanup: consolidated computation of kCanOverflow flag for Add/Sub into range inference phase

BUG=v8:2132
R=svenpanne@chromium.org

Review URL: https://codereview.chromium.org/16741002

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@15060 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/arm/lithium-codegen-arm.cc

@@ -1419,8 +1419,7 @@ void LCodeGen::EmitSignedIntegerDivisionByConstant(
 void LCodeGen::DoDivI(LDivI* instr) {
   if (instr->hydrogen()->HasPowerOf2Divisor()) {
     Register dividend = ToRegister(instr->left());
-    int32_t divisor =
-        HConstant::cast(instr->hydrogen()->right())->Integer32Value();
+    int32_t divisor = instr->hydrogen()->right()->GetInteger32Constant();
     int32_t test_value = 0;
     int32_t power = 0;
 
@@ -1443,10 +1442,19 @@ void LCodeGen::DoDivI(LDivI* instr) {
     }
 
     if (test_value != 0) {
-      // Deoptimize if remainder is not 0.
-      __ tst(dividend, Operand(test_value));
-      DeoptimizeIf(ne, instr->environment());
-      __ mov(dividend, Operand(dividend, ASR, power));
+      if (instr->hydrogen()->CheckFlag(
+          HInstruction::kAllUsesTruncatingToInt32)) {
+        __ cmp(dividend, Operand(0));
+        __ rsb(dividend, dividend, Operand(0), LeaveCC, lt);
+        __ mov(dividend, Operand(dividend, ASR, power));
+        if (divisor > 0) __ rsb(dividend, dividend, Operand(0), LeaveCC, lt);
+        return;  // Don't fall through to "__ rsb" below.
+      } else {
+        // Deoptimize if remainder is not 0.
+        __ tst(dividend, Operand(test_value));
+        DeoptimizeIf(ne, instr->environment());
+        __ mov(dividend, Operand(dividend, ASR, power));
+      }
     }
     if (divisor < 0) __ rsb(dividend, dividend, Operand(0));
 
@@ -1487,11 +1495,14 @@ void LCodeGen::DoDivI(LDivI* instr) {
     CpuFeatureScope scope(masm(), SUDIV);
     __ sdiv(result, left, right);
 
-    // Compute remainder and deopt if it's not zero.
-    const Register remainder = scratch0();
-    __ mls(remainder, result, right, left);
-    __ cmp(remainder, Operand::Zero());
-    DeoptimizeIf(ne, instr->environment());
+    if (!instr->hydrogen()->CheckFlag(
+        HInstruction::kAllUsesTruncatingToInt32)) {
+      // Compute remainder and deopt if it's not zero.
+      const Register remainder = scratch0();
+      __ mls(remainder, result, right, left);
+      __ cmp(remainder, Operand::Zero());
+      DeoptimizeIf(ne, instr->environment());
+    }
   } else {
     const DoubleRegister vleft = ToDoubleRegister(instr->temp());
     const DoubleRegister vright = double_scratch0();
@@ -1500,14 +1511,17 @@ void LCodeGen::DoDivI(LDivI* instr) {
     __ vcvt_f64_s32(vleft, vleft.low());
     __ vcvt_f64_s32(vright, vright.low());
     __ vdiv(vleft, vleft, vright);  // vleft now contains the result.
-
-    // Convert back to integer32; deopt if exact conversion is not possible.
-    // Use vright as scratch register.
     __ vcvt_s32_f64(vright.low(), vleft);
     __ vmov(result, vright.low());
-    __ vcvt_f64_s32(vright, vright.low());
-    __ VFPCompareAndSetFlags(vleft, vright);
-    DeoptimizeIf(ne, instr->environment());
+
+    if (!instr->hydrogen()->CheckFlag(
+        HInstruction::kAllUsesTruncatingToInt32)) {
+      // Deopt if exact conversion to integer was not possible.
+      // Use vright as scratch register.
+      __ vcvt_f64_s32(vright, vright.low());
+      __ VFPCompareAndSetFlags(vleft, vright);
+      DeoptimizeIf(ne, instr->environment());
+    }
   }
 }
 

src/hydrogen-instructions.cc

@@ -528,6 +528,17 @@ bool HValue::CheckUsesForFlag(Flag f) {
 }
 
 
+bool HValue::HasAtLeastOneUseWithFlagAndNoneWithout(Flag f) {
+  bool return_value = false;
+  for (HUseIterator it(uses()); !it.Done(); it.Advance()) {
+    if (it.value()->IsSimulate()) continue;
+    if (!it.value()->CheckFlag(f)) return false;
+    return_value = true;
+  }
+  return return_value;
+}
+
+
 HUseIterator::HUseIterator(HUseListNode* head) : next_(head) {
   Advance();
 }
@@ -1429,14 +1440,6 @@ HValue* HBitNot::Canonicalize() {
 }
 
 
-HValue* HArithmeticBinaryOperation::Canonicalize() {
-  if (representation().IsInteger32() && CheckUsesForFlag(kTruncatingToInt32)) {
-    ClearFlag(kCanOverflow);
-  }
-  return this;
-}
-
-
 static bool IsIdentityOperation(HValue* arg1, HValue* arg2, int32_t identity) {
   return arg1->representation().IsSpecialization() &&
     arg2->EqualsInteger32Constant(identity);
@@ -1446,13 +1449,13 @@ 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();
-  return HArithmeticBinaryOperation::Canonicalize();
+  return this;
 }
 
 
 HValue* HSub::Canonicalize() {
   if (IsIdentityOperation(left(), right(), 0)) return left();
-  return HArithmeticBinaryOperation::Canonicalize();
+  return this;
 }
 
 
@@ -1758,11 +1761,13 @@ Range* HAdd::InferRange(Zone* zone) {
     Range* a = left()->range();
     Range* b = right()->range();
     Range* res = a->Copy(zone);
-    if (!res->AddAndCheckOverflow(b)) {
+    if (!res->AddAndCheckOverflow(b) ||
+        CheckFlag(kAllUsesTruncatingToInt32)) {
       ClearFlag(kCanOverflow);
     }
-    bool m0 = a->CanBeMinusZero() && b->CanBeMinusZero();
-    res->set_can_be_minus_zero(m0);
+    if (!CheckFlag(kAllUsesTruncatingToInt32)) {
+      res->set_can_be_minus_zero(a->CanBeMinusZero() && b->CanBeMinusZero());
+    }
     return res;
   } else {
     return HValue::InferRange(zone);
@@ -1775,10 +1780,13 @@ Range* HSub::InferRange(Zone* zone) {
     Range* a = left()->range();
     Range* b = right()->range();
     Range* res = a->Copy(zone);
-    if (!res->SubAndCheckOverflow(b)) {
+    if (!res->SubAndCheckOverflow(b) ||
+        CheckFlag(kAllUsesTruncatingToInt32)) {
       ClearFlag(kCanOverflow);
     }
-    res->set_can_be_minus_zero(a->CanBeMinusZero() && b->CanBeZero());
+    if (!CheckFlag(kAllUsesTruncatingToInt32)) {
+      res->set_can_be_minus_zero(a->CanBeMinusZero() && b->CanBeZero());
+    }
     return res;
   } else {
     return HValue::InferRange(zone);
@@ -1792,11 +1800,16 @@ Range* HMul::InferRange(Zone* zone) {
     Range* b = right()->range();
     Range* res = a->Copy(zone);
     if (!res->MulAndCheckOverflow(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.
       ClearFlag(kCanOverflow);
     }
-    bool m0 = (a->CanBeZero() && b->CanBeNegative()) ||
-        (a->CanBeNegative() && b->CanBeZero());
-    res->set_can_be_minus_zero(m0);
+    if (!CheckFlag(kAllUsesTruncatingToInt32)) {
+      bool m0 = (a->CanBeZero() && b->CanBeNegative()) ||
+          (a->CanBeNegative() && b->CanBeZero());
+      res->set_can_be_minus_zero(m0);
+    }
     return res;
   } else {
     return HValue::InferRange(zone);
@@ -1809,12 +1822,14 @@ Range* HDiv::InferRange(Zone* zone) {
     Range* a = left()->range();
     Range* b = right()->range();
     Range* result = new(zone) Range();
-    if (a->CanBeMinusZero()) {
-      result->set_can_be_minus_zero(true);
-    }
+    if (!CheckFlag(kAllUsesTruncatingToInt32)) {
+      if (a->CanBeMinusZero()) {
+        result->set_can_be_minus_zero(true);
+      }
 
-    if (a->CanBeZero() && b->CanBeNegative()) {
-      result->set_can_be_minus_zero(true);
+      if (a->CanBeZero() && b->CanBeNegative()) {
+        result->set_can_be_minus_zero(true);
+      }
     }
 
     if (!a->Includes(kMinInt) || !b->Includes(-1)) {
@@ -1846,7 +1861,7 @@ Range* HMod::InferRange(Zone* zone) {
     Range* result = new(zone) Range(left_can_be_negative ? -positive_bound : 0,
                                     a->CanBePositive() ? positive_bound : 0);
 
-    if (left_can_be_negative) {
+    if (left_can_be_negative && !CheckFlag(kAllUsesTruncatingToInt32)) {
       result->set_can_be_minus_zero(true);
     }
 
@@ -2298,10 +2313,6 @@ bool HBinaryOperation::IgnoreObservedOutputRepresentation(
          current_rep.IsInteger32() &&
          // Mul in Integer32 mode would be too precise.
          !this->IsMul() &&
-         // TODO(jkummerow): Remove blacklisting of Div when the Div
-         // instruction has learned not to deopt when the remainder is
-         // non-zero but all uses are truncating.
-         !this->IsDiv() &&
          CheckUsesForFlag(kTruncatingToInt32);
 }
 

src/hydrogen-instructions.h

@@ -54,6 +54,7 @@ class LChunkBuilder;
 
 
 #define HYDROGEN_ABSTRACT_INSTRUCTION_LIST(V)  \
+  V(ArithmeticBinaryOperation)                 \
   V(BinaryOperation)                           \
   V(BitwiseBinaryOperation)                    \
   V(ControlInstruction)                        \
@@ -797,6 +798,7 @@ class HValue: public ZoneObject {
     kAllowUndefinedAsNaN,
     kIsArguments,
     kTruncatingToInt32,
+    kAllUsesTruncatingToInt32,
     // Set after an instruction is killed.
     kIsDead,
     // Instructions that are allowed to produce full range unsigned integer
@@ -996,6 +998,9 @@ class HValue: public ZoneObject {
 
   // Returns true if the flag specified is set for all uses, false otherwise.
   bool CheckUsesForFlag(Flag f);
+  // Returns true if the flag specified is set for all uses, and this set
+  // of uses is non-empty.
+  bool HasAtLeastOneUseWithFlagAndNoneWithout(Flag f);
 
   GVNFlagSet gvn_flags() const { return gvn_flags_; }
   void SetGVNFlag(GVNFlag f) { gvn_flags_.Add(f); }
@@ -3856,7 +3861,7 @@ class HArithmeticBinaryOperation: public HBinaryOperation {
         : representation();
   }
 
-  virtual HValue* Canonicalize();
+  DECLARE_ABSTRACT_INSTRUCTION(ArithmeticBinaryOperation)
 
  private:
   virtual bool IsDeletable() const { return true; }
@@ -4488,9 +4493,8 @@ class HDiv: public HArithmeticBinaryOperation {
                            HValue* right);
 
   bool HasPowerOf2Divisor() {
-    if (right()->IsConstant() &&
-        HConstant::cast(right())->HasInteger32Value()) {
-      int32_t value = HConstant::cast(right())->Integer32Value();
+    if (right()->IsInteger32Constant()) {
+      int32_t value = right()->GetInteger32Constant();
       return value != 0 && (IsPowerOf2(value) || IsPowerOf2(-value));
     }
 

src/hydrogen.cc

@@ -2091,6 +2091,22 @@ void HGraph::FinalizeUniqueValueIds() {
 
 void HGraph::Canonicalize() {
   HPhase phase("H_Canonicalize", this);
+  // Before removing no-op instructions, save their semantic value.
+  // We must be careful not to set the flag unnecessarily, because GVN
+  // cannot identify two instructions when their flag value differs.
+  for (int i = 0; i < blocks()->length(); ++i) {
+    HInstruction* instr = blocks()->at(i)->first();
+    while (instr != NULL) {
+      if (instr->IsArithmeticBinaryOperation() &&
+          instr->representation().IsInteger32() &&
+          instr->HasAtLeastOneUseWithFlagAndNoneWithout(
+              HInstruction::kTruncatingToInt32)) {
+        instr->SetFlag(HInstruction::kAllUsesTruncatingToInt32);
+      }
+      instr = instr->next();
+    }
+  }
+  // Perform actual Canonicalization pass.
   for (int i = 0; i < blocks()->length(); ++i) {
     HInstruction* instr = blocks()->at(i)->first();
     while (instr != NULL) {

src/ia32/lithium-codegen-ia32.cc

@@ -1338,8 +1338,7 @@ void LCodeGen::DoModI(LModI* instr) {
 void LCodeGen::DoDivI(LDivI* instr) {
   if (!instr->is_flooring() && instr->hydrogen()->HasPowerOf2Divisor()) {
     Register dividend = ToRegister(instr->left());
-    int32_t divisor =
-        HConstant::cast(instr->hydrogen()->right())->Integer32Value();
+    int32_t divisor = instr->hydrogen()->right()->GetInteger32Constant();
     int32_t test_value = 0;
     int32_t power = 0;
 
@@ -1362,10 +1361,26 @@ void LCodeGen::DoDivI(LDivI* instr) {
     }
 
     if (test_value != 0) {
-      // Deoptimize if remainder is not 0.
-      __ test(dividend, Immediate(test_value));
-      DeoptimizeIf(not_zero, instr->environment());
-      __ sar(dividend, power);
+      if (instr->hydrogen()->CheckFlag(
+          HInstruction::kAllUsesTruncatingToInt32)) {
+        Label done, negative;
+        __ cmp(dividend, 0);
+        __ j(less, &negative, Label::kNear);
+        __ sar(dividend, power);
+        __ jmp(&done, Label::kNear);
+
+        __ bind(&negative);
+        __ neg(dividend);
+        __ sar(dividend, power);
+        if (divisor > 0) __ neg(dividend);
+        __ bind(&done);
+        return;  // Don't fall through to "__ neg" below.
+      } else {
+        // Deoptimize if remainder is not 0.
+        __ test(dividend, Immediate(test_value));
+        DeoptimizeIf(not_zero, instr->environment());
+        __ sar(dividend, power);
+      }
     }
 
     if (divisor < 0) __ neg(dividend);
@@ -1412,11 +1427,7 @@ void LCodeGen::DoDivI(LDivI* instr) {
   __ cdq();
   __ idiv(right_reg);
 
-  if (!instr->is_flooring()) {
-    // Deoptimize if remainder is not 0.
-    __ test(edx, Operand(edx));
-    DeoptimizeIf(not_zero, instr->environment());
-  } else {
+  if (instr->is_flooring()) {
     Label done;
     __ test(edx, edx);
     __ j(zero, &done, Label::kNear);
@@ -1424,6 +1435,11 @@ void LCodeGen::DoDivI(LDivI* instr) {
     __ sar(edx, 31);
     __ add(eax, edx);
     __ bind(&done);
+  } else if (!instr->hydrogen()->CheckFlag(
+      HInstruction::kAllUsesTruncatingToInt32)) {
+    // Deoptimize if remainder is not 0.
+    __ test(edx, Operand(edx));
+    DeoptimizeIf(not_zero, instr->environment());
   }
 }
 

src/x64/lithium-codegen-x64.cc

@@ -1257,10 +1257,26 @@ void LCodeGen::DoDivI(LDivI* instr) {
     }
 
     if (test_value != 0) {
-      // Deoptimize if remainder is not 0.
-      __ testl(dividend, Immediate(test_value));
-      DeoptimizeIf(not_zero, instr->environment());
-      __ sarl(dividend, Immediate(power));
+      if (instr->hydrogen()->CheckFlag(
+          HInstruction::kAllUsesTruncatingToInt32)) {
+        Label done, negative;
+        __ cmpl(dividend, Immediate(0));
+        __ j(less, &negative, Label::kNear);
+        __ sarl(dividend, Immediate(power));
+        __ jmp(&done, Label::kNear);
+
+        __ bind(&negative);
+        __ negl(dividend);
+        __ sarl(dividend, Immediate(power));
+        if (divisor > 0) __ negl(dividend);
+        __ bind(&done);
+        return;  // Don't fall through to "__ neg" below.
+      } else {
+        // Deoptimize if remainder is not 0.
+        __ testl(dividend, Immediate(test_value));
+        DeoptimizeIf(not_zero, instr->environment());
+        __ sarl(dividend, Immediate(power));
+      }
     }
 
     if (divisor < 0) __ negl(dividend);
@@ -1307,11 +1323,7 @@ void LCodeGen::DoDivI(LDivI* instr) {
   __ cdq();
   __ idivl(right_reg);
 
-  if (!instr->is_flooring()) {
-    // Deoptimize if remainder is not 0.
-    __ testl(rdx, rdx);
-    DeoptimizeIf(not_zero, instr->environment());
-  } else {
+  if (instr->is_flooring()) {
     Label done;
     __ testl(rdx, rdx);
     __ j(zero, &done, Label::kNear);
@@ -1319,6 +1331,11 @@ void LCodeGen::DoDivI(LDivI* instr) {
     __ sarl(rdx, Immediate(31));
     __ addl(rax, rdx);
     __ bind(&done);
+  } else if (!instr->hydrogen()->CheckFlag(
+      HInstruction::kAllUsesTruncatingToInt32)) {
+    // Deoptimize if remainder is not 0.
+    __ testl(rdx, rdx);
+    DeoptimizeIf(not_zero, instr->environment());
   }
 }
 

test/mjsunit/regress/regress-2132.js

+// Copyright 2013 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
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Flags: --allow-natives-syntax
+
+function mul(x, y) {
+  return (x * y) | 0;
+}
+
+mul(0, 0);
+mul(0, 0);
+%OptimizeFunctionOnNextCall(mul);
+assertEquals(0, mul(0, -1));
+assertTrue(%GetOptimizationStatus(mul) != 2);
+
+function div(x, y) {
+  return (x / y) | 0;
+}
+
+div(4, 2);
+div(4, 2);
+%OptimizeFunctionOnNextCall(div);
+assertEquals(1, div(5, 3));
+assertTrue(%GetOptimizationStatus(div) != 2);