[libc-commits] [PATCH] D117684: [libc] Use __builtin_clz to find leading 1 in hypot

Siva Chandra via Phabricator via libc-commits libc-commits at lists.llvm.org
Wed Jan 19 09:08:56 PST 2022

sivachandra added inline comments.

Comment at: libc/src/__support/FPUtil/Hypot.h:26
+// __builtin_clz* rather than using the exactly-sized aliases from stdint.h
+// (such as uint32_t). There are 3 overloads even though 2 will only ever be
+// used by a specific platform, since unsigned long varies in size depending on
lntue wrote:
> builtin_clzl is just extended to either builtin_clz or builtin_clzll depending on the size of unsigned long. It might be better to keep uint32_t and uint64_t specializations and directly use __builtin_clz for uint32_t, builtin_clzll for uint64_t, together with static_assert(sizeof(unsigned int) == 4) and static_assert(sizeof(unsigned long long) == 8) instead?
Considering that `uint32_t` and `uint64_t` are just type sugar and not actual types, the existing code seems to me like the best approach and keeps it really straighforward with no assumptions of any sizes.

Comment at: libc/src/__support/FPUtil/Hypot.h:35
+  if (mant > 0) {
+    shift_length = 31 - __builtin_clz(mant);
Use a pattern like `sizeof(unsigned int) - 1` instead of hard coding here and below. For, in the `unsigned long` case, `63 - <...>`can be more than the width of `unsigned long`.

  rG LLVM Github Monorepo



More information about the libc-commits mailing list