[RFC,v1,06/23] libcamera: base: Add alignment utility functions

23487 diff mbox series

Add a couple internal functions to check alignment, and to
align integers, pointer up to a given alignment.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 include/libcamera/base/details/align.h | 72 ++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)
 create mode 100644 include/libcamera/base/details/align.h

Hi Barnabás

On Fri, Jun 06, 2025 at 06:41:39PM +0200, Barnabás Pőcze wrote:
> Add a couple internal functions to check alignment, and to
> align integers, pointer up to a given alignment.
>
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> ---
>  include/libcamera/base/details/align.h | 72 ++++++++++++++++++++++++++
>  1 file changed, 72 insertions(+)
>  create mode 100644 include/libcamera/base/details/align.h
>
> diff --git a/include/libcamera/base/details/align.h b/include/libcamera/base/details/align.h
> new file mode 100644
> index 000000000..e1c24c6e1
> --- /dev/null
> +++ b/include/libcamera/base/details/align.h

Same question about the additional details/ subfolder

> @@ -0,0 +1,72 @@
> +#pragma once
> +
> +#include <cassert>
> +#include <cstddef>
> +#include <cstdint>
> +
> +#include <libcamera/base/details/cxx20.h>
> +
> +namespace libcamera::details::align {
> +
> +template<typename T>
> +bool is(T *p, std::uintptr_t alignment)

Bikeshedding on names

is
        details::align:is(p, q);

better than a longer name such as is_aligned() ?

Why have you used uintptr_t to express an alignement ?

> +{
> +	assert(alignment > 0);
> +
> +	return reinterpret_cast<std::uintptr_t>(p) % alignment == 0;

This won't work with alignement as an unsigned int, right ?

> +}
> +
> +template<typename T>
> +constexpr T up(T x, cxx20::type_identity_t<T> alignment)

I'm a bit confused. The is() function has

        <typename T> is(T *)

while this one has

        <typename T> up(T)

so I presume we will have

        is<int>(p, q)
        up<int *>(p, q)

?

> +{
> +	static_assert(std::is_unsigned_v<T>);
> +	assert(alignment > 0);
> +
> +	const auto padding = (alignment - (x % alignment)) % alignment;
> +	assert(x + padding >= x);
> +
> +	return x + padding;
> +}
> +
> +template<typename T>
> +auto *up(T *p, std::uintptr_t alignment)
> +{
> +	using U = std::conditional_t<
> +		std::is_const_v<T>,
> +		const std::byte,
> +		std::byte
> +	>;
> +
> +	return reinterpret_cast<U *>(up(reinterpret_cast<std::uintptr_t>(p), alignment));
> +}
> +
> +template<typename T>
> +T *up(std::size_t size, std::size_t alignment, T *&ptr, std::size_t *avail = nullptr)
> +{
> +	assert(alignment > 0);
> +
> +	auto p = reinterpret_cast<std::uintptr_t>(ptr);
> +	const auto padding = (alignment - (p % alignment)) % alignment;
> +
> +	if (avail) {
> +		if (size > *avail || padding > *avail - size)
> +			return nullptr;
> +
> +		*avail -= size + padding;
> +	}
> +
> +	p += padding;
> +	ptr = reinterpret_cast<T *>(p + size);
> +
> +	return reinterpret_cast<T *>(p);
> +}
> +
> +template<typename U, typename T>
> +U *up(T *&ptr, std::size_t *avail = nullptr)
> +{
> +	return reinterpret_cast<std::conditional_t<std::is_const_v<T>, const U, U> *>(
> +		up(sizeof(U), alignof(U), ptr, avail)
> +	);
> +}
> +
> +} /* namespace libcamera::details::align */
> --
> 2.49.0
>

Hi

