[llvm] r197272 - [block-freq] Add the method APInt::nearestLogBase2().
Duncan P. N. Exon Smith
dexonsmith at apple.com
Sun Dec 15 10:06:19 PST 2013
Hi Michael,
On 2013 Dec 13, at 12:47, Michael Gottesman <mgottesman at apple.com> wrote:
> + /// \returns the nearest log base 2 of this APInt. Ties round up.
> + unsigned nearestLogBase2() const {
> + // This is implemented by taking the normal log 2 of a number and adding 1
> + // to it if MSB - 1 is set.
Your use of MSB here is confusing, since MSB == BitWidth - 1 and
MSB - 1 == BitWidth - 2. I think the following is more accurate:
// This is implemented by taking logBase2() and adding 1 if the
// logBase2()-1 bit is set.
> + // We follow the model from logBase2 that logBase2(0) == UINT32_MAX. This
> + // works since if we have 0, MSB will be 0. Then we subtract one yielding
> + // UINT32_MAX. Finally extractBit of MSB - 1 will be UINT32_MAX implying
> + // that we get BitWidth - 1.
This comment is confusing. The prime culprits are the references
to MSB and (the now deleted) extractBit function, but the logic is
tough to follow regardless.
> + unsigned lg = logBase2();
> + return lg + unsigned(extractBit(std::min(lg - 1, BitWidth - 1)));
Is the following naive code somehow slower? I think it’s easier
to understand (and document).
unsigned lg = logBase2();
// Don’t check the lg - 1 bit in degenerate cases.
if (lg - 1 >= BitWidth)
return lg;
return lg + (*this)[lg - 1];
> +TEST(APIntTest, nearestLogBase2) {
I think your tests miss the most interesting corners, i.e.:
- APInt(BitWidth, 0).nearestLogBase2() == UINT32_MAX
- APInt(BitWidth, 1).nearestLogBase2() == 0
- APInt(BitWidth, -1).nearestLogBase2() == BitWidth
The first two, in particular, require special handling to prevent
invalid accesses into APInt.
More information about the llvm-commits
mailing list