[RFC] Patch to fix register allocation (PR680)
Deepak Bhole
dbhole at redhat.com
Fri Mar 25 20:06:58 PDT 2011
* Dr Andrew John Hughes <ahughes at redhat.com> [2011-03-25 20:38]:
> On 19:25 Fri 25 Mar , Deepak Bhole wrote:
> > Hi,
> >
> > I have put all the details in PR680:
> > http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=680
> >
> > To summarize, the current inline assembly code in
> > release_store_fence(volatile jbyte* p, jbyte v) is using an incorrect
> > output operand. It works with the current gcc by pure luck, but with gcc
> > 4.6, it fails.
> >
> > Attached patches for icedtea6 and icedtea7 fix the issue. I have also
> > tested this with icedtea6+hs20 and there were no issues.
> >
> > Thanks to Jakub Jelinek for help with the issue.
> >
>
> Have you run this by the HotSpot developers?
>
> > IcedTea6 ChangeLog:
> > 2011-03-25 Deepak Bhole <dbhole at redhat.com>
> >
> > PR680: Register allocation in release_store_fence(jbyte*, jbyte) is
> > incorrect
> > * Makefile.am: Apply new pr680-gcc-register-allocation-fix.patch patch.
> > * NEWS: Updated
> > * patches/pr680-gcc-register-allocation-fix.patch: New patch. Fixes
> > register allocation by constraining output to one of a/b/c/d registers.
> >
>
> Looks fine in theory but I would like some feedback from the HotSpot developers first.
>
Posted:
http://mail.openjdk.java.net/pipermail/hotspot-dev/2011-March/003991.html
I also addressed a second xchgb call that was using =r (though it
doesn't cause failure, we might as well address it now).
> > IcedTea7 ChangeLog:
> > 2011-03-25 Deepak Bhole <dbhole at redhat.com>
> >
> > PR680: Register allocation in release_store_fence(jbyte*, jbyte) is
> > incorrect
> > * Makefile.am: Apply new icedtea-pr680-gcc-register-allocation-fix.patch
> > patch.
> > * NEWS: Updated
> > * patches/icedtea-pr680-gcc-register-allocation-fix.patch: New patch.
> > Fixes register allocation by constraining output to one of a/b/c/d
> > registers.
> >
>
> For IcedTea7, patches should go into the IcedTea forest where
> possible, not as patches in the tree. This also uses the obsolete
> 'icedtea-' prefix which has been removed on 6 and will be on 7.
>
Ah, sorry I wasn't aware that icedtea- was obsoleted.
> The most sensible route with 7 is to get it pushed into the main
> OpenJDK7 tree and then pull in the patch. This is what we do with
> Shark fixes.
>
Sure, sounds good!
Cheers,
Deepak
> > In addition to the tips, I would also recommend this for 1.10 since gcc 4.6
> > will be out soon and 1.10 will be the stable release for the next little while.
> >
> > Okay to commit?
> >
>
> No.
>
> > Cheers,
> > Deepak
>
> > diff -r 8c01fdae79ef Makefile.am
> > --- a/Makefile.am Fri Mar 25 13:44:12 2011 +0100
> > +++ b/Makefile.am Fri Mar 25 19:07:42 2011 -0400
> > @@ -331,7 +331,8 @@
> > patches/openjdk/7027667-AAShapePipeRegTest.patch \
> > patches/openjdk/7019861-AA-regression-fix.patch \
> > patches/openjdk/6768387-jtable_not_serializable.patch \
> > - patches/mark_sun_toolkit_privileged_code.patch
> > + patches/mark_sun_toolkit_privileged_code.patch \
> > + patches/pr680-gcc-register-allocation-fix.patch
> >
> > if WITH_ALT_HSBUILD
> > ICEDTEA_PATCHES += \
> > diff -r 8c01fdae79ef NEWS
> > --- a/NEWS Fri Mar 25 13:44:12 2011 +0100
> > +++ b/NEWS Fri Mar 25 19:07:42 2011 -0400
> > @@ -20,6 +20,7 @@
> > make check jtreg_checks="langtools hotspot".
> > If none is provided make check runs all testsuites.
> > - Fixed AccessControlContext which was thrown while working with Color class in a PropertyEditor
> > + - PR680: Register allocation in release_store_fence(jbyte*, jbyte) is incorrect
> > * CACAO
> > - Ignore all unknown options, but report them.
> > - Fixes build for newer gcc (at least 4.4) on PPC64, breaks older gcc.
> > diff -r 8c01fdae79ef patches/pr680-gcc-register-allocation-fix.patch
> > --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> > +++ b/patches/pr680-gcc-register-allocation-fix.patch Fri Mar 25 19:07:42 2011 -0400
> > @@ -0,0 +1,12 @@
> > +diff -ur openjdk.orig/hotspot/src/os_cpu/linux_x86/vm/orderAccess_linux_x86.inline.hpp openjdk/hotspot/src/os_cpu/linux_x86/vm/orderAccess_linux_x86.inline.hpp
> > +--- openjdk.orig/hotspot/src/os_cpu/linux_x86/vm/orderAccess_linux_x86.inline.hpp 2011-02-28 11:03:15.000000000 -0500
> > ++++ openjdk/hotspot/src/os_cpu/linux_x86/vm/orderAccess_linux_x86.inline.hpp 2011-03-25 16:15:31.461400000 -0400
> > +@@ -148,7 +148,7 @@
> > + // Must duplicate definitions instead of calling store_fence because we don't want to cast away volatile.
> > + inline void OrderAccess::release_store_fence(volatile jbyte* p, jbyte v) {
> > + __asm__ volatile ( "xchgb (%2),%0"
> > +- : "=r" (v)
> > ++ : "=q" (v)
> > + : "0" (v), "r" (p)
> > + : "memory");
> > + }
>
> > diff -r eeb7d84e7ae5 Makefile.am
> > --- a/Makefile.am Mon Mar 21 20:47:22 2011 +0000
> > +++ b/Makefile.am Fri Mar 25 19:07:13 2011 -0400
> > @@ -281,7 +281,8 @@
> > patches/parisc.patch \
> > patches/sh4-support.patch \
> > patches/jtreg-httpTest.patch \
> > - patches/icedtea-update-bootclasspath.patch
> > + patches/icedtea-update-bootclasspath.patch \
> > + patches/icedtea-pr680-gcc-register-allocation-fix.patch
> >
> > # Conditional patches
> >
> > diff -r eeb7d84e7ae5 NEWS
> > --- a/NEWS Mon Mar 21 20:47:22 2011 +0000
> > +++ b/NEWS Fri Mar 25 19:07:13 2011 -0400
> > @@ -6,6 +6,8 @@
> > - Match Shark in icedtea6, makes OSR work by removing vestigal check.
> > - LLVM 2.7 non-product fixes.
> > - Correct suffix for the llvm.atomic.cmp.swap intrinsic.
> > +* Bug fixes
> > + - PR680: Register allocation in release_store_fence(jbyte*, jbyte) is incorrect
> >
> > New in release 1.13 (2010-07-29)
> >
> > diff -r eeb7d84e7ae5 patches/icedtea-pr680-gcc-register-allocation-fix.patch
> > --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> > +++ b/patches/icedtea-pr680-gcc-register-allocation-fix.patch Fri Mar 25 19:07:13 2011 -0400
> > @@ -0,0 +1,12 @@
> > +diff -ur openjdk.orig/hotspot/src/os_cpu/linux_x86/vm/orderAccess_linux_x86.inline.hpp openjdk/hotspot/src/os_cpu/linux_x86/vm/orderAccess_linux_x86.inline.hpp
> > +--- openjdk.orig/hotspot/src/os_cpu/linux_x86/vm/orderAccess_linux_x86.inline.hpp 2011-02-28 11:03:15.000000000 -0500
> > ++++ openjdk/hotspot/src/os_cpu/linux_x86/vm/orderAccess_linux_x86.inline.hpp 2011-03-25 16:15:31.461400000 -0400
> > +@@ -148,7 +148,7 @@
> > + // Must duplicate definitions instead of calling store_fence because we don't want to cast away volatile.
> > + inline void OrderAccess::release_store_fence(volatile jbyte* p, jbyte v) {
> > + __asm__ volatile ( "xchgb (%2),%0"
> > +- : "=r" (v)
> > ++ : "=q" (v)
> > + : "0" (v), "r" (p)
> > + : "memory");
> > + }
>
>
> --
> 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
More information about the distro-pkg-dev
mailing list