[aarch64-port-dev ] Another reproducer for AArch64 HS

Andrew Haley aph at redhat.com
Thu Sep 12 08:02:21 PDT 2013


On 09/11/2013 04:52 PM, Pavel Tisnovsky wrote:
> Hi Andrew,
> 
> here's another reproducer for AArch64 HS. The reproducer itself looks complicated, but the problem is
> in the following statement:
> 
> boolean b = (new Double(n)).equals(new Double(-0.0));
> 
> which should return false when n==0.0
> 
> The code breaks (starts to return true) after the method myFormat() is JITted.
> 
> Here you can see which values is passed to Double.doubleToLongBits() from Double.equals():
> 
> iter#: 0 value: 0.0...
> doubleToLongBits input 1: -9223372036854775808 (-0.0)
> doubleToLongBits input 2: 0 (0.0)
> false
> 
> iter#: 1 value: 0.0...
> doubleToLongBits input 1: -9223372036854775808 (-0.0)
> doubleToLongBits input 2: 0 (0.0)
> false
> 
> (JIT just compiled the method myFormat()...)
> 
> iter#: 8 value: 0.0...
> doubleToLongBits input 1: 0  <- wrong, should be -0.0 ie in int representation -9223372036854775808
> doubleToLongBits input 2: 0 (0.0)
> true
> 
> 
> I've tried to understand what's wrong (to teach myself AArch64 assembly and JIT internals ;)
> 
> Double.equals() looks like:
> 807    public boolean equals(Object obj) {
> 808        return (obj instanceof Double)
> 809               && (doubleToLongBits(((Double)obj).value) ==
> 810                      doubleToLongBits(value));
> 811    }
> 
> It's bytecode is easy to understand:
>   public boolean equals(java.lang.Object);
>     Code:
>        0: aload_1
>        1: instanceof    #38                 // class java/lang/Double
>        4: ifeq          32
>        7: aload_1
>        8: checkcast     #38                 // class java/lang/Double
>       11: getfield      #49                 // Field value:D
>       14: invokestatic  #19                 // Method doubleToLongBits:(D)J
>       17: aload_0
>       18: getfield      #49                 // Field value:D
>       21: invokestatic  #19                 // Method doubleToLongBits:(D)J
>       24: lcmp
>       25: ifne          32
>       28: iconst_1
>       29: goto          33
>       32: iconst_0
>       33: ireturn
> 
> JITed bytecode is still IMHO ok (don't know exactly the target for those bl/call instructions):
> 
> /jck/7/jdk/bin/java -XX:+UnlockDiagnosticVMOptions -XX:+PrintCompilation -XX:+PrintAssembly FormatTests
> 
>   0x00007fedd9a6b00c: ldr       d1, [x0,#16]    ;*getfield value
>                                                 ; - java.lang.Double::equals at 11 (line 808)
>                                                 ; - FormatTests::myFormat at 18 (line 229)
> 
>   0x00007fedd9a6b010: fmov      d0, d1          ;*invokestatic doubleToLongBits
>                                                 ; - java.lang.Double::equals at 14 (line 809)
>                                                 ; - FormatTests::myFormat at 18 (line 229)
> 
>   0x00007fedd9a6b014: bl        0x00007fedd9948060  ; OopMap{[176]=Oop [184]=Oop [192]=Oop off=184}
>                                                 ;*invokestatic doubleToLongBits
>                                                 ; - java.lang.Double::equals at 14 (line 809)
>                                                 ; - FormatTests::myFormat at 18 (line 229)
>                                                 ;   {static_call}
>   0x00007fedd9a6b018: ldr       x1, [sp,#176]
>   0x00007fedd9a6b01c: ldr       d0, [x1,#16]    ;*getfield value
>                                                 ; - java.lang.Double::equals at 18 (line 809)
>                                                 ; - FormatTests::myFormat at 18 (line 229)
> 
>   0x00007fedd9a6b020: str       x0, [sp,#200]
>   0x00007fedd9a6b024: bl        0x00007fedd9948060  ; OopMap{[184]=Oop [192]=Oop off=200}
>                                                 ;*invokestatic doubleToLongBits
>                                                 ; - java.lang.Double::equals at 21 (line 810)
>                                                 ; - FormatTests::myFormat at 18 (line 229)
>                                                 ;   {static_call}
>   0x00007fedd9a6b028: ldr       x2, [sp,#200]
>   0x00007fedd9a6b02c: cmp       x2, x0
> 
>   0x00007fedd9a6b030: b.ne      0x00007fedd9a6b03c  ;*lcmp
>                                                 ; - java.lang.Double::equals at 24 (line 810)
>                                                 ; - FormatTests::myFormat at 18 (line 229)
>   0x00007fedd9a6b034: orr       w2, wzr, #0x1
>   ;;   64 branch [AL] [B37]
>   0x00007fedd9a6b038: b 0x00007fedd9a6b040      ;*goto
>                                                 ; - java.lang.Double::equals at 29 (line 810)
>                                                 ; - FormatTests::myFormat at 18 (line 229)
> 
>   ;;  block B35 [32, 33]
> 
>   ;; 0x0
>   0x00007fedd9a6b03c: movz      w2, #0x0, lsl #16  ;*ireturn
>                                                 ; - java.lang.Double::equals at 33 (line 810)
>                                                 ; - FormatTests::myFormat at 18 (line 229)
> 
> I'd like to know more how to debug/trace AArm64 code :-) is it possible (how?) to see the JITed
> code of Double.equals() and the code for double autoboxing (I suppose there would be problem)?

Double.equals() was interpreted.  The killer code is the second call to Double.<init>
on Line 235:

  ;;  176 move [metadata:0x0|] [c_rarg3|] [patch_normal] [bci:55]
  0x00007fffee530160: b	0x00007fffee5311e4      ;   {section_word}
  ;;  178 branch [AL] [NewInstanceStub: 0xbca441e8] [bci:55]
  0x00007fffee530164: b	0x00007fffee5311ec      ;*new  ; - FormatTests::myFormat at 55 (line 235)

  0x00007fffee530168: fmov	d1, d0
  0x00007fffee53016c: fmov	d0, d1
  0x00007fffee530170: mov	x1, x0          ;*invokespecial <init>
                                                ; - FormatTests::myFormat at 60 (line 235)

  0x00007fffee530174: str	x0, [sp,#232]
  0x00007fffee530178: bl	0x00007fffee147c20  ; OopMap{[192]=Oop [184]=Oop [216]=Oop [232]=Oop off=956}
                                                ;*invokespecial <init>
                                                ; - FormatTests::myFormat at 60 (line 235)
                                                ;   {optimized virtual_call}
  ;;  190 move [metadata:0x0|] [c_rarg3|] [patch_normal] [bci:63]
  0x00007fffee53017c: b	0x00007fffee531200      ;   {section_word}
  ;;  192 branch [AL] [NewInstanceStub: 0xbca44828] [bci:63]
  0x00007fffee530180: b	0x00007fffee531208      ;*new  ; - FormatTests::myFormat at 63 (line 235)

  0x00007fffee530184: fmov	d0, xzr

HERE IS THE BUG ^^^^^^^^^^^^^^^^^^^^^^^^^^


  0x00007fffee530188: mov	x1, x0          ;*invokespecial <init>
                                                ; - FormatTests::myFormat at 70 (line 235
  0x00007fffee53018c: str	x0, [sp,#248]
  0x00007fffee530190: bl	0x00007fffee147c20  ; OopMap{[192]=Oop [184]=Oop [216]=Oop [232]=Oop [248]=Oop off=980}
                                                ;*invokespecial <init>
                                                ; - FormatTests::myFormat at 70 (line 235)
                                                ;   {optimized virtual_call}

Note that we're using XZR as the source for an argument that's -0.0.  This is wrong:
XZR is the bit pattern for all zeroes, 0.0.

The real cause of the bug is here:

bool Assembler::operand_valid_for_float_immediate(double imm) {
  union {
    unsigned ival;
    float val;
  };
  val = (float)imm;
  unsigned result = fp_immediate_for_encoding(ival, true);
  return encoding_for_fp_immediate(result) == imm;
}

This takes a double, tries to encode it as a floating-point immediate, then
decodes the value and makes sure it's the same.  BUT -- that test is wrong.
encoding_for_fp_immediate(result) returns 0.0 when imm is -0.0.  And the
test then does

   0.0 == -0.0

which returns true.

Here's the fix:

bool Assembler::operand_valid_for_float_immediate(double imm) {
  // We try to encode the immediate value then we convert the encoded
  // back and make sure it's the exact same bit pattern

  unsigned result = fp_immediate_for_encoding(floatToIntBits(imm), true);
  return doubleToLongBits(imm)
    == doubleToLongBits(encoding_for_fp_immediate(result));
}

Andrew.



# HG changeset patch
# User aph
# Date 1378997906 -3600
# Node ID 5b2374dbfc861f06fba6f60be6e0e4555710eb38
# Parent  87e7a0585407aecaa36dda94bf71559d1f62afbc
Fix operand_valid_for_float_immediate so that it returns false for negative zero.

diff -r 87e7a0585407 -r 5b2374dbfc86 src/cpu/aarch64/vm/assembler_aarch64.cpp
--- a/src/cpu/aarch64/vm/assembler_aarch64.cpp	Thu Sep 12 14:37:30 2013 +0100
+++ b/src/cpu/aarch64/vm/assembler_aarch64.cpp	Thu Sep 12 15:58:26 2013 +0100
@@ -1480,14 +1480,33 @@
   return encode_logical_immediate(is32, imm) != 0xffffffff;
 }

+static jlong doubleToLongBits(jdouble d) {
+  union {
+    jdouble double_value;
+    jlong double_bits;
+  };
+
+  double_value = d;
+  return double_bits;
+}
+
+static jint floatToIntBits(jfloat f) {
+  union {
+    jfloat float_value;
+    jint float_bits;
+  };
+
+  float_value = f;
+  return float_bits;
+}
+
 bool Assembler::operand_valid_for_float_immediate(double imm) {
-  union {
-    unsigned ival;
-    float val;
-  };
-  val = (float)imm;
-  unsigned result = fp_immediate_for_encoding(ival, true);
-  return encoding_for_fp_immediate(result) == imm;
+  // We try to encode the immediate value then we convert the encoded
+  // back and make sure it's the exact same bit pattern
+
+  unsigned result = fp_immediate_for_encoding(floatToIntBits(imm), true);
+  return doubleToLongBits(imm)
+    == doubleToLongBits(encoding_for_fp_immediate(result));
 }

 int AbstractAssembler::code_fill_byte() {



More information about the aarch64-port-dev mailing list