On Feb 27, 2007, at 3:35 PM, Reid Spencer wrote:
> On Tue, 2007-02-27 at 14:17 -0800, Chris Lattner wrote:
>>>> This seems like a poor choice of API. RoundDoubleToAPInt should
>>>> take
>>>> a result bitwidth. Without this, this appears like it could make a
>>>> "large" APInt, then truncate it down to a small one when converting
>>>> from double, which is very inefficient.
>>>
>>> Done.
>>
>> Thanks, but why the default argument?
>
> For compatibility with existing code that didn't have that parameter.
> Call me lazy but I didn't feel like messing with all the test cases
> that
> use this.
The test cases are temporary, right? When they go, the default will
go? If you want, I can file a bug so that we don't forget this.
>>
>> The the interface takes a pointer to uint32_t or uint64_t, it doesn't
>> matter what the internal size is, it has to work with that array.
>
> I have since made a bunch of things in the APInt interface private,
> including these enum values.
Thanks!
>>
>> Additionally, why are you checking this here? What does
>> APINT_BITS_PER_WORD have to do with whether or not this operation can
>> be constant folded?
>
> It doesn't. Don't know what I was thinking when I wrote that.
ok
>> I'm just saying that instead of: APInt::doFoo(x, bool), we should
>> have APInt::doFooSigned(x) and APInt::doFooUnsigned(x). This is an
>> interface change, not a functionality change.
>
> Did it several hours ago.
thanks
>>
>>>>>
>>>>> @@ -382,7 +412,7 @@
>>>>> Ops.reserve(numOps);
>>>>> for (unsigned i = 0; i < numOps; ++i) {
>>>>> const Constant *Op =
>>>>> - (i == idxVal) ? Elt : Constant::getNullValue(Elt->getType
>>>>> ());
>>>>> + (idxVal == i) ? Elt : Constant::getNullValue(Elt->getType
>>>>> ());
>>>>
>>>> This sort of change makes the diff more noisy, and there is no
>>>> apparent reason for doing it. Generally, the mutating value (i) is
>>>> on the LHS, why do this (in many places)?
>>>
>>> Becasue there is no operator==(int, APInt) but there is an
>>> APInt::operator==(int).
>>
>> Okay, can you add the other one?
>
> You have two options:
>
> 1. Overload global operator== (bad practice in my books)
> 2. Deal with calls like: APIntOps::operator==(i, idxVal) which is
> uglier
> than
> just reversing the operands.
>
> What's the big deal about reversing the order of the operands?
What's the problem with #1? It's already overloaded for many other
things, as is global operator<< (for printing).
-Chris
_______________________________________________
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
