-
Notifications
You must be signed in to change notification settings - Fork 84
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
Feature/comparisons #44
Conversation
Looks good, except I think we should use the shortest names for the unspecified layout masks, since those would be optimal on all architectures, whereas the full masks would be optimal only on some architectures. People will prefer the types with the shortest names and we should guide them towards the masks that work well everywhere as opposed to just on some architectures. |
I'm open to different naming, but I used |
How about using |
I think that the masks are sufficiently "weird" compared to the rest of the plain element types that |
I agree with Lokathor here, there's no particular reason to use abbreviations in what will generally be a not-common type. |
Aside from that quibble, this seems like it would be fine (if it compiled)! |
Awesome, I've been too busy to look at this recently but should have more time going forward. Fixing the tests should be easy enough, I just forgot to update them 🤞 |
My thoughts on the naming convention right now:
I'm not opposed to longer/more descriptive names for the layout-specified masks but frankly I couldn't think of anything that I was happy with. |
Also, very odd wasm errors: https://travis-ci.com/github/rust-lang/stdsimd/jobs/445441280#L4786-L4790
|
I'd guess that somehow the mask elements are set to something other than |
What about another option would be |
bitmN, wmN, emN etc are getting a little "C-ish" for my taste. Let's throw an underscore in there for the longer names of a specific mask form. |
Existing primitive types are all very abbreviated, and don't have any underscores AFAIK. |
Right, and the "default" mask type would stay just as For example
|
I generally agree with this. The opaque masks are already questionable, and should maybe be |
I forgot to mention, I don't think the opaque masks should be |
Note--my latest commit didn't fix the wasm issue. As far as I can tell it's fixed by a recent update to nightly. The bug only occurred with comparing the equality of the |
Closing this. Comparisons will be added as part of #49. |
This isn't quite done, but I figure I should get some more opinions before moving on to testing.
In this PR I introduce the framework for multiple mask types:
maskNxM
, and have unspecified layout. These can be conditionally compiled on architecture.MaskExt
trait.MaskExt
implementations.