[aarch64-port-dev ] Unnecessary branch in MacroAssembler::corrected_idivl

[Andrew Haley]

Hi,

On 03/26/2014 01:08 AM, D.Sturm wrote:

> since I'm checking special cases, etc. in the Graal compiler generated code
> I generally also check the HotSpot code to see if it does equivalent
> things. Is it actually useful if I report small discrepances where it seems
> HotSpot generates inefficient code such as the following?
> 
> Anyhow, since we're already at it - according to the ARMv8 ISA for division:
> "If a signed integer division (INT_MIN ÷ -1) is performed, where INT_MIN is
> the most negative integer value representable in the selected register
> size, then the result will overflow the signed integer range. No indication
> of this overflow is produced and the result written to the destination
> register will be INT_MIN."
> 
> So the special case for INT_MIN / -1 in MacroAssembler::corrected_idivl is
> unnecessary since that fits exactly the JLS definition for division.

Corrected thusly.

Thanks,
Andrew.



-------------- next part --------------
# HG changeset patch
# User aph
# Date 1396275626 14400
#      Mon Mar 31 10:20:26 2014 -0400
# Node ID e176eb39c5f53127fe18a7e528ca6bc6f0c23cea
# Parent  f2658ddb105cfbc1ca275609fe71e54d852c0624
Remove special-case handling of division arguments.  AArch64 doesn't need it.

diff -r f2658ddb105c -r e176eb39c5f5 src/cpu/aarch64/vm/macroAssembler_aarch64.cpp
--- a/src/cpu/aarch64/vm/macroAssembler_aarch64.cpp	Thu Mar 27 08:02:20 2014 +0000
+++ b/src/cpu/aarch64/vm/macroAssembler_aarch64.cpp	Mon Mar 31 10:20:26 2014 -0400
@@ -1437,40 +1437,22 @@
 int MacroAssembler::corrected_idivl(Register result, Register ra, Register rb,
 				    bool want_remainder, Register scratch)
 {
-  // Full implementation of Java idiv and irem; checks for special
-  // case as described in JVM spec., p.243 & p.271.  The function
-  // returns the (pc) offset of the idivl instruction - may be needed
+  // Full implementation of Java idiv and irem.  The function
+  // returns the (pc) offset of the div instruction - may be needed
   // for implicit exceptions.
   //
   // constraint : ra/rb =/= scratch
-  //         normal case                          special case
+  //         normal case
   //
-  // input : ra: dividend                         min_long
-  //         rb: divisor                          -1
+  // input : ra: dividend
+  //         rb: divisor
   //
   // result: either
-  //         quotient  (= ra idiv rb)         min_long
-  //         remainder (= ra irem rb)         0
+  //         quotient  (= ra idiv rb)
+  //         remainder (= ra irem rb)
 
   assert(ra != scratch && rb != scratch, "reg cannot be scratch");
-  static const int64_t min_long = 0x80000000;
-  Label normal_case, special_case;
-
-  // check for special cases
-  movw(scratch, min_long);
-  cmpw(ra, scratch);
-  br(Assembler::NE, normal_case);
-  // check for -1 in rb
-  cmnw(rb, 1);
-  br(Assembler::NE, normal_case);
-  if (! want_remainder)
-    mov(result, ra);
-  else
-    mov(result, zr);
-  b(special_case);
-
-  // handle normal case
-  bind(normal_case);
+
   int idivl_offset = offset();
   if (! want_remainder) {
     sdivw(result, ra, rb);
@@ -1479,49 +1461,28 @@
     msubw(result, scratch, rb, ra);
   }
 
-  // normal and special case exit
-  bind(special_case);
-
   return idivl_offset;
 }
 
 int MacroAssembler::corrected_idivq(Register result, Register ra, Register rb,
 				    bool want_remainder, Register scratch)
 {
-  // Full implementation of Java idiv and irem; checks for special
-  // case as described in JVM spec., p.243 & p.271.  The function
-  // returns the (pc) offset of the idivq instruction - may be needed
+  // Full implementation of Java ldiv and lrem.  The function
+  // returns the (pc) offset of the div instruction - may be needed
   // for implicit exceptions.
   //
   // constraint : ra/rb =/= scratch
-  //         normal case                          special case
+  //         normal case
   //
-  // input : ra: dividend                         min_long
-  //         rb: divisor                          -1
+  // input : ra: dividend
+  //         rb: divisor
   //
   // result: either
-  //         quotient  (= ra idiv rb)         min_long
-  //         remainder (= ra irem rb)         0
+  //         quotient  (= ra idiv rb)
+  //         remainder (= ra irem rb)
 
   assert(ra != scratch && rb != scratch, "reg cannot be scratch");
-  static const int64_t min_long = 0x8000000000000000ULL;
-  Label normal_case, special_case, nonzero;
-
-  // check for special cases
-  mov(scratch, min_long);
-  cmp(ra, scratch);
-  br(Assembler::NE, normal_case);
-  // check for -1 in rb
-  cmn(rb, 1);
-  br(Assembler::NE, normal_case);
-  if (! want_remainder)
-    mov(result, ra);
-  else
-    mov(result, zr);
-  b(special_case);
-
-  // handle normal case
-  bind(normal_case);
+
   int idivq_offset = offset();
   if (! want_remainder) {
     sdiv(result, ra, rb);
@@ -1530,9 +1491,6 @@
     msub(result, scratch, rb, ra);
   }
 
-  // normal and special case exit
-  bind(special_case);
-
   return idivq_offset;
 }