Simplify SIMD operations #284
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lamphamsy
force-pushed
the
eh/simd_multi_levels
branch
4 times, most recently
from
fcad024 to
fae4de8
Compare
slaperche-scality
suggested changes
Jun 13, 2019
Contributor
slaperche-scality
left a comment
slaperche-scality
left a comment
There was a problem hiding this comment.
Great PR, lot of simplifications.
It's really nice 🙂
test/quadiron_c_utest.cpp
Outdated
| } | ||
| } | ||
|
|
||
| // FIXME: for non-systematic FNT, `quadiron_fnt32_decode` will |
Contributor
There was a problem hiding this comment.
IMHO, it's cleaner to have a copy of _data/call quadiron_fnt32_decode on a copy instead of relying on the order of the test.
Contributor
Author
There was a problem hiding this comment.
yes, it's better. A copy of encoded fragments will be used to avoid such cases.
Some global variables are no longer used as we support only FNT for `w=T/2`, i.e. - FNT(257) using uint16_t - FNT(65537) using uint32_t To clarify reader, we use const reference for VecType variable if it's necessary.
The only support FNT with `w=T/2`, i.e. - FNT(257) using uint16_t - FNT(65537) using uint32_t gives advantages: - Operations are simplified by avoiding the argument cardinal. - Some branches can be avoided.
In butterfly operations, there are three cases depending on the coefficient `r`: - r = 1 - r = q - 1 - 1 < r < q - 1 We use an enum class to clarify such cases.
according changes in modular arithmetics
lamphamsy
force-pushed
the
eh/simd_multi_levels
branch
2 times, most recently
from
388d6c0 to
09181fb
Compare
Contributor
Author
|
@slaperche-scality : it's updated. Thanks for your next reviews :) |
1 task
- Reset metadata of decoded data: for non-systematic code, we use first k parities to store decoded data. These metadata should be reset. - Remove an useless initialisation of data_vec for non-systematic codes.
Note that for non-systematic FNT, in encoding and decoding of QuadIron C API, input data will be overwritten by output data: - `quadiron_fnt32_encode` will store first `k` parities in the input data buffers, and the next `m` parities in the usual parity buffers. - `quadiron_fnt32_decode` will overwrite input data pointers (that stores actually encoded fragments) by decoded data. In the test, coded fragments will be stored to use correct fragments. They will be used to check reconstructed data.
lamphamsy
force-pushed
the
eh/simd_multi_levels
branch
from
856fe81 to
7147e6f
Compare
slaperche-scality
approved these changes
Jun 18, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
NB: the PR is split/extracted from the big one #282
The purpose is to eliminate as much as possible branches, particularly in essential operations s.t. modular operations.
Currently, we support vectorized operations of FNT in the cases:
uint16_tanduint32_tuint32_tBut the support of FNT(257) using
uint32_traises additional branches of checking the cardinal of the field. And its encoding/decoding speeds are obviously smaller than the two other cases: FNT(257) usinguin16_tand FNT(65537) usinguint32_t.Hence, for FNT, we support only FNT(257) using
uin16_tand FNT(65537) usinguint32_t.