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

Wed Jul 10 14:00:30 PDT 2013

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.

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.
>>>