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.

Sign up for GitHub

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

Jump to bottom

<bit>: Test _Countl_zero_bsr() internal helper #1120

Closed
StephanTLavavej opened this issue Jul 31, 2020 · 1 comment · Fixed by #1166
Closed

<bit>: Test _Countl_zero_bsr() internal helper #1120

StephanTLavavej opened this issue Jul 31, 2020 · 1 comment · Fixed by #1166
Labels
fixed Something works now, yay! test Related to test code

Comments

@StephanTLavavej
Copy link
Member

Recommended by @barcharcraz in #1108 (comment) .

After merging #1108, <bit> is going to have an internal helper _Countl_zero_bsr() for x86/x64 that's active only for very old processors; therefore it will receive no regular testing, although it is necessary for correctness. While we've manually tested it for this PR, we should investigate adding automated testing by directly calling the helper (which we generally avoid doing, but this merits a special exception).
@StephanTLavavej StephanTLavavej added the test Related to test code label Jul 31, 2020
@StephanTLavavej StephanTLavavej mentioned this issue Jul 31, 2020
Merged
@AlexGuteniev
Copy link
Contributor

I think a better alternative would be to have a fixture via force include, similar to this one:
https://github.com/microsoft/STL/blob/4e97255b3595a01f5268b98fad397e8579d14ed2/tests/std/include/force_include.hpp

It would add __isa_available = 0 (and __isa_enabled=0 on x86) in constructor of a static variable.

It would participate in test matrix.

A way to build such matrix for <bit> tests should be found, it will also be useful in the context of #954 resolution

AlexGuteniev added a commit to AlexGuteniev/STL that referenced this issue Aug 8, 2020
@AlexGuteniev
Resolves microsoft#1120
ae60448
AlexGuteniev added a commit to AlexGuteniev/STL that referenced this issue Aug 8, 2020
@AlexGuteniev
Revert "Resolves microsoft#1120"
90f1913 
This reverts commit ae60448.
AlexGuteniev added a commit to AlexGuteniev/STL that referenced this issue Aug 8, 2020
@AlexGuteniev
Resolves microsoft#1120
602017f
@AlexGuteniev AlexGuteniev mentioned this issue Aug 8, 2020
Merged
StephanTLavavej added a commit that referenced this issue Aug 18, 2020
@AlexGuteniev @StephanTLavavej @CaseyCarter
Test _Countl_zero_bsr with a dedicated test (#1166)
3de20f9 
Resolves #1120.

Co-authored-by: Stephan T. Lavavej <[email protected]>
Co-authored-by: Casey Carter <[email protected]>
@StephanTLavavej StephanTLavavej added the fixed Something works now, yay! label Aug 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
fixed Something works now, yay! test Related to test code
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Test _Countl_zero_bsr with a dedicated test AlexGuteniev/STL
2 participants
@AlexGuteniev @StephanTLavavej