[clang] 0a89825 - [APSInt] Fix bug in APSInt mentioned in https://github.com/llvm/llvm-project/issues/59515

Peter Rong via cfe-commits cfe-commits at lists.llvm.org
Fri Jan 20 13:25:21 PST 2023


Author: Peter Rong
Date: 2023-01-20T13:25:15-08:00
New Revision: 0a89825a289d149195be390003424adad026067f

URL: https://github.com/llvm/llvm-project/commit/0a89825a289d149195be390003424adad026067f
DIFF: https://github.com/llvm/llvm-project/commit/0a89825a289d149195be390003424adad026067f.diff

LOG: [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

Added: 
    

Modified: 
    clang/lib/CodeGen/CGDebugInfo.cpp
    llvm/include/llvm/ADT/APSInt.h
    llvm/lib/CodeGen/MIRParser/MIParser.cpp
    llvm/unittests/ADT/APSIntTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index f53a9d00ae5fd..b8e16918075e4 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<uint64_t> 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 c64cf303d4c04..90b988875a24d 100644
--- a/llvm/include/llvm/ADT/APSInt.h
+++ b/llvm/include/llvm/ADT/APSInt.h
@@ -85,12 +85,26 @@ class [[nodiscard]] APSInt : public APInt {
   }
   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<int64_t> tryExtValue() const {
+    return isRepresentableByInt64() ? std::optional<int64_t>(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 3d187fdef7750..7facd86b33251 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 932f54c19f18d..f804eba3ca83e 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); };


        


More information about the cfe-commits mailing list