feat(gpu): div_rem_2_2_blocks #2693
Conversation
bbarbakadze
force-pushed
the
bb/feat/div_rem_2_2
branch
from
9db1492 to
0da79f9
Compare
bbarbakadze
force-pushed
the
bb/feat/div_rem_2_2
branch
from
ef26e7a to
84e4363
Compare
There was a problem hiding this comment.
Hey @bbarbakadze! Thanks a lot for the changes! Overall it looks really good: I think it could be optimized more, maybe to try and reuse some buffers/luts. The main caveat in this implementation is the number of streams used: could you measure throughput on 8xH100-SXM5 to see how it compares to current throughput please? Thanks!
| } | ||
| }; | ||
|
|
||
| template <typename Torus> struct int_sub_and_propagate { |
There was a problem hiding this comment.
This and the int_bitop_buffer are just changing places right? Could you put them where they were before to avoid unnecessary conflicts later on please?
| delete c3; | ||
| delete q1; | ||
| delete q2; | ||
| delete q3; |
There was a problem hiding this comment.
After every delete, could you also reset every pointer to nullptr please? Andrei's PR is not merged yet but that's what will become the new pattern very soon.
| as_radix_ciphertext_slice<Torus>(d_msb, d, slice_start, slice_end); | ||
| comparison_blocks->num_radix_blocks = d_msb->num_radix_blocks; | ||
| if (d_msb->num_radix_blocks == 0) { | ||
| cuda_memset_async((Torus *)out_boolean_block->ptr, 0, |
There was a problem hiding this comment.
Here I think you could use set_zero_radix_ciphertext_slice_async instead
| gpu_count: u32, | ||
| mem_ptr_void: *mut *mut i8, | ||
| ); | ||
| } |
There was a problem hiding this comment.
I don't know why the bindings file format is changing, could be worth checking what happens if you revert the changes here?
PR content/description
New optimized algorithm for div_rem when using message=2 and carry = 2 parameters.
Check-list: