Skip to content

Commit d75844d

Browse files
committed
fix(core): fix decomposition algorithm not matching the theory
- problem arose from a shift being done on an unsigned value which did not keep the signed characteristics of the represented signed value - introduce an arithmetic_shift on the UnsignedInteger trait with a blanket implementation - add the edge case which revelead the issue - the asm has been verified to only change for the shift operation being applied, meaning no performance regression will occurr
1 parent ef07963 commit d75844d

File tree

3 files changed

+22
-2
lines changed

3 files changed

+22
-2
lines changed

tfhe/src/core_crypto/commons/math/decomposition/iter.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,9 +143,9 @@ pub(crate) fn decompose_one_level<S: UnsignedInteger>(
143143
mod_b_mask: S,
144144
) -> S {
145145
let res = *state & mod_b_mask;
146-
*state >>= base_log;
146+
*state = (*state).arithmetic_shr(base_log);
147147
let carry = decomposition_bit_trick(res, *state, base_log);
148-
*state += carry;
148+
*state = (*state).wrapping_add(carry);
149149
res.wrapping_sub(carry << base_log)
150150
}
151151

tfhe/src/core_crypto/commons/math/decomposition/tests.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,3 +224,17 @@ fn test_single_level_decompose_balanced() {
224224
// We expect an average value of 0 so the sum is also 0
225225
assert_eq!(sum, 0);
226226
}
227+
228+
#[test]
229+
fn test_decomposition_edge_case_sign_handling() {
230+
let decomposer = SignedDecomposer::new(DecompositionBaseLog(17), DecompositionLevelCount(3));
231+
let val: u64 = 0x8000_00e3_55b0_c827;
232+
233+
let decomp = decomposer.decompose(val);
234+
235+
let expected = [44422i64, 909, -65536];
236+
237+
for (term, expect) in decomp.zip(expected) {
238+
assert_eq!(term.value() as i64, expect, "Problem with term {term:?}");
239+
}
240+
}

tfhe/src/core_crypto/commons/numeric/unsigned.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,12 @@ pub trait UnsignedInteger:
102102
/// Integer division rounding up.
103103
#[must_use]
104104
fn div_ceil(self, divisor: Self) -> Self;
105+
/// Interpret the value as signed and apply an arithmetic right shift, sign extending like on a
106+
/// signed value.
107+
#[must_use]
108+
fn arithmetic_shr(self, rhs: usize) -> Self {
109+
self.into_signed().shr(rhs).into_unsigned()
110+
}
105111
/// Return the casting of the current value to the signed type of the same size.
106112
fn into_signed(self) -> Self::Signed;
107113
/// Return a bit representation of the integer, where blocks of length `block_length` are

0 commit comments

Comments
 (0)