[1.10] RFC: Fix HotSpot regression that prevents use on x86 processors without cmov
Deepak Bhole
dbhole at redhat.com
Thu Mar 31 16:18:04 PDT 2011
* Dr Andrew John Hughes <ahughes at redhat.com> [2011-03-31 19:15]:
> This is my last patch for 1.10.
>
> This issue is discussed on the bug report:
>
> http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=682
>
> and on the HotSpot list:
>
> http://mail.openjdk.java.net/pipermail/hotspot-dev/2011-March/004006.html
>
> The discussion on the HotSpot list came to the conclusion that
> this is a regression from hs18 onwards and twisti has since provided
> a fix. This fix is already in HEAD and I've seen no negative build bot
> reports.
>
> The fix provides an alternative for cmov on x86 processors which lack this
> instruction, causing them to be usable again rather than crashing on even
> a 'java -version' invocation.
>
> Patches for both hs19 and hs20 are included. The only difference between
> the two is that hs19 doesn't need the #include directions as it still has
> IncludeDB. They are otherwise identical.
>
> Ok for 1.10?
>
Yes, this looks fine to me. Okay for commit.
Cheers,
Deepak
> 2011-03-31 Andrew John Hughes <ahughes at redhat.com>
>
> S7032388, PR682: Make HotSpot work on machines without
> cmov instruction again
> * Makefile.am: Add new patch.
> * NEWS: Mention patch.
> * patches/hotspot/hs20/7032388-work_without_cmov_instruction.patch,
> * patches/hotspot/original/7032388-work_without_cmov_instruction.patch:
> Versions of patch for each HotSpot version.
>
> --
> Andrew :)
>
> Free Java Software Engineer
> Red Hat, Inc. (http://www.redhat.com)
>
> Support Free Java!
> Contribute to GNU Classpath and IcedTea
> http://www.gnu.org/software/classpath
> http://icedtea.classpath.org
> PGP Key: F5862A37 (https://keys.indymedia.org/)
> Fingerprint = EA30 D855 D50F 90CD F54D 0698 0713 C3ED F586 2A37
> diff -r 11db21f38d4b Makefile.am
> --- a/Makefile.am Thu Mar 31 19:35:41 2011 +0100
> +++ b/Makefile.am Thu Mar 31 23:49:47 2011 +0100
> @@ -327,7 +327,8 @@
> patches/openjdk/7023591-AAShapePipe.patch \
> patches/openjdk/7027667-AAShapePipeRegTest.patch \
> patches/g356743-libpng-1.5.patch \
> - patches/mark_sun_toolkit_privileged_code.patch
> + patches/mark_sun_toolkit_privileged_code.patch \
> + patches/hotspot/$(HSBUILD)/7032388-work_without_cmov_instruction.patch
>
> if WITH_ALT_HSBUILD
> ICEDTEA_PATCHES += \
> diff -r 11db21f38d4b NEWS
> --- a/NEWS Thu Mar 31 19:35:41 2011 +0100
> +++ b/NEWS Thu Mar 31 23:49:47 2011 +0100
> @@ -15,6 +15,7 @@
> * Backports
> - S7023591, S7027667: Clipped antialiased rectangles are filled, not drawn.
> - Add missing privileged block around access to the sun.awt.nativedebug property.
> + - S7032388, PR682: Make HotSpot work on machines without cmov instruction again
> * Fixes
> - G356743: Support libpng 1.5.
> * CACAO
> diff -r 11db21f38d4b patches/hotspot/hs20/7032388-work_without_cmov_instruction.patch
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/patches/hotspot/hs20/7032388-work_without_cmov_instruction.patch Thu Mar 31 23:49:47 2011 +0100
> @@ -0,0 +1,178 @@
> +--- openjdk/hotspot/src/cpu/x86/vm/assembler_x86.cpp 2011-03-30 11:31:16.408872134 -0700
> ++++ openjdk/hotspot/src/cpu/x86/vm/assembler_x86.cpp 2011-03-30 11:31:16.084614406 -0700
> +@@ -7769,6 +7769,28 @@
> + }
> + }
> +
> ++void MacroAssembler::cmov32(Condition cc, Register dst, Address src) {
> ++ if (VM_Version::supports_cmov()) {
> ++ cmovl(cc, dst, src);
> ++ } else {
> ++ Label L;
> ++ jccb(negate_condition(cc), L);
> ++ movl(dst, src);
> ++ bind(L);
> ++ }
> ++}
> ++
> ++void MacroAssembler::cmov32(Condition cc, Register dst, Register src) {
> ++ if (VM_Version::supports_cmov()) {
> ++ cmovl(cc, dst, src);
> ++ } else {
> ++ Label L;
> ++ jccb(negate_condition(cc), L);
> ++ movl(dst, src);
> ++ bind(L);
> ++ }
> ++}
> ++
> + void MacroAssembler::verify_oop(Register reg, const char* s) {
> + if (!VerifyOops) return;
> +
> +@@ -9019,14 +9041,7 @@
> + movl(result, cnt1);
> + subl(cnt1, cnt2);
> + push(cnt1);
> +- if (VM_Version::supports_cmov()) {
> +- cmovl(Assembler::lessEqual, cnt2, result);
> +- } else {
> +- Label GT_LABEL;
> +- jccb(Assembler::greater, GT_LABEL);
> +- movl(cnt2, result);
> +- bind(GT_LABEL);
> +- }
> ++ cmov32(Assembler::lessEqual, cnt2, result);
> +
> + // Is the minimum length zero?
> + testl(cnt2, cnt2);
> +--- openjdk/hotspot/src/cpu/x86/vm/assembler_x86.hpp 2011-03-30 11:31:17.757655562 -0700
> ++++ openjdk/hotspot/src/cpu/x86/vm/assembler_x86.hpp 2011-03-30 11:31:17.553920606 -0700
> +@@ -2244,10 +2244,13 @@
> +
> + // Data
> +
> +- void cmov(Condition cc, Register dst, Register src) { LP64_ONLY(cmovq(cc, dst, src)) NOT_LP64(cmovl(cc, dst, src)); }
> ++ void cmov32( Condition cc, Register dst, Address src);
> ++ void cmov32( Condition cc, Register dst, Register src);
> +
> +- void cmovptr(Condition cc, Register dst, Address src) { LP64_ONLY(cmovq(cc, dst, src)) NOT_LP64(cmovl(cc, dst, src)); }
> +- void cmovptr(Condition cc, Register dst, Register src) { LP64_ONLY(cmovq(cc, dst, src)) NOT_LP64(cmovl(cc, dst, src)); }
> ++ void cmov( Condition cc, Register dst, Register src) { cmovptr(cc, dst, src); }
> ++
> ++ void cmovptr(Condition cc, Register dst, Address src) { LP64_ONLY(cmovq(cc, dst, src)) NOT_LP64(cmov32(cc, dst, src)); }
> ++ void cmovptr(Condition cc, Register dst, Register src) { LP64_ONLY(cmovq(cc, dst, src)) NOT_LP64(cmov32(cc, dst, src)); }
> +
> + void movoop(Register dst, jobject obj);
> + void movoop(Address dst, jobject obj);
> +--- openjdk/hotspot/src/cpu/x86/vm/c1_LIRAssembler_x86.cpp 2011-03-30 11:31:18.743456717 -0700
> ++++ openjdk/hotspot/src/cpu/x86/vm/c1_LIRAssembler_x86.cpp 2011-03-30 11:31:18.541656202 -0700
> +@@ -23,6 +23,7 @@
> + */
> +
> + #include "precompiled.hpp"
> ++#include "asm/assembler.hpp"
> + #include "c1/c1_Compilation.hpp"
> + #include "c1/c1_LIRAssembler.hpp"
> + #include "c1/c1_MacroAssembler.hpp"
> +@@ -569,24 +570,13 @@
> + __ lea (rdi, Address(rdi, rcx, Address::times_2, arrayOopDesc::base_offset_in_bytes(T_CHAR)));
> +
> + // compute minimum length (in rax) and difference of lengths (on top of stack)
> +- if (VM_Version::supports_cmov()) {
> +- __ movl (rbx, Address(rbx, java_lang_String::count_offset_in_bytes()));
> +- __ movl (rax, Address(rax, java_lang_String::count_offset_in_bytes()));
> +- __ mov (rcx, rbx);
> +- __ subptr (rbx, rax); // subtract lengths
> +- __ push (rbx); // result
> +- __ cmov (Assembler::lessEqual, rax, rcx);
> +- } else {
> +- Label L;
> +- __ movl (rbx, Address(rbx, java_lang_String::count_offset_in_bytes()));
> +- __ movl (rcx, Address(rax, java_lang_String::count_offset_in_bytes()));
> +- __ mov (rax, rbx);
> +- __ subptr (rbx, rcx);
> +- __ push (rbx);
> +- __ jcc (Assembler::lessEqual, L);
> +- __ mov (rax, rcx);
> +- __ bind (L);
> +- }
> ++ __ movl (rbx, Address(rbx, java_lang_String::count_offset_in_bytes()));
> ++ __ movl (rax, Address(rax, java_lang_String::count_offset_in_bytes()));
> ++ __ mov (rcx, rbx);
> ++ __ subptr(rbx, rax); // subtract lengths
> ++ __ push (rbx); // result
> ++ __ cmov (Assembler::lessEqual, rax, rcx);
> ++
> + // is minimum length 0?
> + Label noLoop, haveResult;
> + __ testptr (rax, rax);
> +--- openjdk/hotspot/src/cpu/x86/vm/c1_Runtime1_x86.cpp 2011-03-30 11:31:19.824124145 -0700
> ++++ openjdk/hotspot/src/cpu/x86/vm/c1_Runtime1_x86.cpp 2011-03-30 11:31:19.606167752 -0700
> +@@ -23,6 +23,7 @@
> + */
> +
> + #include "precompiled.hpp"
> ++#include "asm/assembler.hpp"
> + #include "c1/c1_Defs.hpp"
> + #include "c1/c1_MacroAssembler.hpp"
> + #include "c1/c1_Runtime1.hpp"
> +--- openjdk/hotspot/src/cpu/x86/vm/templateTable_x86_32.cpp 2011-03-30 11:31:20.910918826 -0700
> ++++ openjdk/hotspot/src/cpu/x86/vm/templateTable_x86_32.cpp 2011-03-30 11:31:20.703693030 -0700
> +@@ -23,6 +23,7 @@
> + */
> +
> + #include "precompiled.hpp"
> ++#include "asm/assembler.hpp"
> + #include "interpreter/interpreter.hpp"
> + #include "interpreter/interpreterRuntime.hpp"
> + #include "interpreter/templateTable.hpp"
> +@@ -1939,18 +1940,10 @@
> + __ movl(temp, Address(array, h, Address::times_8, 0*wordSize));
> + __ bswapl(temp);
> + __ cmpl(key, temp);
> +- if (VM_Version::supports_cmov()) {
> +- __ cmovl(Assembler::less , j, h); // j = h if (key < array[h].fast_match())
> +- __ cmovl(Assembler::greaterEqual, i, h); // i = h if (key >= array[h].fast_match())
> +- } else {
> +- Label set_i, end_of_if;
> +- __ jccb(Assembler::greaterEqual, set_i); // {
> +- __ mov(j, h); // j = h;
> +- __ jmp(end_of_if); // }
> +- __ bind(set_i); // else {
> +- __ mov(i, h); // i = h;
> +- __ bind(end_of_if); // }
> +- }
> ++ // j = h if (key < array[h].fast_match())
> ++ __ cmov32(Assembler::less , j, h);
> ++ // i = h if (key >= array[h].fast_match())
> ++ __ cmov32(Assembler::greaterEqual, i, h);
> + // while (i+1 < j)
> + __ bind(entry);
> + __ leal(h, Address(i, 1)); // i+1
> +@@ -3478,22 +3471,14 @@
> +
> + // find a free slot in the monitor block (result in rdx)
> + { Label entry, loop, exit;
> +- __ movptr(rcx, monitor_block_top); // points to current entry, starting with top-most entry
> +- __ lea(rbx, monitor_block_bot); // points to word before bottom of monitor block
> ++ __ movptr(rcx, monitor_block_top); // points to current entry, starting with top-most entry
> ++
> ++ __ lea(rbx, monitor_block_bot); // points to word before bottom of monitor block
> + __ jmpb(entry);
> +
> + __ bind(loop);
> + __ cmpptr(Address(rcx, BasicObjectLock::obj_offset_in_bytes()), (int32_t)NULL_WORD); // check if current entry is used
> +-
> +-// TODO - need new func here - kbt
> +- if (VM_Version::supports_cmov()) {
> +- __ cmov(Assembler::equal, rdx, rcx); // if not used then remember entry in rdx
> +- } else {
> +- Label L;
> +- __ jccb(Assembler::notEqual, L);
> +- __ mov(rdx, rcx); // if not used then remember entry in rdx
> +- __ bind(L);
> +- }
> ++ __ cmovptr(Assembler::equal, rdx, rcx); // if not used then remember entry in rdx
> + __ cmpptr(rax, Address(rcx, BasicObjectLock::obj_offset_in_bytes())); // check if current entry is for same object
> + __ jccb(Assembler::equal, exit); // if same object then stop searching
> + __ addptr(rcx, entry_size); // otherwise advance to next entry
> diff -r 11db21f38d4b patches/hotspot/original/7032388-work_without_cmov_instruction.patch
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/patches/hotspot/original/7032388-work_without_cmov_instruction.patch Thu Mar 31 23:49:47 2011 +0100
> @@ -0,0 +1,156 @@
> +diff -Nru openjdk.orig/hotspot/src/cpu/x86/vm/assembler_x86.cpp openjdk/hotspot/src/cpu/x86/vm/assembler_x86.cpp
> +--- openjdk.orig/hotspot/src/cpu/x86/vm/assembler_x86.cpp 2011-02-28 16:03:14.000000000 +0000
> ++++ openjdk/hotspot/src/cpu/x86/vm/assembler_x86.cpp 2011-03-31 03:28:54.031901634 +0100
> +@@ -7643,6 +7643,28 @@
> + }
> + }
> +
> ++void MacroAssembler::cmov32(Condition cc, Register dst, Address src) {
> ++ if (VM_Version::supports_cmov()) {
> ++ cmovl(cc, dst, src);
> ++ } else {
> ++ Label L;
> ++ jccb(negate_condition(cc), L);
> ++ movl(dst, src);
> ++ bind(L);
> ++ }
> ++}
> ++
> ++void MacroAssembler::cmov32(Condition cc, Register dst, Register src) {
> ++ if (VM_Version::supports_cmov()) {
> ++ cmovl(cc, dst, src);
> ++ } else {
> ++ Label L;
> ++ jccb(negate_condition(cc), L);
> ++ movl(dst, src);
> ++ bind(L);
> ++ }
> ++}
> ++
> + void MacroAssembler::verify_oop(Register reg, const char* s) {
> + if (!VerifyOops) return;
> +
> +@@ -8559,14 +8581,7 @@
> + movl(result, cnt1);
> + subl(cnt1, cnt2);
> + push(cnt1);
> +- if (VM_Version::supports_cmov()) {
> +- cmovl(Assembler::lessEqual, cnt2, result);
> +- } else {
> +- Label GT_LABEL;
> +- jccb(Assembler::greater, GT_LABEL);
> +- movl(cnt2, result);
> +- bind(GT_LABEL);
> +- }
> ++ cmov32(Assembler::lessEqual, cnt2, result);
> +
> + // Is the minimum length zero?
> + testl(cnt2, cnt2);
> +diff -Nru openjdk.orig/hotspot/src/cpu/x86/vm/assembler_x86.hpp openjdk/hotspot/src/cpu/x86/vm/assembler_x86.hpp
> +--- openjdk.orig/hotspot/src/cpu/x86/vm/assembler_x86.hpp 2011-02-28 16:03:14.000000000 +0000
> ++++ openjdk/hotspot/src/cpu/x86/vm/assembler_x86.hpp 2011-03-31 03:28:54.031901634 +0100
> +@@ -2174,10 +2174,13 @@
> +
> + // Data
> +
> +- void cmov(Condition cc, Register dst, Register src) { LP64_ONLY(cmovq(cc, dst, src)) NOT_LP64(cmovl(cc, dst, src)); }
> ++ void cmov32( Condition cc, Register dst, Address src);
> ++ void cmov32( Condition cc, Register dst, Register src);
> +
> +- void cmovptr(Condition cc, Register dst, Address src) { LP64_ONLY(cmovq(cc, dst, src)) NOT_LP64(cmovl(cc, dst, src)); }
> +- void cmovptr(Condition cc, Register dst, Register src) { LP64_ONLY(cmovq(cc, dst, src)) NOT_LP64(cmovl(cc, dst, src)); }
> ++ void cmov( Condition cc, Register dst, Register src) { cmovptr(cc, dst, src); }
> ++
> ++ void cmovptr(Condition cc, Register dst, Address src) { LP64_ONLY(cmovq(cc, dst, src)) NOT_LP64(cmov32(cc, dst, src)); }
> ++ void cmovptr(Condition cc, Register dst, Register src) { LP64_ONLY(cmovq(cc, dst, src)) NOT_LP64(cmov32(cc, dst, src)); }
> +
> + void movoop(Register dst, jobject obj);
> + void movoop(Address dst, jobject obj);
> +diff -Nru openjdk.orig/hotspot/src/cpu/x86/vm/c1_LIRAssembler_x86.cpp openjdk/hotspot/src/cpu/x86/vm/c1_LIRAssembler_x86.cpp
> +--- openjdk.orig/hotspot/src/cpu/x86/vm/c1_LIRAssembler_x86.cpp 2011-02-28 16:03:14.000000000 +0000
> ++++ openjdk/hotspot/src/cpu/x86/vm/c1_LIRAssembler_x86.cpp 2011-03-31 03:28:54.035901697 +0100
> +@@ -559,24 +559,13 @@
> + __ lea (rdi, Address(rdi, rcx, Address::times_2, arrayOopDesc::base_offset_in_bytes(T_CHAR)));
> +
> + // compute minimum length (in rax) and difference of lengths (on top of stack)
> +- if (VM_Version::supports_cmov()) {
> +- __ movl (rbx, Address(rbx, java_lang_String::count_offset_in_bytes()));
> +- __ movl (rax, Address(rax, java_lang_String::count_offset_in_bytes()));
> +- __ mov (rcx, rbx);
> +- __ subptr (rbx, rax); // subtract lengths
> +- __ push (rbx); // result
> +- __ cmov (Assembler::lessEqual, rax, rcx);
> +- } else {
> +- Label L;
> +- __ movl (rbx, Address(rbx, java_lang_String::count_offset_in_bytes()));
> +- __ movl (rcx, Address(rax, java_lang_String::count_offset_in_bytes()));
> +- __ mov (rax, rbx);
> +- __ subptr (rbx, rcx);
> +- __ push (rbx);
> +- __ jcc (Assembler::lessEqual, L);
> +- __ mov (rax, rcx);
> +- __ bind (L);
> +- }
> ++ __ movl (rbx, Address(rbx, java_lang_String::count_offset_in_bytes()));
> ++ __ movl (rax, Address(rax, java_lang_String::count_offset_in_bytes()));
> ++ __ mov (rcx, rbx);
> ++ __ subptr(rbx, rax); // subtract lengths
> ++ __ push (rbx); // result
> ++ __ cmov (Assembler::lessEqual, rax, rcx);
> ++
> + // is minimum length 0?
> + Label noLoop, haveResult;
> + __ testptr (rax, rax);
> +diff -Nru openjdk.orig/hotspot/src/cpu/x86/vm/templateTable_x86_32.cpp openjdk/hotspot/src/cpu/x86/vm/templateTable_x86_32.cpp
> +--- openjdk.orig/hotspot/src/cpu/x86/vm/templateTable_x86_32.cpp 2011-02-28 16:03:14.000000000 +0000
> ++++ openjdk/hotspot/src/cpu/x86/vm/templateTable_x86_32.cpp 2011-03-31 03:28:54.035901697 +0100
> +@@ -1931,18 +1931,10 @@
> + __ movl(temp, Address(array, h, Address::times_8, 0*wordSize));
> + __ bswapl(temp);
> + __ cmpl(key, temp);
> +- if (VM_Version::supports_cmov()) {
> +- __ cmovl(Assembler::less , j, h); // j = h if (key < array[h].fast_match())
> +- __ cmovl(Assembler::greaterEqual, i, h); // i = h if (key >= array[h].fast_match())
> +- } else {
> +- Label set_i, end_of_if;
> +- __ jccb(Assembler::greaterEqual, set_i); // {
> +- __ mov(j, h); // j = h;
> +- __ jmp(end_of_if); // }
> +- __ bind(set_i); // else {
> +- __ mov(i, h); // i = h;
> +- __ bind(end_of_if); // }
> +- }
> ++ // j = h if (key < array[h].fast_match())
> ++ __ cmov32(Assembler::less , j, h);
> ++ // i = h if (key >= array[h].fast_match())
> ++ __ cmov32(Assembler::greaterEqual, i, h);
> + // while (i+1 < j)
> + __ bind(entry);
> + __ leal(h, Address(i, 1)); // i+1
> +@@ -3463,22 +3455,14 @@
> +
> + // find a free slot in the monitor block (result in rdx)
> + { Label entry, loop, exit;
> +- __ movptr(rcx, monitor_block_top); // points to current entry, starting with top-most entry
> +- __ lea(rbx, monitor_block_bot); // points to word before bottom of monitor block
> ++ __ movptr(rcx, monitor_block_top); // points to current entry, starting with top-most entry
> ++
> ++ __ lea(rbx, monitor_block_bot); // points to word before bottom of monitor block
> + __ jmpb(entry);
> +
> + __ bind(loop);
> + __ cmpptr(Address(rcx, BasicObjectLock::obj_offset_in_bytes()), (int32_t)NULL_WORD); // check if current entry is used
> +-
> +-// TODO - need new func here - kbt
> +- if (VM_Version::supports_cmov()) {
> +- __ cmov(Assembler::equal, rdx, rcx); // if not used then remember entry in rdx
> +- } else {
> +- Label L;
> +- __ jccb(Assembler::notEqual, L);
> +- __ mov(rdx, rcx); // if not used then remember entry in rdx
> +- __ bind(L);
> +- }
> ++ __ cmovptr(Assembler::equal, rdx, rcx); // if not used then remember entry in rdx
> + __ cmpptr(rax, Address(rcx, BasicObjectLock::obj_offset_in_bytes())); // check if current entry is for same object
> + __ jccb(Assembler::equal, exit); // if same object then stop searching
> + __ addptr(rcx, entry_size); // otherwise advance to next entry
More information about the distro-pkg-dev
mailing list