-
Notifications
You must be signed in to change notification settings - Fork 170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Streebog dynamic dispatch + import of SSE4.1 & MMX implementations #367
base: master
Are you sure you want to change the base?
Conversation
'_mm_empty' call incorrectly wrapped in `#ifndef' instead of `#ifdef'. This was affecting correctness of benchmarks on i686: type 16 bytes 64 bytes 256 bytes 1024 bytes 8192 bytes 16384 bytes streebog512 -nan -nan -nan -nan -nan -nan Fixes: 0755b6e ("gosthash2012: Properly ifdef '_mm_empty' call") Signed-off-by: Vitaly Chikunov <[email protected]>
55bb508
to
b618bbc
Compare
gosthash2012_dispatch.h
Outdated
# error "No static implementation of g() is selected." | ||
# endif | ||
#endif | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Аццкий адЪ!
А диспетчер менее адским сделать не получается, да? :( |
Там всё рационально, только непривычно такое количество логики на препроцессоре. Но, оно нужно, чтоб сохранить уже существующую портабельность. Рассмотрим
|
Вот этот фрагмент скорее всего можно заменить на 1 макрос устанавливающий |
Такое всё равно придется делать при темплейтировании и в |
Пример диспетчера в libntru: https://github.com/tbuktu/libntru/blob/master/src/poly.c#L1082 общая схема такая-же, но всё проще так как этот код только под x86_64. Нам же так нельзя. А что именно показалось адом - темплейт или сам диспетчер? Темплейт ещё можно попытаться украсить или сделать без него (но это не без своих серьезных минусов), а диспетчер мне кажется таким и должен быть. |
Пример из BIKE https://github.com/open-quantum-safe/liboqs/blob/main/src/kem/bike/additional_r3/decode_internal.h#L61 |
Мало кто делает темплейт. Можно пойти на дублирование кода и перенести копии функции Это можно дальше развить и переименовать все эти |
LGTM |
Только сейчас подумал, что можно же было делать |
Это про что? |
Это по совокупности |
This implementation is functionally exact to previous code (just rearrangement), in preparation to run-time dispatch use. Signed-off-by: Vitaly Chikunov <[email protected]>
Currently, only switch between see2 and ref implementations. This should affect only i686, since x86_64 always have SSE2 unless compiled with `-mno-sse2'. This version of dynamic dispatch would work only on GCC-10 / Clang-3, otherwise fallback to static dispatch like before. Signed-off-by: Vitaly Chikunov <[email protected]>
Link: https://github.com/adegtyarev/streebog Signed-off-by: Vitaly Chikunov <[email protected]>
Merged and fixed two MMX implementations. For example, [1] uses SSE2 register types `__m128i', [2] GCC's `mmintrin.h' defines `_mm_cvtsi64_m64' only for `__x86_64__', but we need MMX exactly for IA-32, since x86_64 it have SSE2 in baseline. Link: https://github.com/adegtyarev/streebog Link: https://github.com/sjinks/php-stribog Signed-off-by: Vitaly Chikunov <[email protected]>
Переделал так — убрал прагмы (вместо них attribute |
Так оно стало гораздо читабельнее. |
Идея такая:
gosthash2012_dispatch.h
создаются разные версииg()
под разные микроархтектуры (sse2, sse4.1, и ref).__builtin_cpu_supports()
и__has_builtin()
(так что это работает только на свежих версиях gcc), но потом можно сделать определение через CPUID "как у всех").if
— это наиболее быстрый вариант из доступных.ps.
gosthash2012: Fix '_mm_empty' call
- надо в stable ветки перегнать, это багфикс.