714201 - IonMonkey: JSOP_MOD support for ARM

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Sean Stangl [:sstangl]]

We need integer JSOP_MOD support on ARM to run the crypto-md5 benchmark.

Comment 1

[Eddy Bruel [:ejpbruel]]

Since I've implements JSOP_MOD on x86 and x64, and would like to learn about ARM, I hope it's ok if I take this one as well?

Comment 2

[Marty Rosenberg [:mjrosenb]]

Go for it.  Just a word or two of warning:
JSOP_DIV on arm is just a call to a glibc function.  This function may or may not also store the
modulus
The call to the glibc function is unchecked.  X/0 will throw an assertion, and INT_MIN/-1 will do something that we don't want it to do (return INT_MIN, I believe)

Comment 3

[Marty Rosenberg [:mjrosenb]]

Attachment: c+p of LDivI

This at least implements the integer part of JSOP_MOD for arm

Comment 4

[Jacob Bramley [:jbramley]]

Comment on attachment 593659 [details] [diff] [review]
c+p of LDivI

Review of attachment 593659 [details] [diff] [review]:
-----------------------------------------------------------------

There's a problem with __aeabi_idiv, but the fix is trivial. With that fix, the patch is fine.

::: js/src/ion/arm/CodeGenerator-arm.cpp
@@ +504,5 @@
>      return true;
>  }
>  
>  bool
> +CodeGeneratorARM::visitModI(LModI *ins)

__aeabi_idiv _almost_ returns the modulo in r3, but it doesn't have the correct sign in some cases. Specifically, the relationship "dividend = quotient * divisor + remainder" does not hold if the dividend is negative. Instead, you should use __aeabi_idivmod (returning the quotient in r0 and the remainder in r1). Results from a simple C test:

C operators:
   7/ 3 =  2,  7/ 3 =  1         7 = (2 * 3) + 1
  -7/ 3 = -2, -7/ 3 = -1         -7 = (-2 * 3) + -1
   7/-3 = -2,  7/-3 =  1         7 = (-2 * -3) + 1
  -7/-3 =  2, -7/-3 = -1         -7 = (2 * -3) + -1
__aeabi_idiv:
   7/ 3 =  2,  7/ 3 =  1         7 = (2 * 3) + 1
  -7/ 3 = -2, -7/ 3 =  1  ERROR: -7 != (-2 * 3) +  1
   7/-3 = -2,  7/-3 =  1         7 = (-2 * -3) + 1
  -7/-3 =  2, -7/-3 =  1  ERROR: -7 != (2 * -3) +  1
__aeabi_idivmod:
   7/ 3 =  2,  7/ 3 =  1         7 = (2 * 3) + 1
  -7/ 3 = -2, -7/ 3 = -1         -7 = (-2 * 3) + -1
   7/-3 = -2,  7/-3 =  1         7 = (-2 * -3) + 1
  -7/-3 =  2, -7/-3 = -1         -7 = (2 * -3) + -1

Note that __aeabi_idiv seems to be fine for visitDivI, where the sign of the remainder is not important.

::: js/src/ion/arm/MacroAssembler-arm.cpp
@@ +851,5 @@
>  
>  RelocBranchStyle
>  b_type()
>  {
> +    return MOVWT;

This change looks unrelated.