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

[David Holmes]

On 9/07/2013 11:12 PM, Lindenmaier, Goetz wrote:
> Hi,
>
> Here a webrev without the cleanups:
> http://cr.openjdk.java.net/~goetz/webrevs/8019973-ia64_def-2/

Thanks. Looks good.

David
-----

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