Message ID | 20250606164156.1442682-7-barnabas.pocze@ideasonboard.com |
---|---|
State | New |
Headers | show |
Series |
|
Related | show |
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 >>
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 @@ -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) +{ + assert(alignment > 0); + + return reinterpret_cast<std::uintptr_t>(p) % alignment == 0; +} + +template<typename T> +constexpr T up(T x, cxx20::type_identity_t<T> alignment) +{ + 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 */
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