RFR : 8218136: minor hotspot adjustments for xlclang++ from xlc16 on AIX

[David Holmes]

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.holmes at oracle.com>; 'hotspot-
>> dev at openjdk.java.net' <hotspot-dev at openjdk.java.net>
>> Cc: 'build-dev at openjdk.java.net' <build-dev at 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.holmes at oracle.com>
>>> Sent: Freitag, 1. Februar 2019 05:24
>>> To: Baesken, Matthias <matthias.baesken at sap.com>; 'hotspot-
>>> dev at openjdk.java.net' <hotspot-dev at openjdk.java.net>
>>> Cc: 'build-dev at openjdk.java.net' <build-dev at 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
>>>>