Basic features: Vec8f and Vec4f
#1
Conversation
…unsics to be inlined
…nts; CI: doctests on 32-bits
|
CI problem: rust compiler won't tell about missing intrinsics on no-AVX build. Instead it fails to inline those but then successfully links them, resulting AVX instructions working. |
src/vec4f.rs
Outdated
| #[inline(always)] | ||
| fn from(value: Vec4f) -> Self { | ||
| let mut result = MaybeUninit::<Self>::uninit(); | ||
| unsafe { |
There was a problem hiding this comment.
Когда напишем реализацию без sse, надо будет этот кусок проверить под miri
There was a problem hiding this comment.
Я зачем без sse? Я прогнал под miri, оно не наругалось. Зато нашел другую багу (as_ptr вместо as_mut_ptr). Надо в CI тоже добавить запуск под miri, но с ним есть проблемы:
оно как-то странно работает с.cargo/config.toml(но это ладно)cargo cleanпомог- оно пока не умеет AVX, но есть готовый PR
- наверное из-за этого
compile_fail-тесты дляextract_constкомпилируются при запуском подmiri
- наверное из-за этого
src/vec8f.rs
Outdated
|
|
||
| use crate::{common::SIMDVector, macros::vec_overload_operator, Vec4f}; | ||
|
|
||
| #[cfg(avx)] |
There was a problem hiding this comment.
У тебя в этом файле ооочень много таких проверок на cfg(avx)
Может лучше так:
// vec8f.rs
#[cfg(avx)]
mod vec8f_avx;
#[cfg(avx)]
pub use vec8f_avx::*;
#[cfg(not(avx))]
mod vec8f_noavx;
#[cfg(not(avx))]
pub use vec8f_noavx::*;И писать реализацию уже в vec8f_avx.rs и vec8f_noavx.rs
Обсуждаемо, но нужны аргументы почему лучше все в одном файле
There was a problem hiding this comment.
И да, зачем no_avx если есть cfg(not(avx))?
There was a problem hiding this comment.
Тут потому что мне лень писать :)
There was a problem hiding this comment.
У тебя в этом файле ооочень много таких проверок на
cfg(avx)
Может лучше так:// vec8f.rs #[cfg(avx)] mod vec8f_avx; #[cfg(avx)] pub use vec8f_avx::*; #[cfg(not(avx))] mod vec8f_noavx; #[cfg(not(avx))] pub use vec8f_noavx::*;И писать реализацию уже в
vec8f_avx.rsиvec8f_noavx.rs
Обсуждаемо, но нужны аргументы почему лучше все в одном файле
Я тоже о таком думал, то тогда придется дублировать интерфейс и документацию. Последнее можно избежать, если делать публичные методы общими, а в них вызывать приватные функции, которые и объявлены в платформо-специфисных модулях, но это все равно много кода. Короче мне кажется, что лучше держать имплементации рядом.
Еще там функции с разными имплементациями обычно однострочные, и держать разные их версий рядом громоздко не выглядит. А вот выносить в отдельные модули будет громоздким.
There was a problem hiding this comment.
А в оригинале все рядом лежит у Fog-а?
There was a problem hiding this comment.
А в оригинале все рядом лежит у Fog-а?
У него реализация без AVX отдельно, а оптимизации рядом.
There was a problem hiding this comment.
Можно сделать так, что документация всегда идет из не-avx модуля через cfg(all(avx, not(doc))), а avx - просто реализации. Тогда надо будет аккуратно следить, что интерфейс один и тот же, но по идее тесты это решат.
There was a problem hiding this comment.
Другой вариант, который мне пока нравится больше всего, но потребует больше всего кода:
Сделать три файла:
vec4f.rsvec4f_fallback.rsvec4f_avx.rs
ВVec4f.rsсделать такое:
#[cfg(avx)]
mod impl;
#[cfg(not(avx))]
mod impl;
#[repr(transparent)]
struct Vec4f(impl::Vec4f);
impl Vec4f {
// и тут doc
pub fn extract(&self, index: usize) -> f32 {
self.0.extract
}
}There was a problem hiding this comment.
Другой вариант, который мне пока нравится больше всего, но потребует больше всего кода: Сделать три файла:
1. `vec4f.rs` 2. `vec4f_fallback.rs` 3. `vec4f_avx.rs` В `Vec4f.rs` сделать такое:#[cfg(avx)] mod impl; #[cfg(not(avx))] mod impl; #[repr(transparent)] struct Vec4f(impl::Vec4f); impl Vec4f { // и тут doc pub fn extract(&self, index: usize) -> f32 { self.0.extract } }
Осознать, что это действительно лучший способ, было не тривиально...
src/vec4f.rs
Outdated
| /// Represents a packed vector of 4 single-precision floating-point values. [`__m128`] wrapper. | ||
| #[derive(Clone, Copy)] | ||
| pub struct Vec4f { | ||
| xmm: __m128, |
There was a problem hiding this comment.
А ты уверен, что все процессоры подерживают все инструкции, которые ты тут исопльзовал?
Не нужно cfg проверить какой-нибудь, и fallback на тупую реализацию?
There was a problem hiding this comment.
я вроде ручками проверил, но да, надо это как-то затестить
There was a problem hiding this comment.
По хорошему должен быть файл-интерфейс, который подключает тебе те структуры, которые доступны для текущих флагов компиляции. Ты же можешь делать кросс-компиляцию, когда компилируешь под инструкции, которые на твоей системе не поддерживаются. Но например у программы будет диспетчер в рантайме. Потому обратите внимание не только на проверки текущей машины, а именно на таргет-компиляцию.
There was a problem hiding this comment.
По хорошему должен быть файл-интерфейс, который подключает тебе те структуры, которые доступны для текущих флагов компиляции. Ты же можешь делать кросс-компиляцию, когда компилируешь под инструкции, которые на твоей системе не поддерживаются. Но например у программы будет диспетчер в рантайме. Потому обратите внимание не только на проверки текущей машины, а именно на таргет-компиляцию.
А, в этом смысле... Я тут всё пишу с мыслью, что у меня есть хотя бы sse, и этот модуль в принципе подключается в #cfg(sse), и вроде все инструкции SSE-шные.
Я комментарием в гитхабе писал проблему: нужно как-то проверять, что у меня действительно не используются AVX-фичи на сборке без AVX. Там на все интринзики, конечно, есть #[target_feature(enable = "avx")], но это просто добавляет мне UB в код ¯\_(ツ)_/¯. Как это делать — пока что не знаю. Можете подсказать, если знаете?
…ding (that's according to RFC 1574)
Closes #12