RFR (M): 8023033: PPC64 (part 13): basic changes for AIX

Lindenmaier, Goetz goetz.lindenmaier at sap.com
Thu Aug 22 05:57:06 PDT 2013


Hi Vladimir, David,

I added the #define __IBMCPP__ and reordered the news and deletes.

I also added a '=' we missed in 12 in os_posix.cpp.
I hope it's ok to do this in this change. 
http://cr.openjdk.java.net/~goetz/webrevs/8023033-aixShared-3/

Best regards
  Goetz.

-----Original Message-----
From: hotspot-dev-bounces at openjdk.java.net [mailto:hotspot-dev-bounces at openjdk.java.net] On Behalf Of Vladimir Kozlov
Sent: Mittwoch, 21. August 2013 19:58
Cc: 'ppc-aix-port-dev at openjdk.java.net'; 'hotspot-dev at openjdk.java.net'
Subject: Re: RFR (M): 8023033: PPC64 (part 13): basic changes for AIX

Can we add #ifdef __IBMCPP__ to your change?

  +    void* operator new [](size_t size);
  +// xlC can not compile this code with private operator delete()
  +#ifdef __IBMCPP__
  +  public:
  +#endif
       void  operator delete(void* p);
  -    void* operator new [](size_t size);

Thanks,
Vladimir

