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

2019-01-04 Thread Davide Italiano via lldb-dev
On Fri, Jan 4, 2019 at 3:54 PM Zachary Turner wrote: > > 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

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

2019-01-04 Thread 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

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

2019-01-04 Thread Zachary Turner via lldb-dev
On Fri, Jan 4, 2019 at 3:23 PM Jonas Devlieghere wrote: > On Fri, Jan 4, 2019 at 3:13 PM Zachary Turner 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 0x1000 signed or >> unsigned? It's a trick question,

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

2019-01-04 Thread Greg Clayton via lldb-dev
Option 3? Add APSInt as a new member? Current: Scalar::Type m_type; llvm::APInt m_integer; llvm::APFloat m_float; bool m_ieee_quad = false; Option #3: Scalar::Type m_type; llvm::APInt m_uint; llvm::APSInt m_sint; llvm::APFloat m_float; bool m_ieee_quad = false; I don't know

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

2019-01-04 Thread Jonas Devlieghere via lldb-dev
On Fri, Jan 4, 2019 at 3:13 PM Zachary Turner 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 0x1000 signed or > unsigned? It's a trick question, because there's not enough information! > If it was written

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

2019-01-04 Thread Zachary Turner via lldb-dev
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 0x1000 signed or unsigned? It's a trick question, because there's not enough information! If it was written "int x = 0x1000" then it's signed (and negative). If it was

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

2019-01-04 Thread Jonas Devlieghere via lldb-dev
If I understand the situation correctly I think we should do both. I'd start by doing (2) to improve the current behavior and add a constructor for APSInt. We can then audit the call sites and migrate to APSInt where it's obvious that the type is signed. That should match the semantics of both

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

2019-01-04 Thread Davide Italiano via lldb-dev
On Fri, Jan 4, 2019 at 1:57 PM Davide Italiano wrote: > > While adding support for 512-bit integers in `Scalar`, I figured I > could add some coverage. > > TEST(ScalarTest, Signedness) { > auto s1 = Scalar(APInt(32, 12, false /* isSigned */)); > auto s2 = Scalar(APInt(32, 12, true /* isSigned

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

2019-01-04 Thread Davide Italiano via lldb-dev
While adding support for 512-bit integers in `Scalar`, I figured I could add some coverage. TEST(ScalarTest, Signedness) { auto s1 = Scalar(APInt(32, 12, false /* isSigned */)); auto s2 = Scalar(APInt(32, 12, true /* isSigned */ )); ASSERT_EQ(s1.GetType(), Scalar::e_uint); // fails