[llvm-commits] ConstantInt Patch #3

[Chris Lattner]

On Feb 26, 2007, at 3:09 PM, Reid Spencer wrote:
> This one avoids collisions in the IntConstants map by using APInt
> directly in the DenseMap. This was made possible by relaxing
> APInt::operator= and using a small structure for the key type so  
> that we
> can do operator== correctly for APInt (handling different bit widths).

Please eliminate the uint64_t version of the ConstantInt ctor:  
two ::get method's is fine, but you want just one ctor.  I don't see  
the use of the uint64_t version anyway.


+      APInt first;
+      const Type* second;

Please give these meaningful names.

+      bool operator==(const KeyTy& that) const {
+        if (this == &that)
+          return true;

You can drop this 'fast path' that will never be true.

+        return second == that.second &&
+                this->first.getBitWidth() == that.first.getBitWidth 
() &&
+                this->first == that.first;

If the 'second's match, how could the types not match.

Otherwise, looks good.  I didn't look at the APInt diff.

-Chris

> This passes tests. I'm likely to commit this soonish.
>
> Reid.
>
> On Mon, 2007-02-26 at 12:44 -0800, Reid Spencer wrote:
>> Chris,
>>
>> Here is round two. This is a slightly larger patch that also includes
>> the following changes:
>>
>>       * APInt::getValue() -> getSExtValue() and getZExtValue()
>>       * const APInt& ConstantInt::getValue() const { return Val; }
>>       * ConstantInt::get(const Type*, const APInt&)
>>       * APInt constructor for ConstantInt::get to use
>>       * APInt::getHashValue()  -- used for the DenseMap of  
>> ConstantInt
>>         values.
>>       * Changes in Constants.cpp to use getHashValue() for the key in
>>         the map of ConstantInt values.
>> Reid.
>>
>> On Mon, 2007-02-26 at 10:46 -0800, Reid Spencer wrote:
>>> Hi,
>>>
>>> I would like to commit this small patch but thought I should send  
>>> it out
>>> first. This changes ConstantInt to use an APInt object as its value
>>> instead of a uint64_t. The interface to ConstantInt still uses  
>>> uint64_t
>>> so the APInt is just being used as a glorified uint64_t as all bit
>>> widths are <= 64. This change passes DejaGnu, the MultiSource/ 
>>> Benchmarks
>>> test suite, and a few of the SPEC benchmarks that I tried (444.namd,
>>> 176.gcc, 473.astar).
>>>
>>> Okay to commit?
>>>
>>> Reid.
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> <ConstantInt3.patch>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits