RFR(S): 8151593: Cleanup definition/usage of INLINE/NOINLINE macros and add xlC support

[David Holmes]

On 11/03/2016 7:45 PM, Volker Simonis wrote:
> On Fri, Mar 11, 2016 at 10:36 AM, Mikael Gerdin
> <mikael.gerdin at oracle.com> wrote:
>> Volker,
>>
>> On 2016-03-11 10:31, Volker Simonis wrote:
>>>
>>> Hi Dan,
>>>
>>> thanks for the hint. I've linked the two issues together in JBS and I
>>> can close the second one as duplicate once the fix is committed.
>>>
>>> How can I use two bug IDs in a changeset? Should I just put it in an
>>> additional "Summary:" line?
>>
>>
>> The changeset format actually allows multiple bug ids in a single commit,
>> the format is:
>>
>> XXXXXXX: Bug title 1
>> YYYYYYY: Bug title 2
>> Summary: foo
>> Reviewed-by: bar
>>
>> See
>> http://openjdk.java.net/guide/producingChangeset.html
>>
>
> Ah, I see. Thanks for the info.
> Nevertheless I'd prefer to use one single bug only and close the other
> as duplicate.
>
> I personally think multiple bug IDs are confusing (and they break our
> internal integration scripts :)

Agreed. If there are two bugs for the same issue then one should be 
closed as a duplicate and only a single bugid used.

It would be extremely rare where one changeset needed to fix two 
completely independent bugs IMHO.

David

> Regards,
> Volker
>
>> /Mikael
>>
>>
>>>
>>> As regards to content, I really think the annotations aren't needed
>>> any more. I've tried with gcc4.1.2 which is the oldest we are using it
>>> it works perfectly.
>>>
>>> Regards,
>>> Volker
>>>
>>> On Thu, Mar 10, 2016 at 6:56 PM, Daniel D. Daugherty
>>> <daniel.daugherty at oracle.com> wrote:
>>>>
>>>> Hi Volker,
>>>>
>>>> The objectMonitor.cpp and synchronizer.cpp NOINLINE issue is tracked via:
>>>>
>>>> JDK-8049103 investigate whether inlining still needs to be inhibited in
>>>>               objectMonitor.cpp and synchronizer.cpp
>>>> https://bugs.openjdk.java.net/browse/JDK-8049103
>>>>
>>>> Feel free use both bug IDs in your changeset.
>>>>
>>>> Dan
>>>>
>>>>
>>>>
>>>> On 3/10/16 10:10 AM, Volker Simonis wrote:
>>>>>
>>>>>
>>>>> Hi,
>>>>>
>>>>> can I please have a review and a sponsor for the following clean-up
>>>>> change:
>>>>>
>>>>> http://cr.openjdk.java.net/~simonis/webrevs/2016/8151593/
>>>>> https://bugs.openjdk.java.net/browse/JDK-8151593
>>>>>
>>>>> First I only wanted to implement the INLINE macro for xlC in
>>>>> instanceKlass.inline.hpp. But then I realized that we also define
>>>>> NOINLINE in various other files so I decided to do it "the right way"
>>>>> and clean up the code a little bit.
>>>>>
>>>>> I've now moved the definition of the INLINE/NOINLINE macros to
>>>>> globalDefinitions_<compiler>.hpp. In the case where there is no
>>>>> compiler-specific definition, globalDefinitions.hpp defines empty
>>>>> defaults for the macros.
>>>>>
>>>>> I've also renamed INLINE to ALWAYSINLINE to match its intention more
>>>>> clearly.
>>>>>
>>>>> Currently NOINLINE is really only defined on Linux. This is the same
>>>>> behavior we had before the change. Following some more details:
>>>>>
>>>>> os_linux.cpp
>>>>>
>>>>> - removed the handling for gcc < 3 (even we at SAP don't use this
>>>>> anymore
>>>>> :)
>>>>>
>>>>> objectMonitor.cpp, synchronizer.cpp
>>>>>
>>>>> - completely removed the annotation with NOINLNE which is there since
>>>>> pre-OpenJDK times already. The comment mentions that this was required
>>>>> to prevent build-time failures with 'older' versions of GCC. But we
>>>>> already build without 'NOINLINE' on Linux/ppc64 since years and I've
>>>>> also tested that it still works on Linux/x86.
>>>>>
>>>>> I've Build and smoke-tested on Linux/amd64 and Linux/ppc64,
>>>>> Solaris/SPARC, MacOS X, AIX and Windows.
>>>>>
>>>>> Regards,
>>>>> Volker
>>>>
>>>>
>>>>
>>