[PATCH] D116522: [ValueTracking][SelectionDAG] Rename ComputeMinSignedBits->ComputeMaxSignificantBits. NFC

Tue Jan 4 07:15:24 PST 2022

craig.topper added a comment.

In D116522#3219566 <https://reviews.llvm.org/D116522#3219566>, @spatel wrote:

> In D116522#3219081 <https://reviews.llvm.org/D116522#3219081>, @foad wrote:
>
>>> Rename APInt::getMinSignedBits->getSignificantBits
>>
>> Why drop "Signed" from the name? Now we have `getActiveBits` for the unsigned version of this functionality and `getSignificantBits` for the signed version, which I don't think is very intuitive.
>
> Good point. So we should standardize on either "active" or "significant". Go with "active" since it already exists and is shorter? 
> We could also standardize the verb in these names. Currently, it is "get", "count", or "compute".
> If we unify those, it could be something like this across all of these APIs:
> get[Max]ActiveBitsSigned()
> get[Max]ActiveBitsUnsigned()

I don't think we should apply the "Active" term to signed. In my head Active is kind of associated with bits that are ones. There's also APIntgetActiveWords that is implemented using getActiveBits to determine the last word that has ones in it.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116522