AVX2 suppord - Githubissues

glenvt18 / libdvbcsa

GNU General Public License v2.0

11 stars 16 forks source link

AVX2 suppord #5

Closed mkaluza closed 6 years ago

mkaluza commented 6 years ago

I did another PR as I wanted to leave that branch and discussion intact

I cleaned it up (in my terms). I realize there are functions left in avx header. If you want them converted to macros, I can do it (although I haven't seen any speed difference, and macros in contrast to functions are completely undebugable, which is a problem sometimes).

If you want something else changed just tell me.

JDarnley commented 6 years ago

If I may make a comment. I think you should change the "uninterleaved" in a comment added in 1cf9864 to "deinterleaved" because that is the term used elsewhere in the code.

glenvt18 commented 6 years ago

@mkaluza Looks good.

Some notes.

Inline functions are OK.
Any experimental stuff (block_sbox_avxN) can't be merged into the master branch. Any alternative option must be supported by configure.ac. No commented blocks of code either. If you want to save this work for future use, keep it in your experimental branch. Master is not the appropriate place for that.
Please stick to the libdvbcsa style: no tabs, 2 chars indents, no C++ comments, braces, etc.

mkaluza commented 6 years ago

@glenvt18 ok, I think that's all

glenvt18 commented 6 years ago

@mkaluza Almost:) The patch is OK. I'd like to ask you to make some cosmetic changes to the commit message:

add -> Add
Replace tabs with spaces
Add a short description of what has been done: AVX2 table lookup instruction, how it boosts performance compared to AVX2 + plain lookup, 16 bit fetch, deinterleaving done in the same loop, etc. Just shortly. It will help a viewer to catch the idea quickly.

glenvt18 commented 6 years ago

@mkaluza Found a small issue:

#ifndef DVBCSA_AVX_H_
#define DVBCSA_AVX_H_

should be DVBCSA_AVX2_H_

glenvt18 commented 6 years ago

@mkaluza Those numbers look very nice. Great work! Thanks a lot for your contribution.