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

Add If statement benchmarks #2517

Merged
merged 1 commit into from
Aug 30, 2022
Merged

Add If statement benchmarks #2517

merged 1 commit into from
Aug 30, 2022

Conversation

a74nh
Copy link
Contributor

@a74nh a74nh commented Jul 1, 2022

No description provided.

@a74nh
Copy link
Contributor Author

a74nh commented Jul 1, 2022

These tests are added to test the if conversion pass being added here: dotnet/runtime#67894

The performance of csel vs a branch is dependant on the frequency of the branches being taken or not. If the closer to a 50/50 split then the faster csel will perform. As the branch choices gets closer to always taken or always not taken, then using branches becomes faster.

Therefore for the simple case, I've used a %2 test to approximate 50/50.

For the other cases, getting to 50/50 isn't quite so simple. Keeping with %2 seemed the best approach.

@a74nh
Copy link
Contributor Author

a74nh commented Jul 1, 2022

Some performance diffs when using csel instructions: dotnet/runtime#67894 (comment)

Copy link
Member

@kunalspathak kunalspathak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@danmoseley
Copy link
Member

@kunalspathak should we merge then?

@kunalspathak
Copy link
Member

Sure, I am not sure if centos 7 failures are known, but otherwise, we can merge it.

@danmoseley
Copy link
Member

Yes they are known.

@danmoseley danmoseley merged commit 850e426 into dotnet:main Aug 30, 2022
@danmoseley
Copy link
Member

Thanks @a74nh sorry for overlooking this.

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

Successfully merging this pull request may close these issues.

3 participants