From 0a89825a289d149195be390003424adad026067f Mon Sep 17 00:00:00 2001 From: Peter Rong Date: Mon, 26 Dec 2022 18:05:11 -0800 Subject: [PATCH] [APSInt] Fix bug in APSInt mentioned in https://github.com/llvm/llvm-project/issues/59515 Also provide a `tryExtValue()` API like APInt did in D139683 Reviewed By: RKSimon, efriedma Differential Revision: https://reviews.llvm.org/D140059 --- clang/lib/CodeGen/CGDebugInfo.cpp | 16 ++++++++++++---- llvm/include/llvm/ADT/APSInt.h | 16 +++++++++++++++- llvm/lib/CodeGen/MIRParser/MIParser.cpp | 7 +++++-- llvm/unittests/ADT/APSIntTest.cpp | 17 +++++++++++++++++ 4 files changed, 49 insertions(+), 7 deletions(-) diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index f53a9d0..b8e1691 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -5401,10 +5401,18 @@ void CGDebugInfo::EmitGlobalVariable(const ValueDecl *VD, const APValue &Init) { llvm::DIExpression *InitExpr = nullptr; if (CGM.getContext().getTypeSize(VD->getType()) <= 64) { // FIXME: Add a representation for integer constants wider than 64 bits. - if (Init.isInt()) - InitExpr = - DBuilder.createConstantValueExpression(Init.getInt().getExtValue()); - else if (Init.isFloat()) + if (Init.isInt()) { + const llvm::APSInt &InitInt = Init.getInt(); + std::optional InitIntOpt; + if (InitInt.isUnsigned()) + InitIntOpt = InitInt.tryZExtValue(); + else if (auto tmp = InitInt.trySExtValue(); tmp.has_value()) + // Transform a signed optional to unsigned optional. When cpp 23 comes, + // use std::optional::transform + InitIntOpt = (uint64_t)tmp.value(); + if (InitIntOpt) + InitExpr = DBuilder.createConstantValueExpression(InitIntOpt.value()); + } else if (Init.isFloat()) InitExpr = DBuilder.createConstantValueExpression( Init.getFloat().bitcastToAPInt().getZExtValue()); } diff --git a/llvm/include/llvm/ADT/APSInt.h b/llvm/include/llvm/ADT/APSInt.h index c64cf30..90b9888 100644 --- a/llvm/include/llvm/ADT/APSInt.h +++ b/llvm/include/llvm/ADT/APSInt.h @@ -85,12 +85,26 @@ public: } using APInt::toString; + /// If this int is representable using an int64_t. + bool isRepresentableByInt64() const { + // For unsigned values with 64 active bits, they technically fit into a + // int64_t, but the user may get negative numbers and has to manually cast + // them to unsigned. Let's not bet the user has the sanity to do that and + // not give them a vague value at the first place. + return isSigned() ? isSignedIntN(64) : isIntN(63); + } + /// Get the correctly-extended \c int64_t value. int64_t getExtValue() const { - assert(getMinSignedBits() <= 64 && "Too many bits for int64_t"); + assert(isRepresentableByInt64() && "Too many bits for int64_t"); return isSigned() ? getSExtValue() : getZExtValue(); } + std::optional tryExtValue() const { + return isRepresentableByInt64() ? std::optional(getExtValue()) + : std::nullopt; + } + APSInt trunc(uint32_t width) const { return APSInt(APInt::trunc(width), IsUnsigned); } diff --git a/llvm/lib/CodeGen/MIRParser/MIParser.cpp b/llvm/lib/CodeGen/MIRParser/MIParser.cpp index 3d187fd..7facd86 100644 --- a/llvm/lib/CodeGen/MIRParser/MIParser.cpp +++ b/llvm/lib/CodeGen/MIRParser/MIParser.cpp @@ -1804,9 +1804,12 @@ bool MIParser::parseRegisterOperand(MachineOperand &Dest, bool MIParser::parseImmediateOperand(MachineOperand &Dest) { assert(Token.is(MIToken::IntegerLiteral)); const APSInt &Int = Token.integerValue(); - if (Int.getMinSignedBits() > 64) + if (auto SImm = Int.trySExtValue(); Int.isSigned() && SImm.has_value()) + Dest = MachineOperand::CreateImm(*SImm); + else if (auto UImm = Int.tryZExtValue(); !Int.isSigned() && UImm.has_value()) + Dest = MachineOperand::CreateImm(*UImm); + else return error("integer literal is too large to be an immediate operand"); - Dest = MachineOperand::CreateImm(Int.getExtValue()); lex(); return false; } diff --git a/llvm/unittests/ADT/APSIntTest.cpp b/llvm/unittests/ADT/APSIntTest.cpp index 932f54c..f804eba 100644 --- a/llvm/unittests/ADT/APSIntTest.cpp +++ b/llvm/unittests/ADT/APSIntTest.cpp @@ -62,6 +62,13 @@ TEST(APSIntTest, getUnsigned) { EXPECT_EQ(UINT64_C(0) - 7, APSInt::getUnsigned(-7).getZExtValue()); } +TEST(APSIntTest, isRepresentableByInt64) { + ASSERT_TRUE(APSInt(APInt(3, 7), true).isRepresentableByInt64()); + ASSERT_TRUE(APSInt(APInt(128, 7), true).isRepresentableByInt64()); + ASSERT_TRUE(APSInt(APInt(128, 7), false).isRepresentableByInt64()); + ASSERT_TRUE(APSInt(APInt(64, -1), false).isRepresentableByInt64()); + ASSERT_FALSE(APSInt(APInt(64, (uint64_t)-1), true).isRepresentableByInt64()); +} TEST(APSIntTest, getExtValue) { EXPECT_TRUE(APSInt(APInt(3, 7), true).isUnsigned()); EXPECT_TRUE(APSInt(APInt(3, 7), false).isSigned()); @@ -76,6 +83,16 @@ TEST(APSIntTest, getExtValue) { EXPECT_EQ(9, APSInt(APInt(4, -7), true).getExtValue()); EXPECT_EQ(-7, APSInt(APInt(4, -7), false).getExtValue()); } +TEST(APSIntTest, tryExtValue) { + ASSERT_EQ(-7, APSInt(APInt(64, -7), false).tryExtValue().value_or(42)); + ASSERT_EQ(42, APSInt(APInt(128, -7), false).tryExtValue().value_or(42)); + ASSERT_EQ(-1, + APSInt(APInt::getAllOnes(128), false).tryExtValue().value_or(42)); + ASSERT_EQ(42, APSInt(APInt(64, -7), true).tryExtValue().value_or(42)); + ASSERT_EQ(1, APSInt(APInt(128, 1), true).tryExtValue().value_or(42)); + ASSERT_EQ(42, + APSInt(APInt::getAllOnes(128), true).tryExtValue().value_or(42)); +} TEST(APSIntTest, compareValues) { auto U = [](uint64_t V) { return APSInt::getUnsigned(V); }; -- 2.7.4