gnuradio / volk

The Vector Optimized Library of Kernels
http://libvolk.org
GNU Lesser General Public License v3.0
557 stars 202 forks source link

Simplify the Spiral-generated convolutional decoder #756

Closed argilo closed 9 months ago

argilo commented 10 months ago

Here I've continued cleaning up the convolutional decoder.

Since #755 is not merged yet, I've included it here, and will rebase the PR once it's merged.

In two additional commits I've made the following changes:

  1. Remove loop unrolling. Each of the SIMD protokernels had an unrolled loop that processed two bits of input in a row, as well as special processing for the leftover bit, if any. Since the processing step for each bit is large, I don't see any point in unrolling the loop, and removing the unrolling does not appear to have any impact on performance.
  2. Remove temporary variables. Spiral generates temporary variables for every single quantity and memory address, which makes the code difficult to understand. I removed these, which again appears to have no impact on performance but makes the code much more readable.

With these changes, the tests still pass. I also verified (on x86 and ARM) that various vector lengths still work:

for v in {0..1024}; do apps/volk_profile -n -R k7_r2 -v $v -i 10 | grep fail; done
argilo commented 9 months ago

Rebased.

jdemel commented 9 months ago

@argilo Thanks! I really appreciate your work here. Before I merge this, how are the test run? We had a lot of issues with this kernel in past. I'd like to make sure, we don't screw this kernel up again.

argilo commented 9 months ago

CI should do a decent job of testing the kernel now, since it verifies that all the protokernels produce identical bit streams when given noise as input. One thing that CI doesn't yet do is test with different vector lengths, which I did like so:

for v in {0..1024}; do apps/volk_profile -n -R k7_r2 -v $v -i 10 | grep fail; done
argilo commented 9 months ago

I ran that test on x86-64, ARM-64, and ARM-32.

jdemel commented 9 months ago

Great. Merging.