PING: RFR: JDK-8153743: AllocateHeap() and ReallocateHeap() should use ALWAYSINLINE macro
David Holmes
david.holmes at oracle.com
Wed Jun 22 03:17:15 UTC 2016
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