On 8/21/13 5:35 AM, Lindenmaier, Goetz wrote:
> Hi,
>
> I don't have another workaround at hand right now.
>
> The problem is that there are public destructors in subclasses of
> StackObj which
>
> has the private delete operator.
>
> I shrinked the problem to a minimal test program and addressed the issue to
>
> our IBM compiler contacts.
>
> The minimal change that makes the sources compile is
>
> --- a/src/share/vm/memory/allocation.hpp  Fri Jul 26 00:59:18 2013 +0200
>
> +++ b/src/share/vm/memory/allocation.hpp  Wed Aug 21 10:30:58 2013 +0200
>
> @@ -218,7 +218,8 @@
>
> class StackObj ALLOCATION_SUPER_CLASS_SPEC {
>
>    private:
>
>     void* operator new(size_t size);
>
> +  void* operator new [](size_t size);
>
> + public:
>
>     void  operator delete(void* p);
>
> -  void* operator new [](size_t size);
>
>     void  operator delete [](void* p);
>
> };
>
> I.e., make only the delete operators public.  VALUE_OBJ_CLASS_SPEC is
> defined empty
>
> on aix, so the fix in _ValueObj is currently not essential.
>
> I would appreciate if you can push the change with this fix, so we can
> get to the
>
> point where we can compile on aix soon. If I get a workaround like a
> pragma,
>
> I will undo this.
>
> Best regards,
>
>    Goetz.
>
> // xlC 10 and 12 can not compile this program:
>
> // "test.cpp", line 12.3: 1540-0300 (S) The "private" member
> "A::operator delete(void *)" cannot be accessed.
>
> class A {
>
> private:
>
>    void  operator delete(void* p) {};
>
> };
>
> class B: public A {
>
> public:
>
>    ~B() {}
>
> };
>
> -----Original Message-----
>
> From: hotspot-dev-bounces at openjdk.java.net
> [mailto:hotspot-dev-bounces at openjdk.java.net] On Behalf Of Vladimir Kozlov
>
> Sent: Mittwoch, 21. August 2013 00:06
>
> Cc: 'ppc-aix-port-dev at openjdk.java.net'; 'hotspot-dev at openjdk.java.net'
>
> Subject: Re: RFR (M): 8023033: PPC64 (part 13): basic changes for AIX
>
> I looked throw reviews and fount only one not answered question:
>
> On 8/15/13 5:14 PM, David Holmes wrote:
>
>>> allocation.hpp
>
>>> xlC complains:
>
>>> runtime/mutexLocker.hpp", line 192.3: 1540-0300 (S) The "private"
>
>>> member "StackObj::operator delete(void *)" cannot be accessed.
>
>>
>
>> Hmmm. So the whole point of these being private was so that they could
>
>> not be called but we had to override the use of the global operators.
>
>> The concrete implementations then give fatal errors if you do manage to
>
>> use them (impossible?). So making them public is undesirable.
>
>>
>
>> Is there some other way to resolve this? A pragma to tell xlC to ignore
>
>> the perceived problem?
>
> thanks,
>
> Vladimir
>
> On 8/19/13 9:32 AM, Vladimir Kozlov wrote:
>
>> I tested 8023033-aixShared-2 in JPRT (including builds and tests on ppc
>
>> + arm) and it passed without failures.
>
>>
>
>> Thanks,
>
>> Vladimir
>
>>
>
>> On 8/16/13 3:06 PM, Stefan Karlsson wrote:
>
>>> On 8/16/13 11:28 PM, Lindenmaier, Goetz wrote:
>
>>>>
>
>>>> Hi Stefan,
>
>>>>
>
>>>> the problem is that globalDefinitions defines __STDC_FORMAT_MACROS.
>
>>>>
>
>>>> inttypes.hpp comes in through jni.hpp, which is in both, jvm.hpp and
>
>>>>
>
>>>> globalDefinitions.hpp through globalDefinitions_<compiler>.hpp.
>
>>>>
>
>>>> If jvm.hpp comes first, inttype.hpp is added without the macro defined,
>
>>>>
>
>>>> and the print formats are missing.
>
>>>>
>
>>>> I could also define __STDC_FORMAT_MACROS in jni.hpp or the like.
>
>>>>
>
>>>> The name "globalDefinitions" somehow says that the definitions should
>
>>>> be seen
>
>>>>
>
>>>> everywhere ... so it's basically bad that the file does not end up at
>
>>>> the top of the include
>
>>>>
>
>>>> chain. Maybe I should include it in jni.hpp? or jvm.hpp?
>
>>>>
>
>>>> What do you think?
>
>>>>
>
>>>
>
>>> I see your problem.
>
>>>
>
>>> I think the most stable solution would be to add -D__STDC_FORMAT_MACROS
>
>>> to the compiler flags.
>
>>>
>
>>> But that seems out-of-scope for this change, so go ahead and use the
>
>>> reordering for now (unless someone else complains).
>
>>>
>
>>> thanks,
>
>>> StefanK
>
>>>
>
>>>> Best regards,
>
>>>>
>
>>>> Goetz.
>
>>>>
>
>>>> *From:*Stefan Karlsson [mailto:stefan.karlsson at oracle.com]
>
>>>> *Sent:* Friday, August 16, 2013 3:09 PM
>
>>>> *To:* Lindenmaier, Goetz
>
>>>> *Cc:* 'Vladimir Kozlov'; 'David Holmes';
>
>>>> 'ppc-aix-port-dev at openjdk.java.net'; 'hotspot-dev at openjdk.java.net'
>
>>>> *Subject:* Re: RFR (M): 8023033: PPC64 (part 13): basic changes for AIX
>
>>>>
>
>>>> Hi Goetz,
>
>>>>
>
>>>> On 8/16/13 2:21 PM, Lindenmaier, Goetz wrote:
>
>>>>
>
>>>>     Hi,
>
>>>>
>
>>>>
>
>>>>
>
>>>>     - I removed the throw()
>
>>>>
>
>>>>     - I removed the #ifndef in port.hpp
>
>>>>
>
>>>>     - I fixed the typeo.
>
>>>>
>
>>>>     http://cr.openjdk.java.net/~goetz/webrevs/8023033-aixShared-2/
>
>>>> <http://cr.openjdk.java.net/%7Egoetz/webrevs/8023033-aixShared-2/>
>
>>>>
>
>>>>
>
>>>>
>
>>>>     I always build without precompiled headers, the nightbuild with
>
>>>>
>
>>>>     them.
>
>>>>
>
>>>>
>
>>>> utilities/debug.hpp.udiff.html
>
>>>>
>
>>>> -#include "prims/jvm.h"
>
>>>>   #include "utilities/globalDefinitions.hpp"
>
>>>> +#include "prims/jvm.h"
>
>>>>
>
>>>> I don't think your change to debug.hpp is the correct way to solve
>
>>>> the problems you were seeing with metaspace.hpp. Swapping the files
>
>>>> just means that someone else might hit the same problem adding
>
>>>> prims/jvm.hpp to another file.
>
>>>>
>
>>>>
>
>>>> You probably have a circular include dependency somewhere in the
>
>>>> code. Could you revert the change to utilities/debug.hpp and try to
>
>>>> figure out what the real problem is?
>
>>>>
>
>>>> thanks,
>
>>>> StefanK
>
>>>>
>
>>>>
>
>>>>
>
>>>>
>
>>>>
>
>>>>
>
>>>>     Yes, there will be makefiles for aix, and the platform files.
>
>>>> tTe prototype
>
>>>>
>
>>>>     patches are here
>
>>>>
>
>>>>
>
>>>> http://hg.openjdk.java.net/ppc-aix-port/jdk8/hotspot/file/9677ba28c6d8/ppc_patches/0014_aix_make_changes.patch
>
>>>>
>
>>>>
>
>>>>
>
>>>> http://hg.openjdk.java.net/ppc-aix-port/jdk8/hotspot/file/9677ba28c6d8/ppc_patches/0015_aix_ppc_files.patch
>
>>>>
>
>>>>
>
>>>>
>
>>>>
>
>>>>     But the make change contains mostly new files, except for
>
>>>>
>
>>>>
>
>>>>
>
>>>>     --- a/make/defs.make     Tue Jul 23 21:07:11 2013 +0200
>
>>>>
>
>>>>     +++ b/make/defs.make     Tue Jul 23 22:13:05 2013 +0200
>
>>>>
>
>>>>     @@ -166,11 +166,15 @@
>
>>>>
>
>>>>         HOST := $(shell uname -n)
>
>>>>
>
>>>>       endif
>
>>>>
>
>>>>
>
>>>>
>
>>>>     -# If not SunOS, not Linux and not BSD, assume Windows
>
>>>>
>
>>>>     +# If not SunOS, not Linux not BSD and not AIX, assume Windows
>
>>>>
>
>>>>       ifneq ($(OS), Linux)
>
>>>>
>
>>>>         ifneq ($(OS), SunOS)
>
>>>>
>
>>>>           ifneq ($(OS), bsd)
>
>>>>
>
>>>>     -      OSNAME=windows
>
>>>>
>
>>>>     +      ifneq ($(OS), AIX)
>
>>>>
>
>>>>     +        OSNAME=windows
>
>>>>
>
>>>>     +      else
>
>>>>
>
>>>>     +        OSNAME=aix
>
>>>>
>
>>>>     +      endif
>
>>>>
>
>>>>           else
>
>>>>
>
>>>>             OSNAME=bsd
>
>>>>
>
>>>>           endif
>
>>>>
>
>>>>
>
>>>>
>
>>>>
>
>>>>
>
>>>>     Best regards,
>
>>>>
>
>>>>        Goetz
>
>>>>
>
>>>>
>
>>>>
>
>>>>
>
>>>>
>
>>>>
>
>>>>
>
>>>>     -----Original Message-----
>
>>>>
>
>>>>     From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
>
>>>>
>
>>>>     Sent: Friday, August 16, 2013 7:21 AM
>
>>>>
>
>>>>     To: David Holmes
>
>>>>
>
>>>>     Cc: Lindenmaier, Goetz; 'ppc-aix-port-dev at openjdk.java.net
>
>>>> <mailto:ppc-aix-port-dev at openjdk.java.net>';'hotspot-dev at openjdk.java.net
>
>>>> <mailto:hotspot-dev at openjdk.java.net>'
>
>>>>
>
>>>>     Subject: Re: RFR (M): 8023033: PPC64 (part 13): basic changes for
>
>>>> AIX
>
>>>>
>
>>>>
>
>>>>
>
>>>>     I thought trow() was added long time ago. But it was added, I
>
>>>> think by accident, very recently:
>
>>>>
>
>>>>
>
>>>>
>
>>>>     http://hg.openjdk.java.net/jdk8/jdk8/hotspot/rev/a7fb14888912
>
>>>>
>
>>>>
>
>>>>
>
>>>>     I missed it when I did the review of those changes.
>
>>>>
>
>>>>
>
>>>>
>
>>>>     We should remove throw.
>
>>>>
>
>>>>
>
>>>>
>
>>>>     Vladimir
>
>>>>
>
>>>>
>
>>>>
>
>>>>     On 8/15/13 5:14 PM, David Holmes wrote:
>
>>>>
>
>>>>         On 16/08/2013 7:54 AM, Vladimir Kozlov wrote:
>
>>>>
>
>>>>             On 8/15/13 2:32 PM, Lindenmaier, Goetz wrote:
>
>>>>
>
>>>>                 Hi Vladimir,
>
>>>>
>
>>>>
>
>>>>
>
>>>>                 throw is needed because it`s there in the
>
>>>> implementation in nmethod.cpp.
>
>>>>
>
>>>>                 (So you are using it a bit at least :))
>
>>>>
>
>>>>                 xlc says
>
>>>>
>
>>>>                 "nmethod.cpp", line 802.7: 1540-0400 (S)
>
>>>> "nmethod::operator
>
>>>>
>
>>>>                 new(size_t, int)" has a conflicting declaration.
>
>>>>
>
>>>>                 "nmethod.hpp", line 268.9: 1540-0424 (I) "operator
>
>>>> new" is declared on
>
>>>>
>
>>>>                 line 268 of "nmethod.hpp".
>
>>>>
>
>>>>
>
>>>>
>
>>>>             Okay, it is just declaration.
>
>>>>
>
>>>>
>
>>>>
>
>>>>         Why do we have throw here:
>
>>>>
>
>>>>
>
>>>>
>
>>>>         void* nmethod::operator new(size_t size, int nmethod_size)
>
>>>> throw () {
>
>>>>
>
>>>>             // Not critical, may return null if there is too little
>
>>>> continuous memory
>
>>>>
>
>>>>             return CodeCache::allocate(nmethod_size);
>
>>>>
>
>>>>         }
>
>>>>
>
>>>>
>
>>>>
>
>>>>         Seems to me it should be removed if anything.
>
>>>>
>
>>>>
>
>>>>
>
>>>>         David
>
>>>>
>
>>>>         -----
>
>>>>
>
>>>>
>
>>>>
>
>>>>
>
>>>>
>
>>>>
>
>>>>
>
>>>>                 int64 is defined correctly, uint64 is not defined,
>
>>>> but never used in
>
>>>>
>
>>>>                 hotspot.
>
>>>>
>
>>>>                 I can not reproduce an error, but that's rather old
>
>>>> coding from our VM.
>
>>>>
>
>>>>                 We also switched from xlc8 to xlc10 in the course of
>
>>>> this project.
>
>>>>
>
>>>>                 I will test some more and talk to the person who
>
>>>> implemented that
>
>>>>
>
>>>>                 tomorrow,
>
>>>>
>
>>>>                 and if possible remove the change.
>
>>>>
>
>>>>
>
>>>>
>
>>>>             Okay, I will test it also.
>
>>>>
>
>>>>
>
>>>>
>
>>>>             Vladimir
>
>>>>
>
>>>>
>
>>>>
>
>>>>
>
>>>>
>
>>>>                 Best regards & thanks for the review,
>
>>>>
>
>>>>                     Goetz.
>
>>>>
>
>>>>
>
>>>>
>
>>>>
>
>>>>
>
>>>>
>
>>>>
>
>>>>
>
>>>>
>
>>>>
>
>>>>
>
>>>>                 -----Original Message-----
>
>>>>
>
>>>>                 From: Vladimir Kozlov
>
>>>> [mailto:vladimir.kozlov at oracle.com]
>
>>>>
>
>>>>                 Sent: Thursday, August 15, 2013 5:52 PM
>
>>>>
>
>>>>                 To: Lindenmaier, Goetz
>
>>>>
>
>>>>                 Cc: 'hotspot-dev at openjdk.java.net
>
>>>> <mailto:hotspot-dev at openjdk.java.net>';ppc-aix-port-dev at openjdk.java.net
>
>>>> <mailto:ppc-aix-port-dev at openjdk.java.net>;
>
>  >>>
>
>  >>>                 Albert Noll (albert.noll at oracle.com
>
>  >>> <mailto:albert.noll at oracle.com>)
>
>>>>
>
>>>>                 Subject: Re: RFR (M): 8023033: PPC64 (part 13): basic
>
>>>> changes for AIX
>
>>>>
>
>>>>
>
>>>>
>
>>>>                 Goetz,
>
>>>>
>
>>>>
>
>>>>
>
>>>>                 I only see 2 problems which you did not explain:
>
>>>>
>
>>>>
>
>>>>
>
>>>>                 nmethod.hpp. Why the next change? we don't use C++
>
>>>> exceptions:
>
>>>>
>
>>>>
>
>>>>
>
>>>>                 -  void* operator new(size_t size, int nmethod_size);
>
>>>>
>
>>>>                 +  void* operator new(size_t size, int nmethod_size)
>
>>>> throw ();
>
>>>>
>
>>>>
>
>>>>
>
>>>>                 port.hpp. Did AIX has the same definitions for jlong
>
>>>> and julong?:
>
>>>>
>
>>>>
>
>>>>
>
>>>>                 +#ifndef _AIX
>
>>>>
>
>>>>                 +// These conflict with /usr/include/sys/inttypes.h
>
>>>> on aix.
>
>>>>
>
>>>>                     typedef jlong int64;            // Java long for
>
>>>> my 64-bit type
>
>>>>
>
>>>>                     typedef julong uint64;          // Java long for
>
>>>> my 64-bit type
>
>>>>
>
>>>>                 +#endif
>
>>>>
>
>>>>
>
>>>>
>
>>>>
>
>>>>
>
>>>>                 And of cause we need to test these changes with
>
>>>> compilers we use.
>
>>>>
>
>>>>
>
>>>>
>
>>>>                 Thanks,
>
>>>>
>
>>>>                 Vladimir
>
>>>>
>
>  >>>
>
>  >>>
>
>  >>>                 On 8/15/13 5:10 AM, Lindenmaier, Goetz wrote:
>
>>>>
>
>>>>                     Hi,
>
>>>>
>
>>>>
>
>>>>
>
>>>>                     I prepared a webrev for
>
>>>>
>
>>>>                     8023033: PPC64 (part 13): basic changes for AIX
>
>>>>
>
>>>>
>
>>>> http://cr.openjdk.java.net/~goetz/webrevs/8023033-aixShared/
>
>>>> <http://cr.openjdk.java.net/%7Egoetz/webrevs/8023033-aixShared/>
>
>>>>
>
>>>>
>
>>>>
>
>>>>                     This contains the basic shared changes needed for
>
>>>> the AIX port,
>
>>>>
>
>>>>                     as there are
>
>>>>
>
>>>>                     - #includes
>
>>>>
>
>>>>                     - Fixes to get the code compiling with xlC/on AIX
>
>>>>
>
>>>>                     - Basic adaptions as in vm_version.cpp.
>
>>>>
>
>>>>
>
>>>>
>
>>>>                     It also determines the placement and naming of
>
>>>> the aix files,
>
>>>>
>
>>>>                     which will go to os/aix and os_cpu/aix_ppc, as
>
>>>> you can see in
>
>>>>
>
>>>>
>
>>>> http://hg.openjdk.java.net/ppc-aix-port/jdk8/hotspot/file/9677ba28c6d8/src/os/aix/vm/
>
>>>>
>
>>>>
>
>>>>
>
>>>>
>
>>>>
>
>>>>
>
>>>>                     Some details about the compilation problems:
>
>>>>
>
>>>>
>
>>>>
>
>>>>                     relocInfo.hpp:
>
>>>>
>
>>>>                     xlC wants initialization in inline implementation.
>
>>>>
>
>>>>
>
>>>>
>
>>>>                     vmreg.hpp:
>
>>>>
>
>>>>                     BAD is defined in AIX system header sys/param.h.
>
>>>> Renamed.
>
>>>>
>
>>>>
>
>>>>
>
>>>>                     allocation.hpp
>
>>>>
>
>>>>                     xlC complains:
>
>>>>
>
>>>>                     runtime/mutexLocker.hpp", line 192.3: 1540-0300
>
>>>> (S) The "private"
>
>>>>
>
>>>>                     member "StackObj::operator delete(void *)" cannot
>
>>>> be accessed.
>
>>>>
>
>>>>
>
>>>>
>
>>>>                     sharedRuntimeTrig.cpp
>
>>>>
>
>>>>                     Aix defines hz to be 100, see sys/m_param.h.
>
>>>> Renamed.
>
>>>>
>
>>>>
>
>>>>
>
>>>>                     debug.hpp
>
>>>>
>
>>>>                     With other include order we get a lot of
>
>>>>
>
>>>>                     memory/metaspace.hpp", line 281.66: 1540-0130 (S)
>
>>>> "PRIuPTR" is not
>
>>>>
>
>>>>                     declared.
>
>>>>
>
>>>>
>
>>>>
>
>>>>
>
>>>>
>
>>>>                     Please review and test this change. Comments are
>
>>>> welcome.
>
>>>>
>
>>>>
>
>>>>
>
>>>>                     Thanks and best regards,
>
>  >>>
>
>  >>>                          Goetz.
>
>  >>>
>
>  >>>
>
>  >>>
>
>  >>
>


More information about the ppc-aix-port-dev mailing list