Skip to content

[llvm:ir] Add support for constant data exceeding 4GiB #126481

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions clang/lib/CodeGen/CGExprConstant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -364,14 +364,14 @@ bool ConstantAggregateBuilder::split(size_t Index, CharUnits Hint) {
// FIXME: If possible, split into two ConstantDataSequentials at Hint.
CharUnits ElemSize = getSize(CDS->getElementType());
replace(Elems, Index, Index + 1,
llvm::map_range(llvm::seq(0u, CDS->getNumElements()),
[&](unsigned Elem) {
llvm::map_range(llvm::seq(uint64_t(0u), CDS->getNumElements()),
[&](uint64_t Elem) {
return CDS->getElementAsConstant(Elem);
}));
replace(Offsets, Index, Index + 1,
llvm::map_range(
llvm::seq(0u, CDS->getNumElements()),
[&](unsigned Elem) { return Offset + Elem * ElemSize; }));
llvm::seq(uint64_t(0u), CDS->getNumElements()),
[&](uint64_t Elem) { return Offset + Elem * ElemSize; }));
return true;
}

Expand Down
16 changes: 8 additions & 8 deletions llvm/include/llvm/IR/Constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -617,34 +617,34 @@ class ConstantDataSequential : public ConstantData {

/// If this is a sequential container of integers (of any size), return the
/// specified element in the low bits of a uint64_t.
uint64_t getElementAsInteger(unsigned i) const;
uint64_t getElementAsInteger(uint64_t i) const;

/// If this is a sequential container of integers (of any size), return the
/// specified element as an APInt.
APInt getElementAsAPInt(unsigned i) const;
APInt getElementAsAPInt(uint64_t i) const;

/// If this is a sequential container of floating point type, return the
/// specified element as an APFloat.
APFloat getElementAsAPFloat(unsigned i) const;
APFloat getElementAsAPFloat(uint64_t i) const;

/// If this is an sequential container of floats, return the specified element
/// as a float.
float getElementAsFloat(unsigned i) const;
float getElementAsFloat(uint64_t i) const;

/// If this is an sequential container of doubles, return the specified
/// element as a double.
double getElementAsDouble(unsigned i) const;
double getElementAsDouble(uint64_t i) const;

/// Return a Constant for a specified index's element.
/// Note that this has to compute a new constant to return, so it isn't as
/// efficient as getElementAsInteger/Float/Double.
Constant *getElementAsConstant(unsigned i) const;
Constant *getElementAsConstant(uint64_t i) const;

/// Return the element type of the array/vector.
Type *getElementType() const;

/// Return the number of elements in the array or vector.
unsigned getNumElements() const;
uint64_t getNumElements() const;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you also need to update other APIs? ConstantDataSequential::getElementAsInteger() etc.

Copy link
Contributor Author

@pzzp pzzp Feb 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

other APIs do not affect the emission of large constant data, so we do not modify them for now.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the AsmPrinter, at least, is using getElementAsInteger. Please try some tests that don't hit the CDS->isString() optimized path.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I update getElementAsInteger and getElementAsFloat, it seems that these two are used in codegen.
I test it with this script's generation

#!/bin/bash
cat << EOF
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
EOF

# generate @ARR = global [1073741825 x i32] [...], align 16
for (( i = 0; i<1024; i++)); do
  ARR_1K+="i32 $((RANDOM % 97)), "
done
for (( i = 0; i<1024; i++)); do
  ARR_1M+="$ARR_1K"
done
echo -n "@ARR = global [$((1024 * 1024 * 1024 + 1)) x i32] ["
for (( i = 0; i<$((1024)); i++)); do
  echo -n "$ARR_1M"
done
echo -n 'i32 233], align 16'

cat << EOF
!llvm.module.flags = !{!0, !1, !2, !3}

!0 = !{i32 1, !"wchar_size", i32 4}
!1 = !{i32 8, !"PIC Level", i32 2}
!2 = !{i32 7, !"PIE Level", i32 2}
!3 = !{i32 7, !"uwtable", i32 2}
EOF

I checked that the result is ok. The last number in result asm is 233 and the size is 4294967300

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I add these test to regression tests? but it is really large

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding a python script as a regression test isn't necessarily an issue, but I assume it would take an excessively large amount of time/memory to process a 4GB file. We don't really have any place in-tree for a test like that. Leaving it out is probably fine... it's here for reference if we need it in the future.


/// Return the size (in bytes) of each element in the array/vector.
/// The size of the elements is known to be a multiple of one byte.
Expand Down Expand Up @@ -684,7 +684,7 @@ class ConstantDataSequential : public ConstantData {
}

private:
const char *getElementPointer(unsigned Elt) const;
const char *getElementPointer(uint64_t Elt) const;
};

