Improve code for integral modulus calculation.

src/ia32/lithium-codegen-ia32.cc

Index: src/ia32/lithium-codegen-ia32.cc
diff --git a/src/ia32/lithium-codegen-ia32.cc b/src/ia32/lithium-codegen-ia32.cc
index 98f1e8beb45dde94f0f9204d4f7f3d24a58d3457..1db821f22e40b9aa031b3a5a8a7b8b3d47e64a54 100644
--- a/src/ia32/lithium-codegen-ia32.cc
+++ b/src/ia32/lithium-codegen-ia32.cc
@@ -1234,110 +1234,119 @@ void LCodeGen::DoUnknownOSRValue(LUnknownOSRValue* instr) {
 
 
 void LCodeGen::DoModI(LModI* instr) {
-  if (instr->hydrogen()->HasPowerOf2Divisor()) {
-    Register dividend = ToRegister(instr->left());
-
-    int32_t divisor =
-        HConstant::cast(instr->hydrogen()->right())->Integer32Value();
+  HMod* hmod = instr->hydrogen();
+  HValue* left = hmod->left();
+  HValue* right = hmod->right();
+  if (hmod->HasPowerOf2Divisor()) {
+    // TODO(svenpanne) We should really do the strength reduction on the
+    // Hydrogen level.
+    Register left_reg = ToRegister(instr->left());
+    ASSERT(left_reg.is(ToRegister(instr->result())));
 
+    int32_t divisor = HConstant::cast(right)->Integer32Value();

[Jakob Kummerow, 2013/06/05 17:08:41]

even shorter: right->GetInteger32Constant()

[Sven Panne, 2013/06/06 06:50:23]

Done. Lame excuse: It has been there before. ;-) Furthermore, there are 21 other
places like this, but these should handled in a different cleanup CL.

Added an Abs template on the way.

     if (divisor < 0) divisor = -divisor;
 
-    Label positive_dividend, done;
-    __ test(dividend, Operand(dividend));
-    __ j(not_sign, &positive_dividend, Label::kNear);
-    __ neg(dividend);
-    __ and_(dividend, divisor - 1);
-    __ neg(dividend);
-    if (instr->hydrogen()->CheckFlag(HValue::kBailoutOnMinusZero)) {
-      __ j(not_zero, &done, Label::kNear);
-      DeoptimizeIf(no_condition, instr->environment());
-    } else {
+    Label left_is_not_negative, done;
+    if (left->CanBeNegative()) {
+      __ test(left_reg, Operand(left_reg));
+      __ j(not_sign, &left_is_not_negative, Label::kNear);
+      __ neg(left_reg);
+      __ and_(left_reg, divisor - 1);
+      __ neg(left_reg);
+      if (hmod->CheckFlag(HValue::kBailoutOnMinusZero)) {
+        DeoptimizeIf(zero, instr->environment());
+      }
       __ jmp(&done, Label::kNear);
     }
-    __ bind(&positive_dividend);
-    __ and_(dividend, divisor - 1);
+
+    __ bind(&left_is_not_negative);
+    __ and_(left_reg, divisor - 1);
     __ bind(&done);
-  } else {
-    Label done, remainder_eq_dividend, slow, both_positive;
+
+  } else if (hmod->has_fixed_right_arg()) {
     Register left_reg = ToRegister(instr->left());
+    ASSERT(left_reg.is(ToRegister(instr->result())));
     Register right_reg = ToRegister(instr->right());
-    Register result_reg = ToRegister(instr->result());
 
+    int32_t divisor = hmod->fixed_right_arg_value();
+    ASSERT(IsPowerOf2(divisor));
+
+    // Check if our assumption of a fixed right operand still holds.
+    __ cmp(right_reg, Immediate(divisor));
+    DeoptimizeIf(not_equal, instr->environment());
+
+    Label left_is_not_negative, done;
+    if (left->CanBeNegative()) {
+      __ test(left_reg, Operand(left_reg));
+      __ j(not_sign, &left_is_not_negative, Label::kNear);
+      __ neg(left_reg);
+      __ and_(left_reg, divisor - 1);
+      __ neg(left_reg);
+      if (hmod->CheckFlag(HValue::kBailoutOnMinusZero)) {
+        DeoptimizeIf(zero, instr->environment());
+      }
+      __ jmp(&done, Label::kNear);
+    }
+
+    __ bind(&left_is_not_negative);
+    __ and_(left_reg, divisor - 1);
+    __ bind(&done);
+
+  } else {
+    Register left_reg = ToRegister(instr->left());
     ASSERT(left_reg.is(eax));
-    ASSERT(result_reg.is(edx));
+    Register right_reg = ToRegister(instr->right());
     ASSERT(!right_reg.is(eax));
     ASSERT(!right_reg.is(edx));
+    Register result_reg = ToRegister(instr->result());
+    ASSERT(result_reg.is(edx));
 
-    // Check for x % 0.
-    if (instr->hydrogen()->CheckFlag(HValue::kCanBeDivByZero)) {
+    Label done;
+    // Check for x % 0, idiv would signal a divide error. We have to
+    // deopt in this case because we can't return a NaN.
+    if (right->CanBeZero()) {
       __ test(right_reg, Operand(right_reg));
       DeoptimizeIf(zero, instr->environment());
     }
 
-    __ test(left_reg, Operand(left_reg));
-    __ j(zero, &remainder_eq_dividend, Label::kNear);
-    __ j(sign, &slow, Label::kNear);
-
-    __ test(right_reg, Operand(right_reg));
-    __ j(not_sign, &both_positive, Label::kNear);
-    // The sign of the divisor doesn't matter.
-    __ neg(right_reg);
-
-    __ bind(&both_positive);
-    // If the dividend is smaller than the nonnegative
-    // divisor, the dividend is the result.
-    __ cmp(left_reg, Operand(right_reg));
-    __ j(less, &remainder_eq_dividend, Label::kNear);
-
-    // Check if the divisor is a PowerOfTwo integer.
-    Register scratch = ToRegister(instr->temp());
-    __ mov(scratch, right_reg);
-    __ sub(Operand(scratch), Immediate(1));
-    __ test(scratch, Operand(right_reg));
-    __ j(not_zero, &slow, Label::kNear);
-    __ and_(left_reg, Operand(scratch));
-    __ jmp(&remainder_eq_dividend, Label::kNear);
-
-    // Slow case, using idiv instruction.
-    __ bind(&slow);
-
-    // Check for (kMinInt % -1).
-    if (instr->hydrogen()->CheckFlag(HValue::kCanOverflow)) {
-      Label left_not_min_int;
+    // Check for kMinInt % -1, idiv would signal a divide error. We
+    // have to deopt if we care about -0, because we can't return that.
+    if (left->RangeCanInclude(kMinInt) && right->RangeCanInclude(-1)) {
+      Label no_overflow_possible;
       __ cmp(left_reg, kMinInt);
-      __ j(not_zero, &left_not_min_int, Label::kNear);
+      __ j(not_equal, &no_overflow_possible, Label::kNear);
       __ cmp(right_reg, -1);
-      DeoptimizeIf(zero, instr->environment());
-      __ bind(&left_not_min_int);
+      __ j(not_equal, &no_overflow_possible, Label::kNear);
+      if (hmod->CheckFlag(HValue::kBailoutOnMinusZero)) {
+        __ test(left_reg, Operand(left_reg));

[Jakob Kummerow, 2013/06/05 17:08:41]

Why this test? You already know that left_reg == kMinInt at this point.

[Sven Panne, 2013/06/06 06:50:23]

Ooops, this is a remnant of several iterations of simplifying things here. :-/
Done.

+        DeoptimizeIf(sign, instr->environment());
+      }
+      __ Set(result_reg, Immediate(0));
+      __ jmp(&done);
+      __ bind(&no_overflow_possible);
     }
 
-    // Sign extend to edx.
+    // Sign extend dividend in eax into edx:eax.
     __ cdq();
 
-    // Check for (0 % -x) that will produce negative zero.
-    if (instr->hydrogen()->CheckFlag(HValue::kBailoutOnMinusZero)) {
+    // If we care about -0, test if the dividend is <0 and the result is 0.
+    if (left->CanBeNegative() &&
+        hmod->CanBeZero() &&
+        hmod->CheckFlag(HValue::kBailoutOnMinusZero)) {
       Label positive_left;
-      Label done;
       __ test(left_reg, Operand(left_reg));
       __ j(not_sign, &positive_left, Label::kNear);
-      __ idiv(right_reg);
 
-      // Test the remainder for 0, because then the result would be -0.
+      __ idiv(right_reg);
       __ test(result_reg, Operand(result_reg));
-      __ j(not_zero, &done, Label::kNear);
+      DeoptimizeIf(zero, instr->environment());
+      __ jmp(&done, Label::kNear);
 
-      DeoptimizeIf(no_condition, instr->environment());
       __ bind(&positive_left);
       __ idiv(right_reg);
-      __ bind(&done);
     } else {
       __ idiv(right_reg);
     }
-    __ jmp(&done, Label::kNear);
-
-    __ bind(&remainder_eq_dividend);
-    __ mov(result_reg, left_reg);
-
     __ bind(&done);
   }
 }