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

David Holmes david.holmes at oracle.com
Sun May 8 02:54:50 UTC 2016


On 7/05/2016 8:18 AM, Zhengyu Gu wrote:
> 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?

Yes. So if "inline" were implied we should not have needed to add it to 
make the allocation.inline.hpp use of ALWAYSINLINE work.

Thanks,
David

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


More information about the hotspot-runtime-dev mailing list