Skip to content
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.

Sign up for GitHub

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

Write a constant value to a file without buffering #2874

Merged
merged 9 commits into from
Jul 22, 2024
Merged

Write a constant value to a file without buffering #2874

Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
005e3e8
Write a constant value to single file without buffering to remove spi…
imaihal Jul 12, 2024
b30d88f
Update comments.
imaihal Jul 12, 2024
f7fda4e
Fix offset.
imaihal Jul 12, 2024
7be482f
Merge branch 'main' into mem_reduction
imaihal Jul 12, 2024
575aa4a
Merge branch 'main' into mem_reduction
imaihal Jul 16, 2024
bd30682
Merge branch 'main' into mem_reduction
imaihal Jul 19, 2024
c3851ba
Remove buffer for pading.
imaihal Jul 19, 2024
0b7ed64
int64_t to uint64_t
imaihal Jul 19, 2024
d7d3055
Merge branch 'main' into mem_reduction
tungld Jul 22, 2024
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
27 changes: 15 additions & 12 deletions src/Conversion/KrnlToLLVM/ConvertKrnlToLLVM.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
#include "llvm/ADT/TypeSwitch.h"
#include "llvm/Support/Debug.h"
#include "llvm/Support/Endian.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Path.h"

#include "onnx/onnx_pb.h"
Expand Down Expand Up @@ -515,13 +516,16 @@ bool extractConstantsToFile(ModuleOp &module, std::string filepath,
return (leftAlign < rightAlign);
});

// Pack all constants into a single buffer in order to save to file.
// Store each constant into single file.
// Constants with the highest alignment will be packed first in the file.
// The file will be mmaped later at runtime and aligned at the page boundary,
// So every constants must be correctly aligned in the packed constant. Pads
// are added if necessary.
std::vector<char> packedConst;
llvm::sys::fs::remove(filepath);
std::ofstream outfile(filepath, std::ios::app | std::ios::binary);
int64_t totalConstSize = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

uint64_t is more suitable.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done. Thanks!

for (int64_t i = globalOfInterest.size() - 1; i >= 0; --i) {
std::vector<char> packedConst;
Copy link
Collaborator

Choose a reason for hiding this comment

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

The name packedConst is obsoleted in the new context since we donnot pack constants anymore. Maybe change it to paddedConstant or it can be totally removed (See the below comment)

KrnlGlobalOp op = globalOfInterest[i];
ArrayRef<char> rawData = getRawData(op);

Expand All @@ -531,27 +535,25 @@ bool extractConstantsToFile(ModuleOp &module, std::string filepath,
alignment = op.getAlignment().value();

// Padding if necessary.
if ((alignment > 0) && (packedConst.size() % alignment != 0)) {
if ((alignment > 0) && (totalConstSize % alignment != 0)) {
uint64_t padSize =
((uint64_t)(packedConst.size() / alignment) + 1) * alignment -
packedConst.size();
((uint64_t)(totalConstSize / alignment) + 1) * alignment -
totalConstSize;
SmallVector<char> pads(padSize, (char)0);
packedConst.insert(packedConst.end(), pads.begin(), pads.end());
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can reduce memory more by not using a padded buffer but two writes to a file: one for padding and one for rawData. For example.

// Padding the current data for correct alignment.
uint64_t padSize = 0;
if ((alignment > 0) && (totalConstSize % alignment != 0)) {
 padSize = ((uint64_t)(totalConstSize / alignment) + 1) * alignment - totalConstSize;
 SmallVector<char> pads(padSize, (char)0);
 outfile.write(pads.data(), pads.size());
}

// Write the constant to a file.
uint64_t constSize = rawData.size();
outfile.write(rawData.data(), constSize);

// Update the total size and op's offset.
totalConstSize += padSize + constSize;
op.setOffsetAttr(b.getI64IntegerAttr(totalConstSize));
op.removeValueAttr();

Copy link
Collaborator

Choose a reason for hiding this comment

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

Smart, please do that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for your comments! I updated.

totalConstSize in op.setOffsetAttr(b.getI64IntegerAttr(totalConstSize)) is offset. So the totalConstSize before adding constSize should be used.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks!

}

op.setOffsetAttr(b.getI64IntegerAttr(packedConst.size()));
op.setOffsetAttr(b.getI64IntegerAttr(totalConstSize + packedConst.size()));
op.removeValueAttr();
packedConst.insert(packedConst.end(), rawData.begin(), rawData.end());
outfile.write(packedConst.data(), packedConst.size());
totalConstSize += packedConst.size();
}

// No constant statisfying thresholds, do not store constants to file.
if (packedConst.empty())
if (totalConstSize == 0)
return false;

// Save to file.
std::ofstream outfile(filepath, std::ofstream::binary);
outfile.write(packedConst.data(), packedConst.size());

// Create a global op to store the filename in the IR.
OpBuilder::InsertionGuard guard(b);
b.setInsertionPointToStart(module.getBody());
Expand All @@ -564,7 +566,8 @@ bool extractConstantsToFile(ModuleOp &module, std::string filepath,
create.llvm.globalOp(llvmI64Ty,
/*isConstant=*/true, LLVM::Linkage::Internal,
EXTERNAL_CONSTANT_PREFIX + "filesize",
b.getI64IntegerAttr(packedConst.size()));
b.getI64IntegerAttr(totalConstSize));

// Create a global to store isLE.
bool isLE = llvm::endianness::native == llvm::endianness::little;
create.llvm.globalOp(llvmI8Ty,
Expand Down
Loading