-
Notifications
You must be signed in to change notification settings - Fork 223
Conversation
Codecov Report
@@ Coverage Diff @@
## main #747 +/- ##
==========================================
- Coverage 71.52% 70.49% -1.03%
==========================================
Files 337 343 +6
Lines 18443 18735 +292
==========================================
+ Hits 13191 13207 +16
- Misses 5252 5528 +276
Continue to review full report at Codecov.
|
e1e7af8
to
099d5ed
Compare
Compiling and tests passing 🎉🎉🎉🎉🎉 Unfortunately, we do have major regressions: git checkout main
cargo bench --bench aggregate --features compute_aggregate,benchmarks,simd -- "2\^20"
cargo bench --bench comparison_kernels --features compute_comparison,benchmarks,simd -- "2\^20"
git checkout portable_simd
cargo bench --bench aggregate --features compute_aggregate,benchmarks,simd -- "2\^20"
cargo bench --bench comparison_kernels --features compute_comparison,benchmarks,simd -- "2\^20"
|
Man, those are huge regressions :(. Given that it compiles, maybe just wait a bit and do a benchmark once in a while? |
I agree that we should not merge with these numbers. I will reach the working group for feedback and advice. |
8badb7d
to
81aacde
Compare
Converted to draft to indicate that this is blocked by a perf. regression. |
after some iterations on zulip, the tracking issues are: |
FYI, I also experimented with portable_simd today and created an llvm issue about a missing/incorrect optimization with |
Wow, awesome finding and summary @jhorstmann - thank you for reporting it over there 🙇 |
81aacde
to
ede1db6
Compare
What do you think of keeping this one aside of current SIMD (until the performance is comparable)? Then there is already SIMD support for the |
:) portable simd released the required APIs. I am re-benching this as of now to evaluate where we at wrt to performance. :) |
🤞 |
There are still 2 regressions, so this continues blocked.
|
fb754d7
to
46f361e
Compare
actually, the regression was only from the last commit, which tries to use the intrinsics for the |
There is some noise, but the overall is that we are now equal to packed simd:
|
46f361e
to
de4f3da
Compare
Awesome. Nice work! |
c52abe0
to
db5cdce
Compare
This PR replaces the
packed_simd
dependency by the nightly-availablestd::simd
(aka portable simd).Closes #580