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

[David Holmes]

On 9/07/2013 10:15 PM, Volker Simonis wrote:
> On Tue, Jul 9, 2013 at 2:08 PM, David Holmes <david.holmes at oracle.com> wrote:
>> 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 ???
>>
>
> You're too Oracle-centric, but it's not only Oracle who has closed
> HotSpot ports:)

I understand that, but as I said we have been gradually removing IA64 
support from all over the shared codebase. So I'm still surprised that 
anyone with an IA64 port could possibly be relying on the remaining 
fragments. But so be it.

David
-----


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