[PATCH] D142078: [llvm] Move bit counting functions to bit.h (NFC)

[Kazu Hirata via Phabricator via llvm-commits]

kazu added a comment.

In D142078#4064301 <https://reviews.llvm.org/D142078#4064301>, @dblaikie wrote:

> Is the plan to migrate callers over to this eventually, and remove the countNnn wrappers?

Yes.



================
Comment at: llvm/include/llvm/ADT/bit.h:44-54
+#ifdef _MSC_VER
+// Declare these intrinsics manually rather including intrin.h. It's very
+// expensive, and MathExtras.h is popular.
+// #include <intrin.h>
+extern "C" {
+unsigned char _BitScanForward(unsigned long *_Index, unsigned long _Mask);
+unsigned char _BitScanForward64(unsigned long *_Index, unsigned __int64 _Mask);
----------------
dblaikie wrote:
> Instead of this, perhaps we could write small/simple wrappers, avoiding the need to forward declare things from platform headers? & possibly wrapping the basic operations/differences between the platforms in a more direct way, then exposing these higher level primitives based on those abstractions?
Do you mean something like this?

```
namespace detail {
#if __has_builtin(__builtin_clz) || defined(__GNUC__)
inline constexpr bool has_clz = true;
inline unsigned clz(unsigned Val) {
  return __builtin_clz(Val)
}
#else defined(_MSC_VER)
inline constexpr bool has_clz = true;
inline unsigned clz(unsigned Val) {
  unsigned long Index;
  _BitScanForward(&Index, Val);
  return Index;
}
#else
inline constexpr bool has_clz = false;
#endif
}  // namespace detail
```

Then implement `llvm::countl_zero` with `llvm::detail::has_clz` and `llvm::detail:clz`?

If so, I like this idea, but I don't see how we can avoid forward-declaring things.  The comment says `intrin.h` is very expensive although I don't know whether the comment is still applicable today or how old the comment is.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142078/new/

https://reviews.llvm.org/D142078