On Mon Aug 4, 2025 at 11:16 PM JST, Miguel Ojeda wrote: > On Mon, Aug 4, 2025 at 1:45 PM Alexandre Courbot <[email protected]> wrote: >> >> - The `last_set_bit` function is dropped, with the recommendation to use >> the standard library's `checked_ilog2` which does essentially the same >> thing. > > Yeah, let's see what people think about this one on the kernel side. > > I don't mind either way, i.e. to have a few wrappers with slightly > different semantics if that is more common/understandable. > >> The upstream `Alignment` is more constrained than the `PowerOfTwo` of >> the last revision: it uses `usize` internally instead of a generic >> value, and does not provide `align_down` or `align_up` methods. > > `PowerOfTwo` seemed fine to me as well (or even implementing one in > terms of the other).
`PowerOfTwo` has little prospect of existing upstream, and I think we should be able to live pretty well with `Alignment` thanks to the suggestions you make below. > >> These two shortcomings come together very nicely to gift us with a nice >> headache: we need to align values potentially larger than `usize`, thus >> need to make `align_down` and `align_up` generic. The generic parameter >> needs to be constrained on the operations used to perform the alignment >> (e.g. `BitAnd`, `Not`, etc) and there is one essential operation for >> which no trait exists in the standard library: `checked_add`. Thus the >> first patch of this series introduces a trait for it in the `num` module >> and implements it for all integer types. I suspect we will need >> something alongside these lines for other purposes anyway, and probably >> other traits too. > > This part could be avoided implementing them the other way around, > right? i.e. as an extension trait on the other side. > > It may also be also a bit easier to understand on the call site, too, > since value would be first. Yes! This is much better and more intuitive. > >> This generic nature also restricts these methods to being non-const, >> unfortunately. I have tried to implement them as macros instead, but >> quickly hit a wall due to the inability to convert `Alignment`'s `usize` >> into the type of the value to align. > > I guess we could also just have one per type like for other ones to > have them `const`, like we do for other similar things like > `bit`/`genmask`. This leaves us with two viable solutions: `Alignable` extension trait with `align_up` and `align_down` operations that take an `Alignment` as parameter (with the caveat that they could not be const for now), or a set of per-type functions defined using a macro, similar to bit/genmask. I am fine with both but don't know which one would be preferred, can the R4L leadership provide some guidance? :) Thanks, Alex.
