[llvm-commits] CVS: llvm/lib/Transforms/Scalar/InstructionCombining.cpp

[Chris Lattner]

On Mar 19, 2007, at 5:02 PM, Chris Lattner wrote:

> On Mar 19, 2007, at 2:21 PM, Reid Spencer wrote:
>> On Mon, 2007-03-19 at 14:16 -0700, Chris Lattner wrote:
>>>> Implement isOneBitSet in terms of APInt::countPopulation.
>>>
>>>> @@ -3474,8 +3474,7 @@
>>>>  // isOneBitSet - Return true if there is exactly one bit set in
>>>> the specified
>>>>  // constant.
>>>>  static bool isOneBitSet(const ConstantInt *CI) {
>>>> -  uint64_t V = CI->getZExtValue();
>>>> -  return V && (V & (V-1)) == 0;
>>>> +  return CI->getValue().countPopulation() == 1;
>>>>  }
>>>
>>> Are you sure this is a good idea?  countPopulation is *much* slower
>>> than a couple of and's and a subtract.
>>
>> Its the temporary construction of APInts that makes the  
>> performance of
>> the existing algorithm poor. This will construct 3 temporaries, each
>> potentially with a malloc. Using countPopulation is constant time (I
>> agree, not super fast, but consistent) and much easier to read in the
>> code.
>
> Optimizing for the "big" apint case isn't interesting, please
> optimize for the small case.

Better yet, just call APInt::isPowerOf2.

-Chris