//===----------------------------------------------------------------------===//
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Analysis/TargetTransformInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -919,7 +919,7 @@ TargetTransformInfo::getOperandInfo(const Value *V) {
} else if (const auto *CDS = dyn_cast<ConstantDataSequential>(V)) {
OpInfo = OK_NonUniformConstantValue;
bool AllPow2 = true, AllNegPow2 = true;
for (unsigned I = 0, E = CDS->getNumElements(); I != E; ++I) {
for (uint64_t I = 0, E = CDS->getNumElements(); I != E; ++I) {
if (auto *CI = dyn_cast<ConstantInt>(CDS->getElementAsConstant(I))) {
AllPow2 &= CI->getValue().isPowerOf2();
AllNegPow2 &= CI->getValue().isNegatedPowerOf2();
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Analysis/ValueTracking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6355,7 +6355,7 @@ Value *llvm::isBytewiseValue(Value *V, const DataLayout &DL) {

if (ConstantDataSequential *CA = dyn_cast<ConstantDataSequential>(C)) {
Value *Val = UndefInt8;
for (unsigned I = 0, E = CA->getNumElements(); I != E; ++I)
for (uint64_t I = 0, E = CA->getNumElements(); I != E; ++I)
if (!(Val = Merge(Val, isBytewiseValue(CA->getElementAsConstant(I), DL))))
return nullptr;
return Val;
Expand Down
8 changes: 4 additions & 4 deletions llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2803,7 +2803,7 @@ void ModuleBitcodeWriter::writeConstants(unsigned FirstVal, unsigned LastVal,
cast<ConstantDataSequential>(C)->isString()) {
const ConstantDataSequential *Str = cast<ConstantDataSequential>(C);
// Emit constant strings specially.
unsigned NumElts = Str->getNumElements();
uint64_t NumElts = Str->getNumElements();
// If this is a null-terminated string, use the denser CSTRING encoding.
if (Str->isCString()) {
Code = bitc::CST_CODE_CSTRING;
Expand All @@ -2814,7 +2814,7 @@ void ModuleBitcodeWriter::writeConstants(unsigned FirstVal, unsigned LastVal,
}
bool isCStr7 = Code == bitc::CST_CODE_CSTRING;
bool isCStrChar6 = Code == bitc::CST_CODE_CSTRING;
for (unsigned i = 0; i != NumElts; ++i) {
for (uint64_t i = 0; i != NumElts; ++i) {
unsigned char V = Str->getElementAsInteger(i);
Record.push_back(V);
isCStr7 &= (V & 128) == 0;
Expand All @@ -2831,10 +2831,10 @@ void ModuleBitcodeWriter::writeConstants(unsigned FirstVal, unsigned LastVal,
Code = bitc::CST_CODE_DATA;
Type *EltTy = CDS->getElementType();
if (isa<IntegerType>(EltTy)) {
for (unsigned i = 0, e = CDS->getNumElements(); i != e; ++i)
for (uint64_t i = 0, e = CDS->getNumElements(); i != e; ++i)
Record.push_back(CDS->getElementAsInteger(i));
} else {
for (unsigned i = 0, e = CDS->getNumElements(); i != e; ++i)
for (uint64_t i = 0, e = CDS->getNumElements(); i != e; ++i)
Record.push_back(
CDS->getElementAsAPFloat(i).bitcastToAPInt().getLimitedValue());
}
Expand Down
6 changes: 3 additions & 3 deletions llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3612,9 +3612,9 @@ static void emitGlobalConstantDataSequential(
return AP.OutStreamer->emitBytes(CDS->getAsString());

// Otherwise, emit the values in successive locations.
unsigned ElementByteSize = CDS->getElementByteSize();
uint64_t ElementByteSize = CDS->getElementByteSize();
if (isa<IntegerType>(CDS->getElementType())) {
for (unsigned I = 0, E = CDS->getNumElements(); I != E; ++I) {
for (uint64_t I = 0, E = CDS->getNumElements(); I != E; ++I) {
emitGlobalAliasInline(AP, ElementByteSize * I, AliasList);
if (AP.isVerbose())
AP.OutStreamer->getCommentOS()
Expand All @@ -3624,7 +3624,7 @@ static void emitGlobalConstantDataSequential(
}
} else {
Type *ET = CDS->getElementType();
for (unsigned I = 0, E = CDS->getNumElements(); I != E; ++I) {
for (uint64_t I = 0, E = CDS->getNumElements(); I != E; ++I) {
emitGlobalAliasInline(AP, ElementByteSize * I, AliasList);
emitGlobalConstantFP(CDS->getElementAsAPFloat(I), ET, AP);
}
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1844,7 +1844,7 @@ SDValue SelectionDAGBuilder::getValueImpl(const Value *V) {
if (const ConstantDataSequential *CDS =
dyn_cast<ConstantDataSequential>(C)) {
SmallVector<SDValue, 4> Ops;
for (unsigned i = 0, e = CDS->getNumElements(); i != e; ++i) {
for (uint64_t i = 0, e = CDS->getNumElements(); i != e; ++i) {
SDNode *Val = getValue(CDS->getElementAsConstant(i)).getNode();
// Add each leaf value from the operand to the Constants list
// to form a flattened list of all the values.
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/IR/AsmWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1679,7 +1679,7 @@ static void WriteConstantInternal(raw_ostream &Out, const Constant *CV,
WriterCtx.TypePrinter->print(ETy, Out);
Out << ' ';
WriteAsOperandInternal(Out, CA->getElementAsConstant(0), WriterCtx);
for (unsigned i = 1, e = CA->getNumElements(); i != e; ++i) {
for (uint64_t i = 1, e = CA->getNumElements(); i != e; ++i) {
Out << ", ";
WriterCtx.TypePrinter->print(ETy, Out);
Out << ' ';
Expand Down
23 changes: 10 additions & 13 deletions llvm/lib/IR/Constants.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2850,24 +2850,22 @@ bool ConstantDataSequential::isElementTypeCompatible(Type *Ty) {
return false;
}

unsigned ConstantDataSequential::getNumElements() const {
uint64_t ConstantDataSequential::getNumElements() const {
if (ArrayType *AT = dyn_cast<ArrayType>(getType()))
return AT->getNumElements();
return cast<FixedVectorType>(getType())->getNumElements();
}


uint64_t ConstantDataSequential::getElementByteSize() const {
return getElementType()->getPrimitiveSizeInBits()/8;
return getElementType()->getPrimitiveSizeInBits() / 8;
}

/// Return the start of the specified element.
const char *ConstantDataSequential::getElementPointer(unsigned Elt) const {
const char *ConstantDataSequential::getElementPointer(uint64_t Elt) const {
assert(Elt < getNumElements() && "Invalid Elt");
return DataElements+Elt*getElementByteSize();
return DataElements + Elt * getElementByteSize();
}


/// Return true if the array is empty or all zeros.
static bool isAllZeros(StringRef Arr) {
for (char I : Arr)
Expand Down Expand Up @@ -3106,8 +3104,7 @@ Constant *ConstantDataVector::getSplat(unsigned NumElts, Constant *V) {
return ConstantVector::getSplat(ElementCount::getFixed(NumElts), V);
}


uint64_t ConstantDataSequential::getElementAsInteger(unsigned Elt) const {
uint64_t ConstantDataSequential::getElementAsInteger(uint64_t Elt) const {
assert(isa<IntegerType>(getElementType()) &&
"Accessor can only be used when element is an integer");
const char *EltPtr = getElementPointer(Elt);
Expand All @@ -3127,7 +3124,7 @@ uint64_t ConstantDataSequential::getElementAsInteger(unsigned Elt) const {
}
}

APInt ConstantDataSequential::getElementAsAPInt(unsigned Elt) const {
APInt ConstantDataSequential::getElementAsAPInt(uint64_t Elt) const {
assert(isa<IntegerType>(getElementType()) &&
"Accessor can only be used when element is an integer");
const char *EltPtr = getElementPointer(Elt);
Expand Down Expand Up @@ -3155,7 +3152,7 @@ APInt ConstantDataSequential::getElementAsAPInt(unsigned Elt) const {
}
}

APFloat ConstantDataSequential::getElementAsAPFloat(unsigned Elt) const {
APFloat ConstantDataSequential::getElementAsAPFloat(uint64_t Elt) const {
const char *EltPtr = getElementPointer(Elt);

switch (getElementType()->getTypeID()) {
Expand All @@ -3180,19 +3177,19 @@ APFloat ConstantDataSequential::getElementAsAPFloat(unsigned Elt) const {
}
}

float ConstantDataSequential::getElementAsFloat(unsigned Elt) const {
float ConstantDataSequential::getElementAsFloat(uint64_t Elt) const {
assert(getElementType()->isFloatTy() &&
"Accessor can only be used when element is a 'float'");
return *reinterpret_cast<const float *>(getElementPointer(Elt));
}

double ConstantDataSequential::getElementAsDouble(unsigned Elt) const {
double ConstantDataSequential::getElementAsDouble(uint64_t Elt) const {
assert(getElementType()->isDoubleTy() &&
"Accessor can only be used when element is a 'float'");
return *reinterpret_cast<const double *>(getElementPointer(Elt));
}

Constant *ConstantDataSequential::getElementAsConstant(unsigned Elt) const {
Constant *ConstantDataSequential::getElementAsConstant(uint64_t Elt) const {
if (getElementType()->isHalfTy() || getElementType()->isBFloatTy() ||
getElementType()->isFloatTy() || getElementType()->isDoubleTy())
return ConstantFP::get(getContext(), getElementAsAPFloat(Elt));
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Target/TargetLoweringObjectFile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,14 +104,14 @@ static bool isSuitableForBSS(const GlobalVariable *GV) {
static bool IsNullTerminatedString(const Constant *C) {
// First check: is we have constant array terminated with zero
if (const ConstantDataSequential *CDS = dyn_cast<ConstantDataSequential>(C)) {
unsigned NumElts = CDS->getNumElements();
uint64_t NumElts = CDS->getNumElements();
assert(NumElts != 0 && "Can't have an empty CDS");

if (CDS->getElementAsInteger(NumElts-1) != 0)
return false; // Not null terminated.

// Verify that the null doesn't occur anywhere else in the string.
for (unsigned i = 0; i != NumElts-1; ++i)
for (uint64_t i = 0; i != NumElts - 1; ++i)
if (CDS->getElementAsInteger(i) == 0)
return false;
return true;
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/X86/X86MCInstLower.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1581,7 +1581,7 @@ static void printConstant(const Constant *COp, unsigned BitWidth,
bool IsInteger = EltTy->isIntegerTy();
bool IsFP = EltTy->isHalfTy() || EltTy->isFloatTy() || EltTy->isDoubleTy();
unsigned EltBits = EltTy->getPrimitiveSizeInBits();
unsigned E = std::min(BitWidth / EltBits, CDS->getNumElements());
unsigned E = std::min(BitWidth / EltBits, (unsigned)CDS->getNumElements());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
unsigned E = std::min(BitWidth / EltBits, (unsigned)CDS->getNumElements());
uint64_t E = std::min((uint64_t)(BitWidth / EltBits), CDS->getNumElements());

Copy link
Contributor Author

@pzzp pzzp Feb 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We tested, actually it work well on our llvm-15 based TVM for over half a year...
The function printConstant is only invoked for constants retrieved from the constant pool, which is unlikely to be a large constant.
The purpose of our patch is to enable emitting constant data exceeding 4GB, hence we avoid to do extensive modifications like replace unsigned withuint64_t` across the codebase.

This script gen_large_string.py can generate a test.

print(
'''; ModuleID = 'large_string.c'
source_filename = "large_string.c"
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
'''
)


print('@.str = private unnamed_addr constant [4294968320 x i8] c"', end='')
for i in range(4294968320//1024):
    print('abcdefgh' * 128, end='')
print('", align 1')

print(
'''@s = dso_local local_unnamed_addr global ptr @.str, align 8

!llvm.module.flags = !{!0, !1, !2, !3}
!llvm.ident = !{!4}

!0 = !{i32 1, !"wchar_size", i32 4}
!1 = !{i32 8, !"PIC Level", i32 2}
!2 = !{i32 7, !"PIE Level", i32 2}
!3 = !{i32 7, !"uwtable", i32 2}
!4 = !{!"clang version 21.0.0git (https://github.com/llvm/llvm-project.git 0395fd9680a348c6afc52a165453222c02a3cd48)"}
''')
python3 gen_large_string.py > large_string.ll
llc large_string.ll

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll give you that printConstant isn't really important.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"abcdefgh" is not a good string to use because its length is a power of 2; that means you won't notice if integer indexes wrap.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much for your meticulous review.
I update the test to this gen_large_string.sh

#!/bin/bash
cat << EOF
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
EOF

# generate @S = constant [4294967303 x i8] c"......", align 1
for (( i = 0; i<128; i++)); do
  S_1K+='abcdefgh'
done
for (( i = 0; i<1024; i++)); do
  S_1M+=$S_1K
done
echo -n "@S = constant [$((1024 * 1024 * 1024 * 4 + 7)) x i8] c\""
for (( i = 0; i<$((1024 * 4)); i++)); do
  echo -n $S_1M
done
echo -n 'last7ch'
echo '", align 1'

cat << EOF
!llvm.module.flags = !{!0, !1, !2, !3}

!0 = !{i32 1, !"wchar_size", i32 4}
!1 = !{i32 8, !"PIC Level", i32 2}
!2 = !{i32 7, !"PIE Level", i32 2}
!3 = !{i32 7, !"uwtable", i32 2}
EOF
bash gen_large_string.sh > a.ll
llc a.ll

It generate right result, with last 7 char are 'last7ch'.
image
image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just switch it to uint64_t even if it "works"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want to make too many changes. Changing unsigned to uint64_t here will lead to a series of modifications to printConstant, but printConstant doesn't actually handle constants exceeding 4GiB

if ((BitWidth % EltBits) == 0) {
for (unsigned I = 0; I != E; ++I) {
if (I != 0)
Expand Down