-
Notifications
You must be signed in to change notification settings - Fork 461
Fix SIMD Fallback Import #232
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
Conversation
I also added some details to the references since I was looking through the papers anyway. (It follows the APA style used by Google Scholar.) |
762d335
to
a1cda64
Compare
b9affae
to
fe9529e
Compare
@@ -15,8 +15,13 @@ use memchr::{memchr, memchr2, memchr3}; | |||
use syntax; | |||
|
|||
use freqs::BYTE_FREQUENCIES; | |||
|
|||
#[cfg(feature = "simd-accel")] |
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.
I think you don't actually need this cfg
switch, right? The cfg
switch is done in lib.rs
so that simd_fallback
should never actually be explicitly used.
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.
You still import it as simd_fallback
, though. Might be a good idea to provide a shim module in lib.rs
depending on the feature flag (the shim in simd_accel
only deals with the target arch feature).
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.
Oh, I see. I thought I was using path
, but maybe it was somewhere else. Maybe this should be in lib.rs
?
#[cfg(not(feature = "simd-accel"))]
#[path = "simd_fallback/mod.rs"]
mod simd_accel;
I think that would work? Or something equivalent would be fine. I was just trying to stop the use of cfg
switching everywhere.
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.
Yeah, that should work. Defining the shim in lib.rs
is far better than using cfg here. I'll change this.
d9ad839
to
3ee97ee
Compare
Rebased. |
6f2bb0f
to
203c509
Compare
Add SIMD accelerated multiple pattern search.
@killercup Could you rebase this against master? Sorry for the churn. Thanks! |
3ee97ee
to
71cd186
Compare
@BurntSushi no problem! Rebased on master. |
Awesome, thanks so much! :D |
@killercup Hmm, it looks like this got merged into the |
Oh, Github didn't change the target branch :( just one second!
|
Fixes
cargo doc
and some other stuff.r? @BurntSushi