PPC Linux 64 needs -fsigned-char option for gcc
Hello, We found -fsigned-char is added to ppc platform, but not added to ppc64 platform. As they are different platforms, I think it is needed for ppc64 as well. Currently I just added one line modification as follow, but there may be more places to modify. If some one can give some comments, I can make a complete webrev. The buggy scenario we found needs closed code to reproduce, so it is not reproduced with current openjdk build on ppc linux from AIX porting project. I tested with ibmjdk, the patch works. I found CFLAGS_REQUIRED_ppc is from changeset http://hg.openjdk.java.net/jdk8/jdk8/jdk/rev/54d8193f177b . Is it enough to add ppc64 option for places ppc appears in that patch? ///////////////////////// the patch //////////////////////// diff --git a/make/common/Defs-linux.gmk b/make/common/Defs-linux.gmk --- a/make/common/Defs-linux.gmk +++ b/make/common/Defs-linux.gmk @@ -196,6 +196,7 @@ LDFLAGS_COMMON_sparc += -m32 -mcpu=v9 CFLAGS_REQUIRED_arm += -fsigned-char -D_LITTLE_ENDIAN CFLAGS_REQUIRED_ppc += -fsigned-char -D_BIG_ENDIAN +CFLAGS_REQUIRED_ppc64 += -fsigned-char -D_BIG_ENDIAN ifeq ($(ZERO_BUILD), true) CFLAGS_REQUIRED = $(ZERO_ARCHFLAG) ifeq ($(ZERO_ENDIANNESS), little) -- Best Regards, Sean Chou
On 21/12/2012 09:40, Sean Chou wrote:
Hello,
We found -fsigned-char is added to ppc platform, but not added to ppc64 platform. As they are different platforms, I think it is needed for ppc64 as well. Currently I just added one line modification as follow, but there may be more places to modify. If some one can give some comments, I can make a complete webrev.
The buggy scenario we found needs closed code to reproduce, so it is not reproduced with current openjdk build on ppc linux from AIX porting project. I tested with ibmjdk, the patch works.
I found CFLAGS_REQUIRED_ppc is from changeset http://hg.openjdk.java.net/jdk8/jdk8/jdk/rev/54d8193f177b . Is it enough to add ppc64 option for places ppc appears in that patch?
///////////////////////// the patch ////////////////////////
diff --git a/make/common/Defs-linux.gmk b/make/common/Defs-linux.gmk --- a/make/common/Defs-linux.gmk +++ b/make/common/Defs-linux.gmk @@ -196,6 +196,7 @@ LDFLAGS_COMMON_sparc += -m32 -mcpu=v9 CFLAGS_REQUIRED_arm += -fsigned-char -D_LITTLE_ENDIAN CFLAGS_REQUIRED_ppc += -fsigned-char -D_BIG_ENDIAN +CFLAGS_REQUIRED_ppc64 += -fsigned-char -D_BIG_ENDIAN ifeq ($(ZERO_BUILD), true) CFLAGS_REQUIRED = $(ZERO_ARCHFLAG) ifeq ($(ZERO_ENDIANNESS), little)
You should probably bring this to build-dev to discuss. If the linux-ppc64 port is a jdk8 port then you should probably be looking at the new build and we should be cutting over soon. -Alan
Hi Sean, honestly speaking, I wasn't aware of this problem until now and I just checked that we currently don't use this option, neither internally nor in our port. I found the following nice explanation of the issue: http://www.network-theory.co.uk/docs/gccintro/gccintro_71.html It seems that you only get problems if your programs relies on the fact that 'char' is either unsigned or signed. I suppose that the current OpenJDK doesn't rely on such assumptions (which is good) because we didn't saw any of them until now. If I understand you right, you add some closed code the the JDK which has problems because it makes such assumptions. Is that right? If yes, you should probably first fix that code in the way described in the referenced document. Wouldn't that be possible? Regarding your patch: I suppose you took it against an original JDK and not our port, because in our port we already have the following lines (at least in http://hg.openjdk.java.net/ppc-aix-port/jdk7u//jdk because we haven't started to work on jdk8 until now) CFLAGS_REQUIRED_arm += -fsigned-char -D_LITTLE_ENDIAN CFLAGS_REQUIRED_ppc += -fsigned-char -D_BIG_ENDIAN CFLAGS_REQUIRED_ppc64 += -m64 LDFLAGS_COMMON_ppc64 += -m64 -L/lib64 -Wl,-melf64ppc Notice that we don't set '-D_BIG_ENDIAN' because it is the default. Didn't you observed your problems with jdk7 on Linux/PPC? I think we should patch JDK7 first if this is really necessary. Regards, Volker On Fri, Dec 21, 2012 at 10:40 AM, Sean Chou <zhouyx@linux.vnet.ibm.com>wrote:
Hello,
We found -fsigned-char is added to ppc platform, but not added to ppc64 platform. As they are different platforms, I think it is needed for ppc64 as well. Currently I just added one line modification as follow, but there may be more places to modify. If some one can give some comments, I can make a complete webrev.
The buggy scenario we found needs closed code to reproduce, so it is not reproduced with current openjdk build on ppc linux from AIX porting project. I tested with ibmjdk, the patch works.
I found CFLAGS_REQUIRED_ppc is from changeset http://hg.openjdk.java.net/jdk8/jdk8/jdk/rev/54d8193f177b . Is it enough to add ppc64 option for places ppc appears in that patch?
///////////////////////// the patch ////////////////////////
diff --git a/make/common/Defs-linux.gmk b/make/common/Defs-linux.gmk --- a/make/common/Defs-linux.gmk +++ b/make/common/Defs-linux.gmk @@ -196,6 +196,7 @@ LDFLAGS_COMMON_sparc += -m32 -mcpu=v9 CFLAGS_REQUIRED_arm += -fsigned-char -D_LITTLE_ENDIAN CFLAGS_REQUIRED_ppc += -fsigned-char -D_BIG_ENDIAN +CFLAGS_REQUIRED_ppc64 += -fsigned-char -D_BIG_ENDIAN ifeq ($(ZERO_BUILD), true) CFLAGS_REQUIRED = $(ZERO_ARCHFLAG) ifeq ($(ZERO_ENDIANNESS), little)
--
Best Regards, Sean Chou
Hi Volker, Do you have any idea about the modification considering David Holmes' comments ? On Fri, Dec 21, 2012 at 6:48 PM, Volker Simonis <volker.simonis@gmail.com>wrote:
Hi Sean,
honestly speaking, I wasn't aware of this problem until now and I just checked that we currently don't use this option, neither internally nor in our port. I found the following nice explanation of the issue: http://www.network-theory.co.uk/docs/gccintro/gccintro_71.html
It seems that you only get problems if your programs relies on the fact that 'char' is either unsigned or signed. I suppose that the current OpenJDK doesn't rely on such assumptions (which is good) because we didn't saw any of them until now.
If I understand you right, you add some closed code the the JDK which has problems because it makes such assumptions. Is that right? If yes, you should probably first fix that code in the way described in the referenced document. Wouldn't that be possible?
Regarding your patch: I suppose you took it against an original JDK and not our port, because in our port we already have the following lines (at least in http://hg.openjdk.java.net/ppc-aix-port/jdk7u//jdk because we haven't started to work on jdk8 until now)
CFLAGS_REQUIRED_arm += -fsigned-char -D_LITTLE_ENDIAN CFLAGS_REQUIRED_ppc += -fsigned-char -D_BIG_ENDIAN CFLAGS_REQUIRED_ppc64 += -m64 LDFLAGS_COMMON_ppc64 += -m64 -L/lib64 -Wl,-melf64ppc
Notice that we don't set '-D_BIG_ENDIAN' because it is the default.
Didn't you observed your problems with jdk7 on Linux/PPC? I think we should patch JDK7 first if this is really necessary.
Regards, Volker
On Fri, Dec 21, 2012 at 10:40 AM, Sean Chou <zhouyx@linux.vnet.ibm.com>wrote:
Hello,
We found -fsigned-char is added to ppc platform, but not added to ppc64 platform. As they are different platforms, I think it is needed for ppc64 as well. Currently I just added one line modification as follow, but there may be more places to modify. If some one can give some comments, I can make a complete webrev.
The buggy scenario we found needs closed code to reproduce, so it is not reproduced with current openjdk build on ppc linux from AIX porting project. I tested with ibmjdk, the patch works.
I found CFLAGS_REQUIRED_ppc is from changeset http://hg.openjdk.java.net/jdk8/jdk8/jdk/rev/54d8193f177b . Is it enough to add ppc64 option for places ppc appears in that patch?
///////////////////////// the patch ////////////////////////
diff --git a/make/common/Defs-linux.gmk b/make/common/Defs-linux.gmk --- a/make/common/Defs-linux.gmk +++ b/make/common/Defs-linux.gmk @@ -196,6 +196,7 @@ LDFLAGS_COMMON_sparc += -m32 -mcpu=v9 CFLAGS_REQUIRED_arm += -fsigned-char -D_LITTLE_ENDIAN CFLAGS_REQUIRED_ppc += -fsigned-char -D_BIG_ENDIAN +CFLAGS_REQUIRED_ppc64 += -fsigned-char -D_BIG_ENDIAN ifeq ($(ZERO_BUILD), true) CFLAGS_REQUIRED = $(ZERO_ARCHFLAG) ifeq ($(ZERO_ENDIANNESS), little)
--
Best Regards, Sean Chou
-- Best Regards, Sean Chou
One thing that would be nice to come out of this porting effort is better organisation of platform specific aspects. I would prefer to see an arch.make file for each arch rather than have all these platform specific sections. (It was fine when you only had a couple of flavours but now the balance has shifted). The JDK ppc/arm references were simply the minimum we needed for our embedded builds to work. They don't really offer proper arm/ppc support. For hotspot we rely on setting ARCH and overriding EXTRA_CFLAGS via the environment or make invocation, so these architectures are not treated as first-class citizens by hotspot. This particular example is particularly bad because the arm/ppc values are completely different to the kinds of values set for the other archs! I don't even think we continue to use these in the new build. David ---- On 21/12/2012 7:40 PM, Sean Chou wrote:
Hello,
We found -fsigned-char is added to ppc platform, but not added to ppc64 platform. As they are different platforms, I think it is needed for ppc64 as well. Currently I just added one line modification as follow, but there may be more places to modify. If some one can give some comments, I can make a complete webrev.
The buggy scenario we found needs closed code to reproduce, so it is not reproduced with current openjdk build on ppc linux from AIX porting project. I tested with ibmjdk, the patch works.
I found CFLAGS_REQUIRED_ppc is from changeset http://hg.openjdk.java.net/jdk8/jdk8/jdk/rev/54d8193f177b . Is it enough to add ppc64 option for places ppc appears in that patch?
///////////////////////// the patch ////////////////////////
diff --git a/make/common/Defs-linux.gmk b/make/common/Defs-linux.gmk --- a/make/common/Defs-linux.gmk +++ b/make/common/Defs-linux.gmk @@ -196,6 +196,7 @@ LDFLAGS_COMMON_sparc += -m32 -mcpu=v9 CFLAGS_REQUIRED_arm += -fsigned-char -D_LITTLE_ENDIAN CFLAGS_REQUIRED_ppc += -fsigned-char -D_BIG_ENDIAN +CFLAGS_REQUIRED_ppc64 += -fsigned-char -D_BIG_ENDIAN ifeq ($(ZERO_BUILD), true) CFLAGS_REQUIRED = $(ZERO_ARCHFLAG) ifeq ($(ZERO_ENDIANNESS), little)
--
Best Regards, Sean Chou
participants (4)
-
Alan Bateman
-
David Holmes
-
Sean Chou
-
Volker Simonis