Skip to content

<bit> test coverage for /arch:AVX2 - #2150

Merged
Stephan T. Lavavej (StephanTLavavej) merged 2 commits into
microsoft:mainfrom
AlexGuteniev:bit_of_coverage
Aug 27, 2021
Merged

Stephan T. Lavavej (StephanTLavavej) merged 2 commits into
microsoft:mainfrom
AlexGuteniev:bit_of_coverage

Conversation

@AlexGuteniev

Copy link
Copy Markdown
Contributor

Resolve #2149

@AlexGuteniev
Alex Guteniev (AlexGuteniev) requested a review from a team as a code owner August 22, 2021 13:58
@StephanTLavavej Stephan T. Lavavej (StephanTLavavej) added the test Related to test code label Aug 23, 2021
@StephanTLavavej Stephan T. Lavavej (StephanTLavavej) changed the title <bit> test coverage <bit> test coverage for /arch:AVX2 Aug 23, 2021
@StephanTLavavej

Copy link
Copy Markdown
Member
  • Verified that no other tests including <bit> would substantially benefit from these options.
  • Verified that both tests are sensitive to /arch:AVX2 because bit_ceil() calls countl_zero().
  • tests.py and features.py already know about /arch:AVX2 because floating_point_model_matrix.lst uses it.
  • Accordingly, we don't have to worry about dev/test machine support (also double-checked the list of supported CPUs).
  • popcount() is sensitive to /arch:AVX, but that's implied by /arch:AVX2, and I don't believe that adding /arch:AVX to the matrix would provide useful coverage:

    STL/stl/inc/limits

    Lines 1094 to 1103 in f75c7f5

    #if _HAS_POPCNT_INTRINSICS
    template <class _Ty>
    _NODISCARD int _Checked_x86_x64_popcount(const _Ty _Val) noexcept {
    constexpr int _Digits = numeric_limits<_Ty>::digits;
    #ifndef __AVX__
    const bool _Definitely_have_popcnt = __isa_available >= __ISA_AVAILABLE_SSE42;
    if (!_Definitely_have_popcnt) {
    return _Popcount_fallback(_Val);
    }
    #endif // !defined(__AVX__)

@StephanTLavavej

Copy link
Copy Markdown
Member

I'm mirroring this to an MSVC-internal PR. Changes can still be pushed during final review, but please notify me if that happens.

@StephanTLavavej

Copy link
Copy Markdown
Member

Thanks for adding this test coverage and preventing future regressions! 😻 🎉 ✔️

@AlexGuteniev
Alex Guteniev (AlexGuteniev) deleted the bit_of_coverage branch August 27, 2021 05:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test Related to test code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

<bit>: Expand test coverage to cl /arch:AVX2 and clang-cl /arch:AVX2

4 participants