PING: RFR: JDK-8153743: AllocateHeap() and ReallocateHeap() should use ALWAYSINLINE macro
Yasumasa Suenaga
yasuenag at gmail.com
Wed Jun 22 03:54:03 UTC 2016
Thanks!
Yasumasa
2016/06/22 12:17 "David Holmes" <david.holmes at oracle.com>:
> Changes now pushed.
>
> http://hg.openjdk.java.net/jdk9/hs/hotspot/rev/dffe59badb82
>
> David
>
> On 18/05/2016 10:12 PM, David Holmes wrote:
>
>> Hi Yasumasa,
>>
>> Zhengyu already reviewed this, as did I. But now this can't go in unless
>> it gets approval as an enhancement after the Feature Complete date. That
>> process isn't in place yet.
>>
>> David
>>
>> On 18/05/2016 9:58 PM, Yasumasa Suenaga wrote:
>>
>>> PING: Could you review it?
>>>
>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153743/webrev.01/
>>>>>>>
>>>>>>
>>> Yasumasa
>>>
>>>
>>> On 2016/05/11 23:04, Yasumasa Suenaga wrote:
>>>
>>>> Hi,
>>>>
>>>> I've sent review request for ALWAYSINLINE macro.
>>>> Could you review it?
>>>>
>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153743/webrev.01/
>>>>>>>
>>>>>>
>>>> I need a second reviewer.
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>> On 2016/05/05 11:21, Yasumasa Suenaga wrote:
>>>>
>>>>> Hi David,
>>>>>
>>>>> It did pass jprt.
>>>>>>
>>>>>
>>>>> Thanks!
>>>>>
>>>>> But this suggests to me our inlining attempts may not actually be
>>>>>> working as we expect.
>>>>>>
>>>>>
>>>>> I think it is inlined.
>>>>> I run nm command for fastdebug libjvm on Linux, I cannot found
>>>>> symbols in instanceKlass.inline.hpp .
>>>>> (I do not have Solaris, so I cannot confirm it on Solaris.)
>>>>>
>>>>>
>>>>> A second review of this is still needed anyway, so I've added Kim to
>>>>>> the cc list for his compiler expertise.
>>>>>>
>>>>>
>>>>> Okay, I'm waiting second reviewer.
>>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Yasumasa
>>>>>
>>>>>
>>>>> On 2016/05/05 11:09, David Holmes wrote:
>>>>>
>>>>>> On 4/05/2016 11:17 PM, Yasumasa Suenaga wrote:
>>>>>>
>>>>>>> Hi David,
>>>>>>>
>>>>>>> We also require:
>>>>>>>>
>>>>>>>> diff -r fc5b64f70199
>>>>>>>> src/share/vm/utilities/globalDefinitions_sparcWorks.hpp
>>>>>>>>
>>>>>>>
>>>>>>> Thanks!
>>>>>>> I added it to new webrev:
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8153743/webrev.01/
>>>>>>>
>>>>>>> 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.
>>>>>>
>>>>>> 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