RFR(M): 8019973: PPC64 (part 11): Fix IA64 preprocessor conditionals on AIX.

Wed Jul 10 14:41:49 PDT 2013

On 7/10/13 2:00 PM, Lindenmaier, Goetz wrote:
> Hi Vladimir,
>
> I thought about that, too. But it seems quiet risky to me,
> as it depends on the final order of inclusion whether it works.
> Usually, macro.hpp will be included early, because it's basics that should
> be seen everywhere.  Also, it will end up early in the  preprocessed
> file as it has no other includes itself.
>
> If the system headers are included later, they will overrule the undef in
> macro.hpp.
> I think it's even some kind of pattern in the hotspot files to include the
> system headers after the hotspot headers.
>
> So even if it works now, it easily might break.

Agree. Your changes are good then.

thanks,
Vladimir

>
> Best regards,
>    Goetz.
>
>
> -----Original Message-----
> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
> Sent: Wednesday, July 10, 2013 9:18 PM
> To: Lindenmaier, Goetz
> Cc: David Holmes; ppc-aix-port-dev at openjdk.java.net; 'hotspot-dev at openjdk.java.net'
> Subject: Re: RFR(M): 8019973: PPC64 (part 11): Fix IA64 preprocessor conditionals on AIX.
>
> Goetz,
>
> macros.hpp is included in all other files. Can we just undef IA64 there?:
>
> // This is a REALLY BIG HACK, but on AIX <sys/systemcfg.h>
> unconditionally defines IA64.
> // At least on AIX 7.1 this is a real problem because 'systemcfg.h' is
> indirectly included
> // by 'pthread.h' and other common system headers.
> #ifdef AIX
> #undef IA64
> #endif
>
> thanks,
> Vladimir
>
> On 7/9/13 6:12 AM, Lindenmaier, Goetz wrote:
>> Hi,
>>
>> Here a webrev without the cleanups:
>> http://cr.openjdk.java.net/~goetz/webrevs/8019973-ia64_def-2/
>>
>> Best regards,
>>     Goetz.
>>
>>
>>
>> -----Original Message-----
>> From: David Holmes [mailto:david.holmes at oracle.com]
>> Sent: Dienstag, 9. Juli 2013 14:08
>> To: Lindenmaier, Goetz
>> Cc: ppc-aix-port-dev at openjdk.java.net; 'hotspot-dev at openjdk.java.net'; Vladimir Kozlov
>> Subject: Re: RFR(M): 8019973: PPC64 (part 11): Fix IA64 preprocessor conditionals on AIX.
>>
>> On 9/07/2013 7:56 PM, Lindenmaier, Goetz wrote:
>>> Hi David,
>>>
>>>> A curious thing to do ;-)
>>> Yes, I agree.
>>>
>>>> Aside: presumably this is included in turn by other standard headers,
>>>> not directly included - otherwise we could just undef IA64 after the
>>>> include.
>>> Yes, that's the case.
>>>
>>>> Might be simpler if we just eradicate the IA64 remnants in the code. I
>>>> think we may even have a RFE for this. AFAIK there are no remaining
>>>> users of the IA64 code in the hotspot codebase.
>>> Yes, there is a remaining user.  We have Java 4-7 VMs on linux, hp & win
>>> Ia64.  8 in development, all soon based on hs25. We also contributed to
>>> the change cleaning up the IA64 defines.  There are only such left
>>> we need in our code.
>>> http://hg.openjdk.java.net/hsx/hotspot-main/hotspot/rev/9fae07c31641
>>
>> I'm somewhat perplexed. The IA64 uses you still see in the hotspot
>> sources are ancient remnants of the IA64 support. Most of it has been
>> stripped out completely. So how anyone could be using any of this is, as
>> I said, perplexing ???
>>
>>>> #if defined(__GNUC__) && !defined(IA64)
>>> You are right, I could just leave it as is, except if __GNUC__ is define on AIX,
>>> who knows ;).  If we don't remove it, I need to add !PPC64 so that it's
>>> inlined in our port on linux.
>>>> more to avoid build-failures, but we may still want to avoid inlining
>>>> them for other reasons! So this aspect needs further investigation. Or
>>> I think it's a good idea to clean it up.  The build problem
>>> is documented.  There may be 'other reasons', but unexpected issues
>>> can be there with any change.
>>
>> But the aim is to avoid making gratuitous changes to code that is not
>> part of the ppc64/AIX port. If you start inlining these functions you
>> don't know what the impact will be. So why change them when it is not
>> necessary.
>>
>> David
>> -----
>>
>>>>> prims/forte.cpp uses a lot of different mechanisms all disabling forte
>>>> Again if we just get rid of IA64 this will be a non-issue right?
>>> Here I would add !AIX.
>>> But have a look at the file, there are also several other platforms
>>> where this is excluded, and the exclusion is implemented differently
>>> on each platform.  Not that nice ...
>>>
>>> Anyways, I'm fine with adding !AIX/!PPC64 everywhere -- it's nothing
>>> functional.
>>>
>>> Best regards,
>>>      Goetz.
>>>
>>>
>>>
>>> -----Original Message-----
>>> From: David Holmes [mailto:david.holmes at oracle.com]
>>> Sent: Dienstag, 9. Juli 2013 03:56
>>> To: Lindenmaier, Goetz
>>> Cc: ppc-aix-port-dev at openjdk.java.net; 'hotspot-dev at openjdk.java.net'; Vladimir Kozlov
>>> Subject: Re: RFR(M): 8019973: PPC64 (part 11): Fix IA64 preprocessor conditionals on AIX.
>>>
>>> Hi Goetz,
>>>
>>> On 8/07/2013 10:45 PM, Lindenmaier, Goetz wrote:
>>>> Hi,
>>>>
>>>> This is in preparation of the PPC AIX port.
>>>>
>>>> A header file (systemcfg.h) on Aix 7.1 defines the macro "IA64" unconditionally.
>>>
>>> A curious thing to do ;-)
>>>
>>> Aside: presumably this is included in turn by other standard headers,
>>> not directly included - otherwise we could just undef IA64 after the
>>> include.
>>>
>>>> This leads to wrong configurations of shared files on Aix where IA64 is used.
>>>> This change replaces uses of "IA64" by "IA64 && !AIX".
>>>
>>> Might be simpler if we just eradicate the IA64 remnants in the code. I
>>> think we may even have a RFE for this. AFAIK there are no remaining
>>> users of the IA64 code in the hotspot codebase.
>>>
>>>> To reduce the complexity we propose to simplify two matters:
>>>>
>>>> Since the initial checkin objectMonitor.cpp and synchronizer.cpp
>>>> define  #define ATTR __attribute__((noinline)) claiming this is needed
>>>> to avoid a gcc build time error with - at that time - old gcc versions.
>>>> This define depends on IA64. We remove it altogether.
>>>
>>> Well it doesn't "depend" on IA64:
>>>
>>> #if defined(__GNUC__) && !defined(IA64)
>>>       // Need to inhibit inlining for older versions of GCC to avoid
>>> build-time failures
>>>       #define ATTR __attribute__((noinline))
>>> #else
>>>       #define ATTR
>>> #endif
>>>
>>> it becomes empty on IA64 (and non gcc systems). So removing it
>>> altogether is changing things for all the other gcc using platforms. Now
>>> it may be that we don't need to preclude inlining of these methods any
>>> more to avoid build-failures, but we may still want to avoid inlining
>>> them for other reasons! So this aspect needs further investigation. Or
>>> you can just leave it as-is - if you have IA64 always defined then you
>>> will get the empty #else part. Or if we go with the "eradicate IA64"
>>> path then you can change IA64 to AIX (though it is odd to exchange an
>>> architecture check with an OS check).
>>>
>>>> prims/forte.cpp uses a lot of different mechanisms all disabling forte
>>>> support on windows, apple and ia64. We would have to add Aix.
>>>> Instead, we propose to use a check for the supported platforms (see webrev).
>>>> We could also add a macro SUPPORTS_FORTE that is defined in the corresponding
>>>> makefiles, and/or move forte.cpp into the os/solaris and os_cpu/linux_x86
>>>> directories.
>>>
>>> Again if we just get rid of IA64 this will be a non-issue right?
>>>
>>> Thanks,
>>> David
>>>
>>>> Are there other platforms that need to be supported? I derived the platforms
>>>> from the comment in forte.cpp.
>>>>
>>>> Please review this change. I'm happy to incorporate your comments.
>>>> http://cr.openjdk.java.net/~goetz/webrevs/8019973-ia64_def/aixia64-hotspot.changeset
>>>>
>>>> Best regards,
>>>>       Goetz.
>>>>