[lldb-dev] Signedness of scalars built from APInt(s)

[Zachary Turner via lldb-dev]

It seems like we have 3 uses of this constructor in LLDB.

IRInterpreter.cpp: Constructs a Scalar for an llvm::Constant.
IRForTarget.cpp:  Constructs a Scalar for an llvm::Constant.
ClangASTContext.cpp: bitcasts an APFloat to an APInt.

The first two we should just treat constants in LLVM IR as signed, so we
could construct an APSInt at the call-sites with signed=true.

The third seems like a bug, we should just have a Scalar constructor that
takes an APFloat directly.  We already have one that takes a float and it
just sets the `APFloat m_float` member variable, I don't know why we're
jumping through this bitcast hoop (which is probably wrong for negative
floats anyway).

On Fri, Jan 4, 2019 at 3:38 PM Zachary Turner <zturner at google.com> wrote:

> On Fri, Jan 4, 2019 at 3:23 PM Jonas Devlieghere <jonas at devlieghere.com>
> wrote:
>
>> On Fri, Jan 4, 2019 at 3:13 PM Zachary Turner <zturner at google.com> wrote:
>>
>>> I don't think #2 is a correct change.  Just because the sign bit is set
>>> doesn't mean it's signed.  Is the 4-byte value 0x10000000 signed or
>>> unsigned?  It's a trick question, because there's not enough information!
>>> If it was written "int x = 0x10000000" then it's signed (and negative).  If
>>> it was written "unsigned x = 0x10000000" then it's unsigned (and
>>> positive).  What about the 4-byte value 0x1?  Still a trick!  If it was
>>> written "int x = 1" then it's signed (and positive), and if it was written
>>> "unsigned x = 1" then it's unsigned (and positive).
>>>
>>> My point is that signedness of the *type* does not necessarly imply
>>> signedness of the value, and vice versa.
>>>
>>> APInt is purely a bit-representation and a size, there is no information
>>> whatsoever about whether type *type* is signed.  It doesn't make sense to
>>> say "is this APInt negative?" without additional information.
>>>
>>> With APSInt, on the other hand, it does make sense to ask that
>>> question.  If you have an APSInt where isSigned() is true, *then* you can
>>> use the sign bit to determine whether it's negative.  And if you have an
>>> APSInt where isSigned() is false, then the "sign bit" is not actually a
>>> sign bit at all, it is just an extra power of 2 for the unsigned value.
>>>
>>> This is my understanding of the classes, someone correct me if I'm wrong.
>>>
>>
>>> IIUC though, the way to fix this is by using APSInt throughout the
>>> class, and delete all references to APInt.
>>>
>>
>> I think we share the same understanding. If we know at every call site
>> whether the type is signed or not then I totally agree, we should only use
>> APSInt. The reason I propose doing (2) first is for the first scenario you
>> described, where you don't know. Turning it into an explicit APSInt is as
>> bad as using an APInt and looking at the value. The latter has the
>> advantage that it conveys that you don't know, while the other may or may
>> not be a lie.
>>
>
> Do we ever not know though?  And if so, then why don't we know whether the
> type is supposed to be signed or unsigned?  Because guessing is always
> going to be wrong sometimes.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20190104/10e529dd/attachment.html>