On Fri, Jun 13, 2025 at 11:16:10PM +0900, Alexandre Courbot wrote: [...] > >> + /// Aligns `self` down to `alignment`. > >> + /// > >> + /// # Examples > >> + /// > >> + /// ``` > >> + /// use kernel::num::PowerOfTwo; > >> + /// > >> + /// > >> assert_eq!(PowerOfTwo::<u32>::new(0x1000).align_down(0x4fff), 0x4000); > >> + /// ``` > >> + #[inline(always)] > >> + pub const fn align_down(self, value: $t) -> $t { > > > > I'm late to party, but could we instead implement: > > > > pub const fn round_down<i32>(value: i32, shift: i32) -> i32 { > > value & !((1 << shift) - 1) > > } > > > > pub const fn round_up<i32>(value: i32, shift: i32) -> i32 { > > let mask = (1 << shift) - 1; > > value.wrapping_add(mask) & !mask > > } > > > > ? It's much harder to pass an invalid alignment with this. > > It also forces you to think in terms of shifts instead of values - i.e. > you cannot round to `0x1000` as it commonly done in the kernel, now you
Well, for const values, you can always define: const ROUND_SHIFT_0X1000: i32 = 12; because `0x1000` is just a name ;-) or we define an Alignment in term of the shift: pub struct Alignment { shift: i8, } ipml Alignment { pub const new(shift: i8) -> Self { Self { shift } } } then const ALIGN_0x1000: Alignment = Alignment::new(12); and pub const fn round_down_i32(value: i32, align: Alignment) -> i32 { ... } My point was that instead of the value itself, we can always use the shift to represent a power of two, and that would avoid troubles when we need to check the internal representation. That said, after some experiments by myself, I haven't found any significant difference between shift representations vs value representations. So no strong reason of using a shift representation. Regards, Boqun > need to do some mental gymnastics to know it is actually a shift of `12`. > Being able to use the actual value to round to is more familiar (and > natural) to me.