Skip to content

Commit 30cc198

Browse files
authored
[APInt] Add default-disabled assertion to APInt constructor (#106524)
If the uint64_t constructor is used, assert that the value is actually a signed or unsigned N-bit integer depending on whether the isSigned flag is set. Provide an implicitTrunc flag to restore the previous behavior, where the argument is silently truncated instead. In this commit, implicitTrunc is enabled by default, which means that the new assertions are disabled and no actual change in behavior occurs. The plan is to flip the default once all places violating the assertion have been fixed. See #80309 for the scope of the necessary changes. The primary motivation for this change is to avoid incorrectly specified isSigned flags. A recurring problem we have is that people write something like `APInt(BW, -1)` and this works perfectly fine -- until the code path is hit with `BW > 64`. Most of our i128 specific miscompilations are caused by variants of this issue. The cost of the change is that we have to specify the correct isSigned flag (and make sure there are no excess bits) for uses where BW is always <= 64 as well.
1 parent e4e0dfb commit 30cc198

File tree

3 files changed

+29
-9
lines changed

3 files changed

+29
-9
lines changed

llvm/include/llvm/ADT/APInt.h

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,11 +106,26 @@ class [[nodiscard]] APInt {
106106
/// \param numBits the bit width of the constructed APInt
107107
/// \param val the initial value of the APInt
108108
/// \param isSigned how to treat signedness of val
109-
APInt(unsigned numBits, uint64_t val, bool isSigned = false)
109+
/// \param implicitTrunc allow implicit truncation of non-zero/sign bits of
110+
/// val beyond the range of numBits
111+
APInt(unsigned numBits, uint64_t val, bool isSigned = false,
112+
bool implicitTrunc = true)
110113
: BitWidth(numBits) {
114+
if (!implicitTrunc) {
115+
if (BitWidth == 0) {
116+
assert(val == 0 && "Value must be zero for 0-bit APInt");
117+
} else if (isSigned) {
118+
assert(llvm::isIntN(BitWidth, val) &&
119+
"Value is not an N-bit signed value");
120+
} else {
121+
assert(llvm::isUIntN(BitWidth, val) &&
122+
"Value is not an N-bit unsigned value");
123+
}
124+
}
111125
if (isSingleWord()) {
112126
U.VAL = val;
113-
clearUnusedBits();
127+
if (implicitTrunc || isSigned)
128+
clearUnusedBits();
114129
} else {
115130
initSlowCase(val, isSigned);
116131
}

llvm/lib/Support/APInt.cpp

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,8 @@ APInt& APInt::operator-=(uint64_t RHS) {
234234
APInt APInt::operator*(const APInt& RHS) const {
235235
assert(BitWidth == RHS.BitWidth && "Bit widths must be the same");
236236
if (isSingleWord())
237-
return APInt(BitWidth, U.VAL * RHS.U.VAL);
237+
return APInt(BitWidth, U.VAL * RHS.U.VAL, /*isSigned=*/false,
238+
/*implicitTrunc=*/true);
238239

239240
APInt Result(getMemory(getNumWords()), getBitWidth());
240241
tcMultiply(Result.U.pVal, U.pVal, RHS.U.pVal, getNumWords());
@@ -455,15 +456,17 @@ APInt APInt::extractBits(unsigned numBits, unsigned bitPosition) const {
455456
"Illegal bit extraction");
456457

457458
if (isSingleWord())
458-
return APInt(numBits, U.VAL >> bitPosition);
459+
return APInt(numBits, U.VAL >> bitPosition, /*isSigned=*/false,
460+
/*implicitTrunc=*/true);
459461

460462
unsigned loBit = whichBit(bitPosition);
461463
unsigned loWord = whichWord(bitPosition);
462464
unsigned hiWord = whichWord(bitPosition + numBits - 1);
463465

464466
// Single word result extracting bits from a single word source.
465467
if (loWord == hiWord)
466-
return APInt(numBits, U.pVal[loWord] >> loBit);
468+
return APInt(numBits, U.pVal[loWord] >> loBit, /*isSigned=*/false,
469+
/*implicitTrunc=*/true);
467470

468471
// Extracting bits that start on a source word boundary can be done
469472
// as a fast memory copy.
@@ -907,7 +910,8 @@ APInt APInt::trunc(unsigned width) const {
907910
assert(width <= BitWidth && "Invalid APInt Truncate request");
908911

909912
if (width <= APINT_BITS_PER_WORD)
910-
return APInt(width, getRawData()[0]);
913+
return APInt(width, getRawData()[0], /*isSigned=*/false,
914+
/*implicitTrunc=*/true);
911915

912916
if (width == BitWidth)
913917
return *this;
@@ -955,7 +959,7 @@ APInt APInt::sext(unsigned Width) const {
955959
assert(Width >= BitWidth && "Invalid APInt SignExtend request");
956960

957961
if (Width <= APINT_BITS_PER_WORD)
958-
return APInt(Width, SignExtend64(U.VAL, BitWidth));
962+
return APInt(Width, SignExtend64(U.VAL, BitWidth), /*isSigned=*/true);
959963

960964
if (Width == BitWidth)
961965
return *this;

llvm/unittests/ADT/APIntTest.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,11 +220,12 @@ TEST(APIntTest, i256) {
220220
}
221221

222222
TEST(APIntTest, i1) {
223-
const APInt neg_two(1, static_cast<uint64_t>(-2), true);
223+
const APInt neg_two(1, static_cast<uint64_t>(-2), true,
224+
/*implicitTrunc=*/true);
224225
const APInt neg_one(1, static_cast<uint64_t>(-1), true);
225226
const APInt zero(1, 0);
226227
const APInt one(1, 1);
227-
const APInt two(1, 2);
228+
const APInt two(1, 2, false, /*implicitTrunc=*/true);
228229

229230
EXPECT_EQ(0, neg_two.getSExtValue());
230231
EXPECT_EQ(-1, neg_one.getSExtValue());

0 commit comments

Comments
 (0)