2025. 06. 10. 18:39 keltezéssel, Jacopo Mondi írta:
> Hi Barnabás
> 
> On Fri, Jun 06, 2025 at 06:41:39PM +0200, Barnabás Pőcze wrote:
>> Add a couple internal functions to check alignment, and to
>> align integers, pointer up to a given alignment.
>>
>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
>> ---
>>   include/libcamera/base/details/align.h | 72 ++++++++++++++++++++++++++
>>   1 file changed, 72 insertions(+)
>>   create mode 100644 include/libcamera/base/details/align.h
>>
>> diff --git a/include/libcamera/base/details/align.h b/include/libcamera/base/details/align.h
>> new file mode 100644
>> index 000000000..e1c24c6e1
>> --- /dev/null
>> +++ b/include/libcamera/base/details/align.h
> 
> Same question about the additional details/ subfolder
> 
>> @@ -0,0 +1,72 @@
>> +#pragma once
>> +
>> +#include <cassert>
>> +#include <cstddef>
>> +#include <cstdint>
>> +
>> +#include <libcamera/base/details/cxx20.h>
>> +
>> +namespace libcamera::details::align {
>> +
>> +template<typename T>
>> +bool is(T *p, std::uintptr_t alignment)
> 
> Bikeshedding on names
> 
> is
>          details::align:is(p, q);
> 
> better than a longer name such as is_aligned() ?

I don't know, I am open to suggestions. I chose these names because
they are in the `align` namespace.


> 
> Why have you used uintptr_t to express an alignement ?

Since the pointer is cast to `uintptr_t`, that seemed like the obvious choice.
I think any unsigned type should do.


> 
>> +{
>> +	assert(alignment > 0);
>> +
>> +	return reinterpret_cast<std::uintptr_t>(p) % alignment == 0;
> 
> This won't work with alignement as an unsigned int, right ?

It should.


> 
>> +}
>> +
>> +template<typename T>
>> +constexpr T up(T x, cxx20::type_identity_t<T> alignment)
> 
> I'm a bit confused. The is() function has
> 
>          <typename T> is(T *)
> 
> while this one has
> 
>          <typename T> up(T)
> 
> so I presume we will have
> 
>          is<int>(p, q)
>          up<int *>(p, q)

I don't expect anyone to specify the template parameters explicitly.
But in any case, I have modified `is()` not to be a template but instead
accept `const void *`.


Regards,
Barnabás Pőcze

> 
> ?
> 
>> +{
>> +	static_assert(std::is_unsigned_v<T>);
>> +	assert(alignment > 0);
>> +
>> +	const auto padding = (alignment - (x % alignment)) % alignment;
>> +	assert(x + padding >= x);
>> +
>> +	return x + padding;
>> +}
>> +
>> +template<typename T>
>> +auto *up(T *p, std::uintptr_t alignment)
>> +{
>> +	using U = std::conditional_t<
>> +		std::is_const_v<T>,
>> +		const std::byte,
>> +		std::byte
>> +	>;
>> +
>> +	return reinterpret_cast<U *>(up(reinterpret_cast<std::uintptr_t>(p), alignment));
>> +}
>> +
>> +template<typename T>
>> +T *up(std::size_t size, std::size_t alignment, T *&ptr, std::size_t *avail = nullptr)
>> +{
>> +	assert(alignment > 0);
>> +
>> +	auto p = reinterpret_cast<std::uintptr_t>(ptr);
>> +	const auto padding = (alignment - (p % alignment)) % alignment;
>> +
>> +	if (avail) {
>> +		if (size > *avail || padding > *avail - size)
>> +			return nullptr;
>> +
>> +		*avail -= size + padding;
>> +	}
>> +
>> +	p += padding;
>> +	ptr = reinterpret_cast<T *>(p + size);
>> +
>> +	return reinterpret_cast<T *>(p);
>> +}
>> +
>> +template<typename U, typename T>
>> +U *up(T *&ptr, std::size_t *avail = nullptr)
>> +{
>> +	return reinterpret_cast<std::conditional_t<std::is_const_v<T>, const U, U> *>(
>> +		up(sizeof(U), alignof(U), ptr, avail)
>> +	);
>> +}
>> +
>> +} /* namespace libcamera::details::align */
>> --
>> 2.49.0
>>