[libc-commits] [PATCH] D127097: [libc][math] Improved FBits performance and readablity.

[Kirill Okhotnikov via Phabricator via libc-commits]

orex added inline comments.


================
Comment at: libc/src/__support/FPUtil/FloatProperties.h:49
 
-  static constexpr uint32_t BIT_WIDTH = sizeof(BitsType) << 3;
+  static constexpr uint32_t BIT_WIDTH = sizeof(BitsType) * 8;
 
----------------
sivachandra wrote:
> orex wrote:
> > sivachandra wrote:
> > > Why is multiplication by 8 preferred over shift by 3?
> > Just to improve readability. Everybody knows, that byte has 8 bits. But in previous representation it should:
> > 1) Understand that `<< ` refers to explicit multiplication, but not bit/masks operations as everywhere else in the code.
> > 2) Remember that 2^3=8.
> > Of course, it is not a big problem, but the code is still complex and need attention. And we should not add extra complications for nothing.
> To be absolutely clear, I am OK with using `* 8` in this patch. At the same time, I would have found the reasoning more appealing if it was like, "we should let the compilers choose the best instructions for the semantic of multiplying by 8."
To be absolutely clear, I'll use your suggestion in the future. It is absolutely reasonable. But in this particular case, this is a` constexpr`, therefore the "best instruction" never appears in the code. It will be done during the compile time, so will not affect runtime at all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127097