[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