Skip to content
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

Faster Mask::firstOne() #237

Open
wants to merge 2 commits into
base: 1.4
Choose a base branch
from
Open

Conversation

andrelrt
Copy link

@andrelrt andrelrt commented Mar 21, 2019

This change removes the undefined behaviour on firstOne method when used on an empty mask.

I did that for 2 reasons:

  1. Undefined behaviour is not exactly a nice thing. And more important,
  2. Is faster than check beforehand if the mask is empty.

This change made a 5% speed improvement on my tests [1] on average. The code is a generic lower_bound using SIMD instructions and uses Vc.

I choose to return Mask::Size because will change the function return pattern just a little. Instead of return [0, Size), returns [0, Size]. One alternative is firstOne receive the default value for empty masks, I think the speedup should be the same.

[1] https://github.com/andrelrt/VcAlgo/blob/CodeMigration/include/VcAlgo/details/lower_bound.h

@amadio amadio self-assigned this Apr 8, 2022
@amadio
Copy link
Collaborator

amadio commented Apr 8, 2022

@andrelrt Can you please rebase this on the current branch, so that it can go through the CI? I'd like to get through all the current merge requests and merge what makes sense before we make a new release. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants