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

test: add more tests for popcount #688

Merged
merged 2 commits into from
Jan 8, 2021
Merged

test: add more tests for popcount #688

merged 2 commits into from
Jan 8, 2021

Conversation

axic
Copy link
Member

@axic axic commented Jan 8, 2021

No description provided.

@axic axic requested review from chfast and gumb0 January 8, 2021 12:42
Copy link
Collaborator

@chfast chfast left a comment

Choose a reason for hiding this comment

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

Cases with single bit are missing.
Same cases should be added to TEST(execute_numeric, i{32,64}_popcnt).

@codecov
Copy link

codecov bot commented Jan 8, 2021

Codecov Report

Merging #688 (fc74d4e) into master (bd56198) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #688      +/-   ##
==========================================
- Coverage   99.31%   99.31%   -0.01%     
==========================================
  Files          72       72              
  Lines       10154    10148       -6     
==========================================
- Hits        10084    10078       -6     
  Misses         70       70              
Flag Coverage Δ
spectests 91.50% <ø> (ø)
unittests 99.31% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
test/unittests/cxx20_bit_test.cpp 100.00% <100.00%> (ø)
test/unittests/execute_numeric_test.cpp 100.00% <100.00%> (ø)

@axic
Copy link
Member Author

axic commented Jan 8, 2021

Cases with single bit are missing.

Added two single bit cases.

Same cases should be added to TEST(execute_numeric, i{32,64}_popcnt).

At this point though the tests should be changed to a pair then.

@chfast
Copy link
Collaborator

chfast commented Jan 8, 2021

At this point though the tests should be changed to a pair then.

Not sure what exactly do you mean, but you can create a new header file with test cases for clz, ctz, popcnt:

struct BitCountTestCase64
{
uint64_t value;
uint32_t expected_clz;
...
};

constexpr BitCountTestCase64 test_cases_64[] = {{0,32,32,0}, ...}; 

@axic axic requested a review from chfast January 8, 2021 14:34
test/unittests/cxx20_bit_test.cpp Outdated Show resolved Hide resolved
test/unittests/execute_numeric_test.cpp Outdated Show resolved Hide resolved
@axic
Copy link
Member Author

axic commented Jan 8, 2021

Any more test cases to be added?

@chfast
Copy link
Collaborator

chfast commented Jan 8, 2021

Any more test cases to be added?

Looks ok for now.

@axic axic force-pushed the popcount-tests branch 2 times, most recently from 1860a7b to 01764c7 Compare January 8, 2021 14:50
@axic axic requested a review from chfast January 8, 2021 14:50
@axic axic requested review from chfast and gumb0 January 8, 2021 16:41
Also use these test cases in execute_numeric_test.
@axic axic merged commit c7ed699 into master Jan 8, 2021
@axic axic deleted the popcount-tests branch January 8, 2021 17:16
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.

3 participants