RFR: 8178489: Make align functions more type safe and consistent
Stefan Karlsson
stefan.karlsson at oracle.com
Tue Jul 4 13:33:46 UTC 2017
Hi Mikael,
Thanks for the review.
Inlined:
On 2017-07-04 13:55, Mikael Gerdin wrote:
> Hi Stefan,
>
> On 2017-06-30 11:16, Stefan Karlsson wrote:
>> Hi all,
>>
>> Please review this patch to make the align functions more type safe
>> and consistent.
>>
>> http://cr.openjdk.java.net/~stefank/8178489/webrev.00
>
> I looked through this and I think it looks good but I think you may need
> to add some extra brackets around the use of the parameters in
> is_size_aligned_
>
> change
> #define is_size_aligned_(size, alignment) ((size) ==
> (align_size_up_(size, alignment)))
>
> to
> #define is_size_aligned_(size, alignment) ((size) ==
> (align_size_up_((size), (alignment))))
This is a pre-existing issue with these macros, I just moved it in this
file.
I talked to Mikael offline and we agreed on creating a new Bug for this,
and push after the current set of patches have been pushed. This way I
won't have to rebase the follow-up patches.
StefanK
>
> Thanks
> /Mikael
>
>
>> https://bugs.openjdk.java.net/browse/JDK-8178489
>>
>> Note that this patch needs to be applied on top of the following
>> patches that are out for review:
>> http://cr.openjdk.java.net/~stefank/8178491/webrev.02
>> http://cr.openjdk.java.net/~stefank/8178495/webrev.01
>>
>> Currently, the align functions forces the user to often explicitly
>> cast either the input parameters, or the return type, or both.
>>
>> Two examples of the current API:
>> inline intptr_t align_size_up(intptr_t size, intptr_t alignment);
>> inline void* align_ptr_up(const void* ptr, size_t alignment);
>>
>> I propose that we change the API to use templates to return the
>> aligned value as the same type as the type of the unaligned input.
>>
>> The proposed API would look like this:
>>
>> template <typename T, typename A>
>> inline T align_size_up(T size, A alignment);
>>
>> template <typename T, typename A>
>> inline T* align_ptr_up(T* ptr, A alignment);
>>
>> and a follow-up RFE (JDK-8178499) would get rid of _size_ and _ptr_
>> from the names.
>>
>> Usages of these align functions would then look like:
>>
>> size_t aligned_size = align_up(alloc_size, os::vm_page_size())
>> HeapWord* aligned_top = align_up(top, region_size)
>>
>> Please, take an extra close look at the reinterpret_cast I added in
>> atomic.hpp. This was needed because the align_ptr_down now returns T*
>> and not void*, and the compiler complained when we tried to do a
>> static cast from a volatile jbyte* to a volatile jint*.
>>
>> Tested with the align unit test and JPRT.
>>
>> Thanks,
>> StefanK
More information about the hotspot-dev
mailing list