[PATCH] bug in Assembler::mov_immediate (JDK9, aarch32)

[Xiang Yuan]

Hi,
   Our team have found a bug in Assembler::mov_immedate, JDK9 aarch32. And
it is described as belowing:

Test Case:
    [When the processor doesn’t support VFPV3]
/***************code start***************/
public class Mov_Immediate {
    public static void main(String args[]) {
        double data = 1234.56789;
        System.out.println(data);
        data = 1.0;
        System.out.println(data);
    }
}
/***************code end****************/

The result of OpenJDK9(aarch32 without VFPV3):
1234.56789
1.0028762817382812

The correct result:
1234.56789
1.0

Bug Description:
    In order to assign 1.0 to data, the bytecode dconst_1 is used. The
procedure “void TemplateTable::dconst(int value)”
calls mov(Register, uint32), and finally calls the procedure “void
Assembler::mov_immediate(Register dst, uint32_t imm32, Condition cond, bool
s)”
in “./hotspot/src/cpu/aarch32/vm/assembler_aarch32.cpp” to assign a 32bits
immediate(0x3FF00000, high 32bits of double value 1.0) to a register.
    Assigning a 32bits immediate to a register needs 2 instructions in
aarch32. This procedure tries to optimize the assignment when the immediate
is in some special forms. Such as, if the high 16bits of the immediate is
0, only one movw instruction is enough. But when the low 16bits of the
immediate are all zero, it uses a single movt instruction to do the
assignment.

/***************code start***************/
else if (!s && VM_Version::features() & (FT_ARMV7 | FT_ARMV6T2) &&
           !(imm32 & ((1 << 16) - 1))) {
        movt_i(dst, (unsigned)(imm32 >> 16), cond);
    }
/***************code end****************/

    The movt instruction moves a 16bits immediate to the high 16bits of a
register in aarch32, and keeps the low 16bits of the register unchanged.
This means that movt doesn’t set 0 to the low 16bits of the register, and
causes the wrong result.


Bug Solution:
    Aarch32 doesn’t have a single instruction that can do the assignment in
such case, and then remove these codes can fix this bug.

Bug Patch:

/***************patch start***************/
--- a/src/cpu/aarch32/vm/assembler_aarch32.cpp        Wed Dec 23 12:21:32
2015 +0000
+++ b/src/cpu/aarch32/vm/assembler_aarch32.cpp     Mon Dec 28 17:12:30 2015
+0800
@@ -1594,9 +1599,6 @@
   } else if (!s && VM_Version::features() & (FT_ARMV7 | FT_ARMV6T2) &&
              (imm32 < (1 << 16))) {
     movw_i(dst, (unsigned)imm32, cond);
-  } else if (!s && VM_Version::features() & (FT_ARMV7 | FT_ARMV6T2) &&
-             !(imm32 & ((1 << 16) - 1))) {
-    movt_i(dst, (unsigned)(imm32 >> 16), cond);
   } else { // TODO Could expand to varied numbers of mov and orrs
     //Need to do a full 32 bits
     mov_immediate32(dst, imm32, cond, s);
/***************patch end****************/

And the attachment is the test case and patch.

Best Regards!!
-------------- next part --------------
--- a/src/cpu/aarch32/vm/assembler_aarch32.cpp	Wed Dec 23 12:21:32 2015 +0000
+++ b/src/cpu/aarch32/vm/assembler_aarch32.cpp	Tue Dec 29 18:50:42 2015 +0800
@@ -1594,9 +1599,6 @@
   } else if (!s && VM_Version::features() & (FT_ARMV7 | FT_ARMV6T2) &&
              (imm32 < (1 << 16))) {
     movw_i(dst, (unsigned)imm32, cond);
-  } else if (!s && VM_Version::features() & (FT_ARMV7 | FT_ARMV6T2) &&
-             !(imm32 & ((1 << 16) - 1))) {
-    movt_i(dst, (unsigned)(imm32 >> 16), cond);
   } else { // TODO Could expand to varied numbers of mov and orrs
     //Need to do a full 32 bits
     mov_immediate32(dst, imm32, cond, s);