Optimize gcd to use builtins

[Michael Schellenberger Costa (miscco)]

Description

This fixes #298 by moving the meat of the bitscan implementation to the limits header.

This seems unfortunate but is the smallest common denominator of and

Also we cannot go without limits anyway because the implementation needs it.

Checklist

Be sure you've read README.md and understand the scope of this repo.

If you're unsure about a box, leave it unchecked. A maintainer will help you.

• Identifiers in product code changes are properly _Ugly as per
  https://eel.is/c++draft/lex.name#3.1 or there are no product code changes.
• The STL builds successfully and all tests have passed (must be manually
  verified by an STL maintainer before automated testing is enabled on GitHub,
  leave this unchecked for initial submission).
• These changes introduce no known ABI breaks (adding members, renaming
  members, adding virtual functions, changing whether a type is an aggregate
  or trivially copyable, etc.).
• These changes were written from scratch using only this repository,
  the C++ Working Draft (including any cited standards), other WG21 papers
  (excluding reference implementations outside of proposed standard wording),
  and LWG issues as reference material. If they were derived from a project
  that's already listed in NOTICE.txt, that's fine, but please mention it.
  If they were derived from any other project (including Boost and libc++,
  which are not yet listed in NOTICE.txt), you must mention it here,
  so we can determine whether the license is compatible and what else needs
  to be done.

[Billy O'Neal (BillyONeal)]

Test failures look legit on this one :)

[Michael Schellenberger Costa (miscco)]

Hm I compiled for me, I will check again :(

[Michael Schellenberger Costa (miscco)]

Go home test you are drunk:

C:\agent_work\1\a\x86\out\inc\limits(1021,26): note: candidate template ignored: requirement '_Is_standard_unsigned_integer' was not satisfied [with _Ty = int]

So it seems that common_unsigned does funny things

[Michael Schellenberger Costa (miscco)]

The full error is:

In file included from C:\agent_work\1\s\llvm-project\libcxx\test\std\numerics\numeric.ops\numeric.ops.gcd\gcd.pass.cpp:16:
C:\agent_work\1\a\x86\out\inc\numeric(864,39): error: no matching function for call to '_Countl_zero'

return static_cast(_Countl_zero(~_Mask));
^~~~~~~~~~~~
C:\agent_work\1\a\x86\out\inc\numeric(898,39): note: in instantiation of function template specialization 'std::_Stl_bitscan_forward' requested here

const auto _Mx_trailing_zeroes = _Stl_bitscan_forward(_Mx_magnitude);

C:\agent_work\1\s\llvm-project\libcxx\test\std\numerics\numeric.ops\numeric.ops.gcd\gcd.pass.cpp(48,45): note: in instantiation of function template specialization 'std::gcd' requested here

assert(static_cast
(out) == std::gcd(value1, value2));

C:\agent_work\1\s\llvm-project\libcxx\test\std\numerics\numeric.ops\numeric.ops.gcd\gcd.pass.cpp(64,27): note: in instantiation of function template specialization 'test0' requested here

        accumulate &= test0(TC.x, TC.y, TC.expect);

                      ^

C:\agent_work\1\s\llvm-project\libcxx\test\std\numerics\numeric.ops\numeric.ops.gcd\gcd.pass.cpp(100,19): note: in instantiation of function template specialization 'do_test' requested here

static_assert(do_test(), "");

              ^

C:\agent_work\1\a\x86\out\inc\limits(1021,26): note: candidate template ignored: requirement '_Is_standard_unsigned_integer' was not satisfied [with _Ty = int]

_NODISCARD constexpr int _Countl_zero(const _Ty _Val) noexcept {

So in the test the type is signed_char that is passed into make_unsigned_t to deduce _Common_unsigned. I guess that should make it a unsigned char which is the type of _Mx_magnitude.

So the type passed to _Countl_zero is unsigned char where does it get the int from?

[Michael Schellenberger Costa (miscco)]

It seems that for whatever reason the template argument deduction fails. if I specify _Countl_zero<_Unsigned> it works.

That however smells terribly like a compiler bug

[Michael Schellenberger Costa (miscco)]

So tests are good, although I would like an opinion whether this is a compiler bug or just me not understanding C++.

The only thing I could com up with is that ~_Mask is changing the type but please tell me it doesnt

[Alex Guteniev (AlexGuteniev)]

that ~_Mask is changing the type

Why not?

https://eel.is/c++draft/expr.unary.op#10

Integral promotions are performed. The type of the result is the type of the promoted operand.

https://eel.is/c++draft/conv.prom#2

A prvalue of type char16_­t, char32_­t, or wchar_­t ([basic.fundamental]) can be converted to a prvalue of the first of the following types that can represent all the values of its underlying type: int, unsigned int, long int, unsigned long int, long long int, or unsigned long long int.

("Can" here means the conversion exists as integral promotion. The key verb here is "are" above. Promotions are performed)

[Stephan T. Lavavej (StephanTLavavej)]

Looks great, thank you! I tried my hardest, but the only issue I could find was a whitespace nitpick.

[Casey Carter (CaseyCarter)]

Thanks for the performance enhancement! 🐎