Hi Anthony, I investigated yet a bit more, and finally am sure about the underlying cause: Standard gcc does not define __MMX__, but ours does, perhaps because it has been configured to make MMX support the default.
$ gcc -E -dM main.c | grep __MMX__ $ /usr/local/google/fauxJDKCompiler/gcc -E -dM main.c | grep __MMX__ #define __MMX__ 1 By default, pngconf.h uses mmx instructions if __MMX__ is defined. But this can be disabled by -DPNG_NO_MMX_CODE. Since the existing code in OpenJDK depends on MMX not being used (since pnggccrd.c is never compiled) -DPNG_NO_MMX_CODE should be added to CPPFLAGS in the splashscreen makefile unconditionally. Requesting permission to push... You could file a new bug, or we could reuse 6613927, at your option. Martin # HG changeset patch # User martin # Date 1219960566 25200 # Node ID 198b5e82cd0fa4b42cafabce3e763e2a2cb3888c # Parent 1267605489211c6c162bb246f653759e933bd06e 6613927: Compilation of splashscreen png library failed on Ubuntu 7.04 (64bit) Summary: Define -DPNG_NO_MMX_CODE unconditionally, not just on 64-bit Linux Reviewed-by: anthony diff --git a/make/sun/splashscreen/Makefile b/make/sun/splashscreen/Makefile --- a/make/sun/splashscreen/Makefile +++ b/make/sun/splashscreen/Makefile @@ -85,13 +85,6 @@ CPPFLAGS += -I$(PLATFORM_SRC)/native/$(PKGDIR)/splashscreen -I$(SHARE_SRC)/native/$(PKGDIR)/splashscreen CPPFLAGS += -I$(SHARE_SRC)/native/$(PKGDIR)/image/jpeg -I$(SHARE_SRC)/native/java/util/zip/zlib-1.1.3 -ifeq ($(PLATFORM), linux) - ifeq ($(ARCH_DATA_MODEL), 64) - # 64-bit gcc has problems compiling MMX instructions. - # Google it for more details. Possibly the newer versions of - # the PNG-library and/or the new compiler will not need this - # option in the future. - CPPFLAGS += -DPNG_NO_MMX_CODE - endif -endif - +# Shun the less than portable MMX assembly code in pnggccrd.c, +# and use alternative implementations in C. +CPPFLAGS += -DPNG_NO_MMX_CODE On Thu, Aug 28, 2008 at 9:33 AM, Martin Buchholz <[EMAIL PROTECTED]> wrote: > I'm thinking: > - the MMX support is in pnggccrd.c, > - but that file is never compiled in OpenJDK > - so... how could png ever work when PNG_NO_MMX_CODE is not defined, > on any platform? > > I am suspicious that png splashscreens are actually broken > without my proposed patch. Has this actually been tested? > > I googled a little and have a theory. > "-z defs" is supposed to be ignored for shared libs > (but why are we using it to build libsplashscreen.so?) > but very recent gcc toolchains don't ignore it the way they > perhaps should. > > E.g. Here's a debian bug report > http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=489142 > On hppa -Wl,-z,defs is not ignored for shared libraries > > But using "-z defs" is actually good engineering practice, > since it catches problems at link time instead of run time. > Whether or not it's a bug in my toolchain, my toolchain > detects the unsatisfied symbols in the link. > There's a good chance that my change will be necessary > to build OpenJDK on future stock Linux systems. > > Martin > > > On Thu, Aug 28, 2008 at 3:11 AM, Anthony Petrov <[EMAIL PROTECTED]> wrote: >> Hello Martin, >> >> Thank you for the patch. Though there's a little concern: how does the bare >> libpng deal with this issue when compiled using the toolchain you use? I >> mean, the problem is not reproducible when compiling on a "regular" 32-bit >> system, so it might be important to identify the root cause of this issue. >> Could you please take a look at the libpng source (the version that was >> forked for the purposes of OpenJDK is 1.2.18) and try compiling it with your >> tools (by the way, what are they?)? If this is the problem with this >> particular version of libpng, and newer versions do not experience this >> problem, then this might be probably a justification for updating our >> version of libpng rather than disabling the usage of MMX for all platforms. >> What do you think? >> >> -- >> best regards, >> Anthony >> >> >> On 08/28/2008 02:23 AM Martin Buchholz wrote: >>> >>> Anthony, >>> >>> Thanks for your patience; you were entirely right, >>> it was indeed necessary to define PNG_NO_MMX_CODE. >>> Since pnggccrd.c is never compiled, and it contains the MMX code, >>> the only correct thing appears to be to define this macro unconditionally. >>> The obvious patch follows (tested on 32-bit Ubuntu Linux with >>> very recent gcc toolchain) >>> >>> (Has anyone actually tested png splashscreen images on 32 or 64-bit >>> Linux?) >>> >>> # HG changeset patch >>> # User martin >>> # Date 1219875308 25200 >>> # Node ID 86b160e4bc1aae927aa71dfd712bb2365ebc0e1c >>> # Parent 1267605489211c6c162bb246f653759e933bd06e >>> 6613927: Compilation of splashscreen png library failed on Ubuntu 7.04 >>> (64bit) >>> Summary: Define -DPNG_NO_MMX_CODE unconditionally, not just on 64-bit >>> Linux >>> Reviewed-by: anthony >>> >>> diff --git a/make/sun/splashscreen/Makefile >>> b/make/sun/splashscreen/Makefile >>> --- a/make/sun/splashscreen/Makefile >>> +++ b/make/sun/splashscreen/Makefile >>> @@ -85,13 +85,6 @@ >>> CPPFLAGS += -I$(PLATFORM_SRC)/native/$(PKGDIR)/splashscreen >>> -I$(SHARE_SRC)/native/$(PKGDIR)/splashscreen >>> CPPFLAGS += -I$(SHARE_SRC)/native/$(PKGDIR)/image/jpeg >>> -I$(SHARE_SRC)/native/java/util/zip/zlib-1.1.3 >>> >>> -ifeq ($(PLATFORM), linux) >>> - ifeq ($(ARCH_DATA_MODEL), 64) >>> - # 64-bit gcc has problems compiling MMX instructions. >>> - # Google it for more details. Possibly the newer versions of >>> - # the PNG-library and/or the new compiler will not need this >>> - # option in the future. >>> - CPPFLAGS += -DPNG_NO_MMX_CODE >>> - endif >>> -endif >>> - >>> +# Shun the less than portable MMX assembly code in pnggccrd.c. >>> +# Newer versions of the PNG-library may not need this option. >>> +CPPFLAGS += -DPNG_NO_MMX_CODE >>> >>> Martin >>> >>> On Wed, Aug 27, 2008 at 5:48 AM, Anthony Petrov <[EMAIL PROTECTED]> >>> wrote: >>>> >>>> Hi Martin, >>>> >>>> On 08/25/2008 08:17 PM Martin Buchholz wrote: >>>>> >>>>> Thanks for the link, but... >>>>> - The fix for 6613927 is applied in my workspace >>>> >>>> And it gets activated when building on a 64-bit system only. Could you >>>> please take a look at the make/sun/splashscreen/Makefile file and make >>>> sure >>>> it defines the "CPPFLAGS += -DPNG_NO_MMX_CODE" in every case. It is >>>> really >>>> very interesting if this will resolve the issue. Thank you in advance. >>> >> >
