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

Vladimir Kozlov vladimir.kozlov at oracle.com
Thu Aug 22 09:47:37 PDT 2013


Thank you, Goetz

The job in JPRT.

Vladimir

On 8/22/13 5:57 AM, Lindenmaier, Goetz wrote:
> 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