[LLVM][Support][NFC] Make multiple bitwise MathExtras functions constexpr

[gAlfonso-bit]

Summary

Submitted as an upstream fix

MathExtras has many useful tools. However, they aren't available to use in constexpr equations or functions. Originally this was a patch for swift llvm but it works upstream and as such I have put this PR upstream.

[gAlfonso-bit]

Made even more functions constexpr

[gAlfonso-bit]

Fix formatting

[gAlfonso-bit]

Fix compilation errors

[craig.topper]

Please upload patches with full context using -U999999 per the instructions here https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

[gAlfonso-bit]

In D107240#2926914, @craig.topper wrote:

Please upload patches with full context using -U999999 per the instructions here https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

Alright, I did.

[craig.topper]

llvm/include/llvm/Support/MathExtras.h

192

Does _BitScanReverse work in a constexpr in MSVC? The implementation in clang-cl doesn't work in constexpr, but clang-cl would use __builtin_clz.

[gAlfonso-bit]

llvm/include/llvm/Support/MathExtras.h

192

Yes it does. This is one of those exceptions where you can pass a reference because you don't read Index, but only write to it, and Index is in a constexpr anyway

[craig.topper]

llvm/include/llvm/Support/MathExtras.h

192

I tried it on godbolt and it didn't seem to work. Am I doing something wrong? https://godbolt.org/z/MnEY763E4

[gAlfonso-bit]

llvm/include/llvm/Support/MathExtras.h

192

Not sure, but so far LLVM compiles on Windows without a hitch, and the testbot is not having issues either

[gAlfonso-bit]

clang-tidy is complaining about function names, but changing every function instance would modify too many files

[craig.topper]

llvm/include/llvm/Support/MathExtras.h

192

Maybe that just means llvm never uses it in a context where it needs to be evaluated as a constexpr since your patch didn't add any new usages. So it might fail if someone tries to use the constexpr capability?

[gAlfonso-bit]

llvm/include/llvm/Support/MathExtras.h

192

Well, the other functions I made constexpr calls them, so that isn't it.

[gAlfonso-bit]

@craig.topper Only clang-tidy is having issues

[craig.topper]

I copied a chunk of this patch to godbolt and added two different constexpr usages of countLeadingZeros that both fail with MSVC https://godbolt.org/z/Maen4bac4 I'm guessing that because everything is a template, MSVC doesn't diagnose anything until you try to use it in constexpr context.

[craig.topper]

llvm/include/llvm/Support/MathExtras.h

116

Doesn't this mean that users of this that want to use it in constexpr can't compile that code with MSVC and would need a different solution just for MSVC? Is that useful for the project?

[gAlfonso-bit]

llvm/include/llvm/Support/MathExtras.h

116

No, since people who want to use it with constexpr but can't already exist, but this group gets smaller. Hopefully Microsoft finds a way, but there are downstream projects that would benefit from this already, like Swift.

[gAlfonso-bit]

By the way, it is not windows that gives clang tidy errors, but Linux.

[gAlfonso-bit]

Unfortunately it can't be constexpr for Windows at this time.

[gAlfonso-bit]

No build errors, only clang-tidy ones!

[gAlfonso-bit]

Remove contexpr from functions that call _BitScan Microsoft functions

[gAlfonso-bit]

This comment was removed by gAlfonso-bit.

llvm/include/llvm/Support/MathExtras.h

192

So I think I figured it out. Since I made the FIRST count function constexpr where the second value in the <> braces isn't constant, the compiler is using that instead of the non constexpr ones in these cases.

[craig.topper]

Is it possible to add unittests showing the count functions being used in constexpr contexts to prove that this will work with MSVC?

[gAlfonso-bit]

Unfortunately, as long as we are using c++14, there is actually no easy solution to this

[gAlfonso-bit]

@craig.topper I could use the libcpp_clz, which is constexpr, but that seems to be c++ 20 only.

[gAlfonso-bit]

I don't see how to progress here, so I am closing this for now

llvm/include/llvm/Support/MathExtras.h

 template <typename T, std::size_t SizeOfT> struct TrailingZerosCounter {
-  static unsigned count(T Val, ZeroBehavior) {
+  static constexpr unsigned count(T Val, ZeroBehavior) {
     if (!Val)

 template <typename T> struct TrailingZerosCounter<T, 4> {
-  static unsigned count(T Val, ZeroBehavior ZB) {
+  static constexpr unsigned count(T Val, ZeroBehavior ZB) {
     if (ZB != ZB_Undefined && Val == 0)

 template <typename T> struct TrailingZerosCounter<T, 8> {
-  static unsigned count(T Val, ZeroBehavior ZB) {
+  static constexpr unsigned count(T Val, ZeroBehavior ZB) {
     if (ZB != ZB_Undefined && Val == 0)

 template <typename T>
-unsigned countTrailingZeros(T Val, ZeroBehavior ZB = ZB_Width) {
+constexpr unsigned countTrailingZeros(T Val, ZeroBehavior ZB = ZB_Width) {
   static_assert(std::numeric_limits<T>::is_integer &&

 template <typename T, std::size_t SizeOfT> struct LeadingZerosCounter {
-  static unsigned count(T Val, ZeroBehavior) {
+  static constexpr unsigned count(T Val, ZeroBehavior) {
     if (!Val)

 template <typename T> struct LeadingZerosCounter<T, 4> {
-  static unsigned count(T Val, ZeroBehavior ZB) {
+  static constexpr unsigned count(T Val, ZeroBehavior ZB) {
     if (ZB != ZB_Undefined && Val == 0)

 template <typename T> struct LeadingZerosCounter<T, 8> {
-  static unsigned count(T Val, ZeroBehavior ZB) {
+  static constexpr unsigned count(T Val, ZeroBehavior ZB) {
     if (ZB != ZB_Undefined && Val == 0)

 template <typename T>
-unsigned countLeadingZeros(T Val, ZeroBehavior ZB = ZB_Width) {
+constexpr unsigned countLeadingZeros(T Val, ZeroBehavior ZB = ZB_Width) {
   static_assert(std::numeric_limits<T>::is_integer &&