Skip to content

[clang][CodeGen] Shift relink option implementation away from module cloning #81693

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

Merged
merged 3 commits into from
Feb 14, 2024
Merged

[clang][CodeGen] Shift relink option implementation away from module cloning #81693

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
aff288a
[clang][CodeGen] Shift relink option implementation away from module …
lamb-j Feb 14, 2024
b85e53a
Remove redundant default
lamb-j Feb 14, 2024
80fad72
Restart BuildKite
lamb-j Feb 14, 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
4 changes: 2 additions & 2 deletions clang/include/clang/Basic/FileManager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,15 +283,15 @@ class FileManager : public RefCountedBase<FileManager> {
bool RequiresNullTerminator = true);
llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>
getBufferForFile(StringRef Filename, bool isVolatile = false,
bool RequiresNullTerminator = true) {
bool RequiresNullTerminator = true) const {
return getBufferForFileImpl(Filename, /*FileSize=*/-1, isVolatile,
RequiresNullTerminator);
}

private:
llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>
getBufferForFileImpl(StringRef Filename, int64_t FileSize, bool isVolatile,
bool RequiresNullTerminator);
bool RequiresNullTerminator) const;

public:
/// Get the 'stat' information for the given \p Path.
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Basic/FileManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,7 @@ FileManager::getBufferForFile(FileEntryRef FE, bool isVolatile,
llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>
FileManager::getBufferForFileImpl(StringRef Filename, int64_t FileSize,
bool isVolatile,
bool RequiresNullTerminator) {
bool RequiresNullTerminator) const {
if (FileSystemOpts.WorkingDir.empty())
return FS->getBufferForFile(Filename, FileSize, RequiresNullTerminator,
isVolatile);
Expand Down
16 changes: 10 additions & 6 deletions clang/lib/CodeGen/BackendConsumer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class BackendConsumer : public ASTConsumer {
const CodeGenOptions &CodeGenOpts;
const TargetOptions &TargetOpts;
const LangOptions &LangOpts;
const FileManager &FileMgr;
std::unique_ptr<raw_pwrite_stream> AsmOutStream;
ASTContext *Context;
IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS;
Expand Down Expand Up @@ -74,8 +75,8 @@ class BackendConsumer : public ASTConsumer {
const HeaderSearchOptions &HeaderSearchOpts,
const PreprocessorOptions &PPOpts,
const CodeGenOptions &CodeGenOpts,
const TargetOptions &TargetOpts,
const LangOptions &LangOpts, const std::string &InFile,
const TargetOptions &TargetOpts, const LangOptions &LangOpts,
const FileManager &FileMgr, const std::string &InFile,
SmallVector<LinkModule, 4> LinkModules,
std::unique_ptr<raw_pwrite_stream> OS, llvm::LLVMContext &C,
CoverageSourceInfo *CoverageInfo = nullptr);
Expand All @@ -88,8 +89,8 @@ class BackendConsumer : public ASTConsumer {
const HeaderSearchOptions &HeaderSearchOpts,
const PreprocessorOptions &PPOpts,
const CodeGenOptions &CodeGenOpts,
const TargetOptions &TargetOpts,
const LangOptions &LangOpts, llvm::Module *Module,
const TargetOptions &TargetOpts, const LangOptions &LangOpts,
const FileManager &FileMgr, llvm::Module *Module,
SmallVector<LinkModule, 4> LinkModules, llvm::LLVMContext &C,
CoverageSourceInfo *CoverageInfo = nullptr);

Expand All @@ -111,10 +112,13 @@ class BackendConsumer : public ASTConsumer {
void AssignInheritanceModel(CXXRecordDecl *RD) override;
void HandleVTable(CXXRecordDecl *RD) override;


// Links each entry in LinkModules into our module. Returns true on error.
// Links each entry in LinkModules into our module. Returns true on error.
bool LinkInModules(llvm::Module *M, bool ShouldLinkFiles = true);

// Load a bitcode module from -mlink-builtin-bitcode option using
// methods from a BackendConsumer instead of CompilerInstance
bool ReloadModules(llvm::Module *M);

/// Get the best possible source location to represent a diagnostic that
/// may have associated debug info.
const FullSourceLoc getBestLocationFromDebugLoc(
Expand Down
161 changes: 84 additions & 77 deletions clang/lib/CodeGen/CodeGenAction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,56 +109,52 @@ static void reportOptRecordError(Error E, DiagnosticsEngine &Diags,
});
}

BackendConsumer::BackendConsumer(BackendAction Action, DiagnosticsEngine &Diags,
IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
const HeaderSearchOptions &HeaderSearchOpts,
const PreprocessorOptions &PPOpts,
const CodeGenOptions &CodeGenOpts,
const TargetOptions &TargetOpts,
const LangOptions &LangOpts,
const std::string &InFile,
SmallVector<LinkModule, 4> LinkModules,
std::unique_ptr<raw_pwrite_stream> OS,
LLVMContext &C,
CoverageSourceInfo *CoverageInfo)
: Diags(Diags), Action(Action), HeaderSearchOpts(HeaderSearchOpts),
CodeGenOpts(CodeGenOpts), TargetOpts(TargetOpts), LangOpts(LangOpts),
AsmOutStream(std::move(OS)), Context(nullptr), FS(VFS),
LLVMIRGeneration("irgen", "LLVM IR Generation Time"),
LLVMIRGenerationRefCount(0),
Gen(CreateLLVMCodeGen(Diags, InFile, std::move(VFS), HeaderSearchOpts,
PPOpts, CodeGenOpts, C, CoverageInfo)),
LinkModules(std::move(LinkModules)) {
TimerIsEnabled = CodeGenOpts.TimePasses;
llvm::TimePassesIsEnabled = CodeGenOpts.TimePasses;
llvm::TimePassesPerRun = CodeGenOpts.TimePassesPerRun;
BackendConsumer::BackendConsumer(
BackendAction Action, DiagnosticsEngine &Diags,
IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
const HeaderSearchOptions &HeaderSearchOpts,
const PreprocessorOptions &PPOpts, const CodeGenOptions &CodeGenOpts,
const TargetOptions &TargetOpts, const LangOptions &LangOpts,
const FileManager &FileMgr, const std::string &InFile,
SmallVector<LinkModule, 4> LinkModules,
std::unique_ptr<raw_pwrite_stream> OS, LLVMContext &C,
CoverageSourceInfo *CoverageInfo)
: Diags(Diags), Action(Action), HeaderSearchOpts(HeaderSearchOpts),
CodeGenOpts(CodeGenOpts), TargetOpts(TargetOpts), LangOpts(LangOpts),
FileMgr(FileMgr), AsmOutStream(std::move(OS)), Context(nullptr), FS(VFS),
LLVMIRGeneration("irgen", "LLVM IR Generation Time"),
LLVMIRGenerationRefCount(0),
Gen(CreateLLVMCodeGen(Diags, InFile, std::move(VFS), HeaderSearchOpts,
PPOpts, CodeGenOpts, C, CoverageInfo)),
LinkModules(std::move(LinkModules)) {
TimerIsEnabled = CodeGenOpts.TimePasses;
llvm::TimePassesIsEnabled = CodeGenOpts.TimePasses;
llvm::TimePassesPerRun = CodeGenOpts.TimePassesPerRun;
}

// This constructor is used in installing an empty BackendConsumer
// to use the clang diagnostic handler for IR input files. It avoids
// initializing the OS field.
BackendConsumer::BackendConsumer(BackendAction Action, DiagnosticsEngine &Diags,
IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
const HeaderSearchOptions &HeaderSearchOpts,
const PreprocessorOptions &PPOpts,
const CodeGenOptions &CodeGenOpts,
const TargetOptions &TargetOpts,
const LangOptions &LangOpts,
llvm::Module *Module,
SmallVector<LinkModule, 4> LinkModules,
LLVMContext &C,
CoverageSourceInfo *CoverageInfo)
: Diags(Diags), Action(Action), HeaderSearchOpts(HeaderSearchOpts),
CodeGenOpts(CodeGenOpts), TargetOpts(TargetOpts), LangOpts(LangOpts),
Context(nullptr), FS(VFS),
LLVMIRGeneration("irgen", "LLVM IR Generation Time"),
LLVMIRGenerationRefCount(0),
Gen(CreateLLVMCodeGen(Diags, "", std::move(VFS), HeaderSearchOpts,
PPOpts, CodeGenOpts, C, CoverageInfo)),
LinkModules(std::move(LinkModules)), CurLinkModule(Module) {
TimerIsEnabled = CodeGenOpts.TimePasses;
llvm::TimePassesIsEnabled = CodeGenOpts.TimePasses;
llvm::TimePassesPerRun = CodeGenOpts.TimePassesPerRun;
BackendConsumer::BackendConsumer(
BackendAction Action, DiagnosticsEngine &Diags,
IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
const HeaderSearchOptions &HeaderSearchOpts,
const PreprocessorOptions &PPOpts, const CodeGenOptions &CodeGenOpts,
const TargetOptions &TargetOpts, const LangOptions &LangOpts,
const FileManager &FileMgr, llvm::Module *Module,
SmallVector<LinkModule, 4> LinkModules, LLVMContext &C,
CoverageSourceInfo *CoverageInfo)
: Diags(Diags), Action(Action), HeaderSearchOpts(HeaderSearchOpts),
CodeGenOpts(CodeGenOpts), TargetOpts(TargetOpts), LangOpts(LangOpts),
FileMgr(FileMgr), Context(nullptr), FS(VFS),
LLVMIRGeneration("irgen", "LLVM IR Generation Time"),
LLVMIRGenerationRefCount(0),
Gen(CreateLLVMCodeGen(Diags, "", std::move(VFS), HeaderSearchOpts, PPOpts,
CodeGenOpts, C, CoverageInfo)),
LinkModules(std::move(LinkModules)), CurLinkModule(Module) {
TimerIsEnabled = CodeGenOpts.TimePasses;
llvm::TimePassesIsEnabled = CodeGenOpts.TimePasses;
llvm::TimePassesPerRun = CodeGenOpts.TimePassesPerRun;
}

llvm::Module* BackendConsumer::getModule() const {
Expand Down Expand Up @@ -233,9 +229,37 @@ void BackendConsumer::HandleInterestingDecl(DeclGroupRef D) {
HandleTopLevelDecl(D);
}

bool BackendConsumer::ReloadModules(llvm::Module *M) {
for (const CodeGenOptions::BitcodeFileToLink &F :
CodeGenOpts.LinkBitcodeFiles) {
auto BCBuf = FileMgr.getBufferForFile(F.Filename);
if (!BCBuf) {
Diags.Report(diag::err_cannot_open_file)
<< F.Filename << BCBuf.getError().message();
LinkModules.clear();
return true;
}

LLVMContext &Ctx = getModule()->getContext();
Expected<std::unique_ptr<llvm::Module>> ModuleOrErr =
getOwningLazyBitcodeModule(std::move(*BCBuf), Ctx);

if (!ModuleOrErr) {
handleAllErrors(ModuleOrErr.takeError(), [&](ErrorInfoBase &EIB) {
Diags.Report(diag::err_cannot_open_file) << F.Filename << EIB.message();
});
LinkModules.clear();
return true;
}
LinkModules.push_back({std::move(ModuleOrErr.get()), F.PropagateAttrs,
F.Internalize, F.LinkFlags});
}

return false; // success
}

// Links each entry in LinkModules into our module. Returns true on error.
bool BackendConsumer::LinkInModules(llvm::Module *M, bool ShouldLinkFiles) {

for (auto &LM : LinkModules) {
assert(LM.Module && "LinkModule does not actually have a module");

Expand All @@ -257,37 +281,19 @@ bool BackendConsumer::LinkInModules(llvm::Module *M, bool ShouldLinkFiles) {
CurLinkModule = LM.Module.get();
bool Err;
Copy link
Contributor

Choose a reason for hiding this comment

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

Got error:

clang/lib/CodeGen/CodeGenAction.cpp:282:10: error: variable 'Err' set but not used [-Werror,-Wunused-but-set-variable]
    bool Err;
         ^
1 error generated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oversight on my part. Should be fixed by #81777

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks!


auto DoLink = [&](auto &Mod) {
if (LM.Internalize) {
Err = Linker::linkModules(
*M, std::move(Mod), LM.LinkFlags,
[](llvm::Module &M, const llvm::StringSet<> &GVS) {
internalizeModule(M, [&GVS](const llvm::GlobalValue &GV) {
return !GV.hasName() || (GVS.count(GV.getName()) == 0);
});
if (LM.Internalize) {
Err = Linker::linkModules(
*M, std::move(LM.Module), LM.LinkFlags,
[](llvm::Module &M, const llvm::StringSet<> &GVS) {
internalizeModule(M, [&GVS](const llvm::GlobalValue &GV) {
return !GV.hasName() || (GVS.count(GV.getName()) == 0);
});
} else
Err = Linker::linkModules(*M, std::move(Mod), LM.LinkFlags);
};

// Create a Clone to move to the linker, which preserves the original
// linking modules, allowing them to be linked again in the future
if (ClRelinkBuiltinBitcodePostop) {
// TODO: If CloneModule() is updated to support cloning of unmaterialized
// modules, we can remove this
if (Error E = CurLinkModule->materializeAll())
return false;

std::unique_ptr<llvm::Module> Clone = llvm::CloneModule(*LM.Module);

DoLink(Clone);
}
// Otherwise we can link (and clean up) the original modules
else {
DoLink(LM.Module);
}
});
} else
Err = Linker::linkModules(*M, std::move(LM.Module), LM.LinkFlags);
}

LinkModules.clear();
return false; // success
}

Expand Down Expand Up @@ -1037,8 +1043,9 @@ CodeGenAction::CreateASTConsumer(CompilerInstance &CI, StringRef InFile) {
std::unique_ptr<BackendConsumer> Result(new BackendConsumer(
BA, CI.getDiagnostics(), &CI.getVirtualFileSystem(),
CI.getHeaderSearchOpts(), CI.getPreprocessorOpts(), CI.getCodeGenOpts(),
CI.getTargetOpts(), CI.getLangOpts(), std::string(InFile),
std::move(LinkModules), std::move(OS), *VMContext, CoverageInfo));
CI.getTargetOpts(), CI.getLangOpts(), CI.getFileManager(),
std::string(InFile), std::move(LinkModules), std::move(OS), *VMContext,
CoverageInfo));
BEConsumer = Result.get();

// Enable generating macro debug info only when debug info is not disabled and
Expand Down Expand Up @@ -1199,7 +1206,7 @@ void CodeGenAction::ExecuteAction() {
BackendConsumer Result(BA, CI.getDiagnostics(), &CI.getVirtualFileSystem(),
CI.getHeaderSearchOpts(), CI.getPreprocessorOpts(),
CI.getCodeGenOpts(), CI.getTargetOpts(),
CI.getLangOpts(), TheModule.get(),
CI.getLangOpts(), CI.getFileManager(), TheModule.get(),
std::move(LinkModules), *VMContext, nullptr);

// Link in each pending link module.
Expand Down
14 changes: 12 additions & 2 deletions clang/lib/CodeGen/LinkInModulesPass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,26 @@
#include "LinkInModulesPass.h"
#include "BackendConsumer.h"

#include "clang/Basic/CodeGenOptions.h"
#include "clang/Basic/FileManager.h"
#include "clang/Basic/SourceManager.h"

using namespace llvm;

LinkInModulesPass::LinkInModulesPass(clang::BackendConsumer *BC,
bool ShouldLinkFiles)
: BC(BC), ShouldLinkFiles(ShouldLinkFiles) {}

PreservedAnalyses LinkInModulesPass::run(Module &M, ModuleAnalysisManager &AM) {
if (!BC)
return PreservedAnalyses::all();

// Re-load bitcode modules from files
if (BC->ReloadModules(&M))
report_fatal_error("Bitcode module re-loading failed, aborted!");

if (BC && BC->LinkInModules(&M, ShouldLinkFiles))
report_fatal_error("Bitcode module linking failed, compilation aborted!");
if (BC->LinkInModules(&M, ShouldLinkFiles))
report_fatal_error("Bitcode module re-linking failed, aborted!");

return PreservedAnalyses::all();
}