[cfe-commits] [llvm-commits] [Patch] Update to APSInt

[Richard Trieu]

On Sun, Jul 22, 2012 at 7:39 AM, Chris Lattner <clattner at apple.com> wrote:

>
> On Jul 18, 2012, at 6:16 PM, Richard Trieu wrote:
>
> On Wed, Jul 18, 2012 at 11:19 AM, Chandler Carruth <chandlerc at google.com>wrote:
>
>> On Wed, Jul 18, 2012 at 11:12 AM, John McCall <rjmccall at apple.com> wrote:
>>
>>> On Jul 18, 2012, at 2:28 AM, Chandler Carruth wrote:
>>>
>>> On Tue, Jul 17, 2012 at 11:47 PM, Richard Smith <richard at metafoo.co.uk>wrote:
>>>
>>>> On Tue, Jul 17, 2012 at 11:26 PM, Chandler Carruth <
>>>> chandlerc at google.com> wrote:
>>>>>
>>>>> I find the definition of APInt's operator== deeply troubling. Why
>>>>> *assert* if the bit widths aren't equal? That doesn't make a lot of sense
>>>>> to me. The function that it calls to actually implement it turns around and
>>>>> considers mismatched bitwdiths to cause *inequality*.
>>>>>
>>>>> However it seems that there is a very simple definition of equality we
>>>>> could use instead: zero-extended equality for APInt, and sign-extended
>>>>> equality for APSInt. I wonder if there would be general support for making
>>>>> APInt::operator== and APSInt::operator== work in this more rational model...
>>>>>
>>>>
>>>> APInt has no knowledge of whether its high bit is a sign bit, so always
>>>> zero-extending will be wrong in the case where it is in fact a sign bit.
>>>> APSInt does know this, so if we want to support heterogenous comparisons,
>>>> we should sign-extend if the APSInt is signed, and zero-extend if it is
>>>> unsigned. Heterogenous comparison on APInt is fundamentally unsafe, so
>>>> asserting there seems reasonable to me.
>>>>
>>>
>>> Well, Nick's comment may obviate the extension question, which leaves us
>>> with a simpler problem of comparing the same sizes for equality or
>>> inequality. I don't actually see any problems comparing same-sized APInts
>>> and APSInts for equality or inequality as-if they were both APInts. Given
>>> two APSInts, I think that the signedness should participate in the equality
>>> test though...
>>>
>>>
>>> It seems silly for APInt to treat bitwidth inequality as an illegal
>>> operation but APSInt to treat it as a semantic difference.  APInt's
>>> assertions *do* find bugs;  I would much rather extend those to APSInt than
>>> have it forge a new contract.
>>>
>>
>> I never intended to suggest inconsistency between the two. I just didn't
>> understand the motivation for the assertion even at the APInt layer. Nick
>> provided that though, which was all I needed. =]
>>
>> New patch.  Switched int parameters to int64_t.  APSInt::operator!= now
> refers to APSInt::operator==
>
>
> LGTM, thanks!
>
> -Chris
>
>
Committed at r160641 and r160642.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120723/2080e360/attachment.html>