Hi Matthias,

On 1/02/2019 10:36 pm, Baesken, Matthias wrote:
New webrev :

http://cr.openjdk.java.net/~mbaesken/webrevs/8218136.1/

- adjusted  globalDefinitions_xlc.hpp

I don't think it makes sense to keep the comment which was obviously copied from the gcc file:

// On Linux NULL is defined as a special type '__null'. Assigning __null to // integer variable will cause gcc warning. Use NULL_WORD in places where a // pointer is stored as integer value. On some platforms, sizeof(intptr_t) > // sizeof(void*), so here we want something which is integer type, but has the
  // same size as a pointer.

Rather something like:

// Some platform/tool-chain combinations can't assign NULL to an integer
// type so we define NULL_WORD to use in those contexts. For xlc they
// are the same.

Thanks,
David


- fixed some copyright years


Best regards, Matthias



-----Original Message-----
From: Baesken, Matthias
Sent: Freitag, 1. Februar 2019 11:16
To: 'David Holmes' <david.hol...@oracle.com>; 'hotspot-
d...@openjdk.java.net' <hotspot-...@openjdk.java.net>
Cc: 'build-dev@openjdk.java.net' <build-dev@openjdk.java.net>
Subject: RE: RFR : 8218136: minor hotspot adjustments for xlclang++ from
xlc16 on AIX


Confused. Which other xlc compilers set __GNUC_ as you are changing this
for all of them? Though to be honest I don't understand this whole
section anyway - we have a lengthy comment saying why you can't
necessarily assign NULL to an integer type and to use NULL_WORD instead
but then it's defined as NULL anyway! I wonder if we used to have some
other conditions there where it was something different

Hi  David ,  not sure  but  I guess   the  #ifdef __GNUC__    came  from
linux/gcc and was copied to the  aix / xlc file  when the port was done back
then .
See :

globalDefinitions_gcc.hpp

118#ifdef __GNUC__
119  #ifdef _LP64
120    #define NULL_WORD  0L
121  #else
122    // Cast 0 to intptr_t rather than int32_t since they are not the same
type
123    // on platforms such as Mac OS X.
124    #define NULL_WORD  ((intptr_t)0)
125  #endif
126#else
127  #define NULL_WORD  NULL
128#endif

The NULL_WORD  is mostly used in x86-only coding.  But also used  at some
(but few) places in shared coding , like :

intptr_t*   addr;
*addr = NULL_WORD;

intptr_t _callee_registers[RegisterMap::reg_count];
  _callee_registers[i] = src != NULL ? *src : NULL_WORD;


In any case  having an if and else that do exactly the same thing seems
rather
pointless to me.


Yes I agree ,  I think I remove  the #ifdef  ... #else     and just define  
#define
NULL_WORD  NULL   for AIX .

Best regards, Matthias


-----Original Message-----
From: David Holmes <david.hol...@oracle.com>
Sent: Freitag, 1. Februar 2019 05:24
To: Baesken, Matthias <matthias.baes...@sap.com>; 'hotspot-
d...@openjdk.java.net' <hotspot-...@openjdk.java.net>
Cc: 'build-dev@openjdk.java.net' <build-dev@openjdk.java.net>
Subject: Re: RFR : 8218136: minor hotspot adjustments for xlclang++ from
xlc16 on AIX

Hi Matthias,

On 1/02/2019 12:50 am, Baesken, Matthias wrote:
Please review  this small webrev  . It contains a few changes  for  building
hotspot   on AIX with  xlclang++  / xlc16  .
( most likely switching to   xlclang++  / xlc16    will be a must once  we
introduce C++11/14 features )

Some comments on the changes :


- porting_aix.cpp  :  workaround for demangle.h (does not work with
xlclang++)

Can't comment as I know nothing about it.

- arguments.cpp/hpp  :  the UNSUPPORTED_OPTON macro lead  to
assigning false to  AllocateHeapAt which is a bad idea (and does not work
with xlclang++)

Good catch!

-   globalDefinitions_xlc.hpp   : xlclang++ sets   __GNUC__ so we must not
have #error ... in this case

Confused. Which other xlc compilers set __GNUC_ as you are changing this
for all of them? Though to be honest I don't understand this whole
section anyway - we have a lengthy comment saying why you can't
necessarily assign NULL to an integer type and to use NULL_WORD instead
but then it's defined as NULL anyway! I wonder if we used to have some
other conditions there where it was something different? In any case
having an if and else that do exactly the same thing seems rather
pointless to me.

Thanks,
David



Bug/webrev :

https://bugs.openjdk.java.net/browse/JDK-8218136

http://cr.openjdk.java.net/~mbaesken/webrevs/8218136.0/


Thanks, Matthias

Reply via email to