<AWT Dev> splashscreen.so is missing pnggccrd.c
martinrb at google.com
Thu Aug 28 14:58:21 PDT 2008
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.
# 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
diff --git a/make/sun/splashscreen/Makefile b/make/sun/splashscreen/Makefile
@@ -85,13 +85,6 @@
CPPFLAGS += -I$(PLATFORM_SRC)/native/$(PKGDIR)/splashscreen
CPPFLAGS += -I$(SHARE_SRC)/native/$(PKGDIR)/image/jpeg
-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
+# 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 <martinrb at google.com> 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
> 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.
> On Thu, Aug 28, 2008 at 3:11 AM, Anthony Petrov <Anthony.Petrov at sun.com> 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,
>> On 08/28/2008 02:23 AM Martin Buchholz wrote:
>>> 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
>>> # HG changeset patch
>>> # User martin
>>> # Date 1219875308 25200
>>> # Node ID 86b160e4bc1aae927aa71dfd712bb2365ebc0e1c
>>> # Parent 1267605489211c6c162bb246f653759e933bd06e
>>> 6613927: Compilation of splashscreen png library failed on Ubuntu 7.04
>>> Summary: Define -DPNG_NO_MMX_CODE unconditionally, not just on 64-bit
>>> Reviewed-by: anthony
>>> diff --git a/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
>>> CPPFLAGS += -I$(SHARE_SRC)/native/$(PKGDIR)/image/jpeg
>>> -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
>>> +# 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
>>> On Wed, Aug 27, 2008 at 5:48 AM, Anthony Petrov <Anthony.Petrov at sun.com>
>>>> 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
>>>> it defines the "CPPFLAGS += -DPNG_NO_MMX_CODE" in every case. It is
>>>> very interesting if this will resolve the issue. Thank you in advance.
More information about the awt-dev