PING: RFR: JDK-8153743: AllocateHeap() and ReallocateHeap() should use ALWAYSINLINE macro

[Zhengyu Gu]

Hi David,
>>>>>> I'm running it through our build system.
>>>>
>>>> If this patch passes JPRT, I think this patch is ready to merge.
>>>
>>> It did pass jprt. I am wondering though, why the existing uses of
>>> ALWAYSINLINE in instanceKLass.inline.hpp compile fine on Solaris
>>> without needing the addition of "inline" to the ALWAYSINLINE macro?
>>> All those uses appear in template definitions so perhaps that
>>> influences things somehow? But this suggests to me our inlining
>>> attempts may not actually be working as we expect.
>>>
>> According to Solaris Studio User's Guide
>> (https://docs.oracle.com/cd/E24457_01/html/E21991/gljol.html)
>> always_inline
>>
>>    Equivalent to#pragma inlineand-xinline
>>
>> sounds like "always_inline" implies "inline"
>
> But if that were the case we should not have needed to add "inline" to 
> make this change compile.
>
I might have missed some context, what you said, is that, without "inline",
it works with ALWAYSINLINE in instanceKlass.inline.hpp, but not 
allocation.inline.hpp?

Thanks,

-Zhengyu

> Cheers,
> David
> -----
>
>> Change looks good to me.
>>
>> -Zhengyu
>>
>>> A second review of this is still needed anyway, so I've added Kim to
>>> the cc list for his compiler expertise.
>>>
>>> Thanks,
>>> David
>>>
>>>>
>>>> Thanks,
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>> On 2016/05/04 19:16, David Holmes wrote:
>>>>> On 4/05/2016 12:10 PM, David Holmes wrote:
>>>>>> On 25/04/2016 10:11 PM, Yasumasa Suenaga wrote:
>>>>>>> Hi all,
>>>>>>>
>>>>>>> Could you review and sponsor it?
>>>>>>> I confirmed this patch can apply to jdk9/hs repos.
>>>>>>>
>>>>>>> However, I checked this patch with GCC on Linux.
>>>>>>> I cannot check with other compiler.
>>>>>>
>>>>>> I'm running it through our build system.
>>>>>
>>>>> We also require:
>>>>>
>>>>> diff -r fc5b64f70199
>>>>> src/share/vm/utilities/globalDefinitions_sparcWorks.hpp
>>>>> --- a/src/share/vm/utilities/globalDefinitions_sparcWorks.hpp
>>>>> +++ b/src/share/vm/utilities/globalDefinitions_sparcWorks.hpp
>>>>> @@ -279,6 +279,6 @@
>>>>>
>>>>>  // Inlining support
>>>>>  #define NOINLINE
>>>>> -#define ALWAYSINLINE __attribute__((always_inline))
>>>>> +#define ALWAYSINLINE inline __attribute__((always_inline))
>>>>>
>>>>>  #endif // SHARE_VM_UTILITIES_GLOBALDEFINITIONS_SPARCWORKS_HPP
>>>>>
>>>>> David
>>>>>
>>>>>
>>>>>>> Also other compilers might need "inline" statement in ALWAYSINLINE
>>>>>>> macro.
>>>>>>> So I want to hear whether this patch can work with other compilers.
>>>>>>
>>>>>> I don't have any input on this - sorry.
>>>>>>
>>>>>> David
>>>>>> -----
>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Yasumasa
>>>>>>>
>>>>>>>
>>>>>>> On 2016/04/16 1:10, Daniel D. Daugherty wrote:
>>>>>>>> JDK9-hs and JDK9-hs-rt are currently locked down due to the merge.
>>>>>>>> Please see the "Merging jdk9/hs-rt with jdk9/hs" e-mail thread on
>>>>>>>> the hotspot-dev at openjdk.java.net alias.
>>>>>>>>
>>>>>>>> Dan
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 4/15/16 7:28 AM, Yasumasa Suenaga wrote:
>>>>>>>>> PING: Could you review and sponsor it?
>>>>>>>>>
>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153743/webrev.00/
>>>>>>>>> Yasumasa
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 2016/04/07 23:13, Yasumasa Suenaga wrote:
>>>>>>>>>> For force inlining, JDK-8076212 uses always_inline attribute to
>>>>>>>>>> them.
>>>>>>>>>> JDK-8151593 added ALWAYSINLINE macro for force inlining.
>>>>>>>>>>
>>>>>>>>>> For consistency, and for other compiler support, AllocateHeap()
>>>>>>>>>> and
>>>>>>>>>> ReallocateHeap() should use ALWAYSINLINE.
>>>>>>>>>>
>>>>>>>>>> I used ALWAYSINLINE to them, but I got error message as below;
>>>>>>>>>> ------------------------------
>>>>>>>>>> Building target 'images' in configuration
>>>>>>>>>> 'linux-x86_64-normal-server-release'
>>>>>>>>>> In file included from
>>>>>>>>>> /home/ysuenaga/OpenJDK/hs-rt/hotspot/src/share/vm/utilities/array.hpp:29:0, 
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>                   from
>>>>>>>>>> /home/ysuenaga/OpenJDK/hs-rt/hotspot/src/share/vm/memory/universe.hpp:29, 
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>                   from
>>>>>>>>>> /home/ysuenaga/OpenJDK/hs-rt/hotspot/src/share/vm/code/oopRecorder.hpp:28, 
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>                   from
>>>>>>>>>> /home/ysuenaga/OpenJDK/hs-rt/hotspot/src/share/vm/asm/codeBuffer.hpp:28, 
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>                   from
>>>>>>>>>> /home/ysuenaga/OpenJDK/hs-rt/hotspot/src/share/vm/asm/assembler.hpp:28, 
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>                   from
>>>>>>>>>> /home/ysuenaga/OpenJDK/hs-rt/hotspot/src/share/vm/precompiled/precompiled.hpp:29: 
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> /home/ysuenaga/OpenJDK/hs-rt/hotspot/src/share/vm/memory/allocation.inline.hpp:72:20: 
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> エラー: always_inline function might not be inlinable
>>>>>>>>>> [-Werror=attributes]
>>>>>>>>>>   ALWAYSINLINE char* ReallocateHeap(char *old, size_t size,
>>>>>>>>>> MEMFLAGS
>>>>>>>>>> flag,
>>>>>>>>>>                      ^
>>>>>>>>>> ------------------------------
>>>>>>>>>> * This message includes Japanese because my environment is
>>>>>>>>>> Japanese
>>>>>>>>>> locale :-)
>>>>>>>>>>
>>>>>>>>>> According to GCC manual [1], non-static inline function is 
>>>>>>>>>> always
>>>>>>>>>> compiled
>>>>>>>>>> on its own in the usual fashion.
>>>>>>>>>> However, we can compile as inline function with "inline" and
>>>>>>>>>> "always_inline".
>>>>>>>>>> always_inline attribute does not imply inlining [2].
>>>>>>>>>> GCC testcase [3] uses both "inline" and "always_inline" 
>>>>>>>>>> actually.
>>>>>>>>>>
>>>>>>>>>> I uploaded webrev. Could you review it?
>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153743/webrev.00/
>>>>>>>>>>
>>>>>>>>>> I cannot access JPRT.
>>>>>>>>>> So I need a sponsor.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>>
>>>>>>>>>> Yasumasa
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> [1] https://gcc.gnu.org/onlinedocs/gcc/Inline.html
>>>>>>>>>> [2] https://chromiumcodereview.appspot.com/14820003/
>>>>>>>>>> [3]
>>>>>>>>>> https://gcc.gnu.org/viewcvs/gcc/trunk/gcc/testsuite/gcc.dg/vmx/gcc-bug-i.c?view=markup&pathrev=178730 
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>