From b5f9e052556a7f59c7a646b48dd2a9c8d18cc9d4 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Mon, 13 Jul 2020 20:42:53 -0700 Subject: [PATCH 1/4] [TBDGen] Make enumeratePublicSymbols more functional Instead of taking an out parameter, have it return the set directly. Also coalesce the two overloads into a single overload that takes a `TBDGenDescriptor`. --- include/swift/TBDGen/TBDGen.h | 6 ++---- lib/FrontendTool/FrontendTool.cpp | 20 +++++++++++--------- lib/FrontendTool/TBD.cpp | 8 ++------ lib/TBDGen/TBDGen.cpp | 16 +++------------- 4 files changed, 18 insertions(+), 32 deletions(-) diff --git a/include/swift/TBDGen/TBDGen.h b/include/swift/TBDGen/TBDGen.h index 1da2386d3132a..67863bb949021 100644 --- a/include/swift/TBDGen/TBDGen.h +++ b/include/swift/TBDGen/TBDGen.h @@ -14,6 +14,7 @@ #include "llvm/ADT/Hashing.h" #include "llvm/ADT/StringSet.h" +#include "swift/AST/TBDGenRequests.h" #include "swift/Basic/Version.h" #include @@ -88,10 +89,7 @@ struct TBDGenOptions { } }; -void enumeratePublicSymbols(FileUnit *module, llvm::StringSet<> &symbols, - const TBDGenOptions &opts); -void enumeratePublicSymbols(ModuleDecl *module, llvm::StringSet<> &symbols, - const TBDGenOptions &opts); +llvm::StringSet<> getPublicSymbols(TBDGenDescriptor desc); void writeTBDFile(ModuleDecl *M, llvm::raw_ostream &os, const TBDGenOptions &opts); diff --git a/lib/FrontendTool/FrontendTool.cpp b/lib/FrontendTool/FrontendTool.cpp index 06347e600b319..92f7ebfce6d84 100644 --- a/lib/FrontendTool/FrontendTool.cpp +++ b/lib/FrontendTool/FrontendTool.cpp @@ -1054,9 +1054,9 @@ static bool writeLdAddCFileIfNeeded(CompilerInstance &Instance) { } auto tbdOpts = Invocation.getTBDGenOptions(); tbdOpts.LinkerDirectivesOnly = true; - llvm::StringSet<> ldSymbols; auto *module = Instance.getMainModule(); - enumeratePublicSymbols(module, ldSymbols, tbdOpts); + auto ldSymbols = + getPublicSymbols(TBDGenDescriptor::forModule(module, tbdOpts)); std::error_code EC; llvm::raw_fd_ostream OS(Path, EC, llvm::sys::fs::F_None); if (EC) { @@ -1663,15 +1663,17 @@ static bool generateCode(CompilerInstance &Instance, StringRef OutputFilename, OutputFilename, Instance.getStatsReporter()); } -static void collectLinkerDirectives(const CompilerInvocation &Invocation, - ModuleOrSourceFile MSF, - llvm::StringSet<> &Symbols) { +static llvm::StringSet<> +collectLinkerDirectives(const CompilerInvocation &Invocation, + ModuleOrSourceFile MSF) { auto tbdOpts = Invocation.getTBDGenOptions(); tbdOpts.LinkerDirectivesOnly = true; - if (MSF.is()) - enumeratePublicSymbols(MSF.get(), Symbols, tbdOpts); - else - enumeratePublicSymbols(MSF.get(), Symbols, tbdOpts); + if (auto *SF = MSF.dyn_cast()) { + return getPublicSymbols(TBDGenDescriptor::forFile(SF, tbdOpts)); + } else { + return getPublicSymbols( + TBDGenDescriptor::forModule(MSF.get(), tbdOpts)); + } } static bool performCompileStepsPostSILGen(CompilerInstance &Instance, diff --git a/lib/FrontendTool/TBD.cpp b/lib/FrontendTool/TBD.cpp index f466c8e5b94d5..5697ddf0a41e9 100644 --- a/lib/FrontendTool/TBD.cpp +++ b/lib/FrontendTool/TBD.cpp @@ -139,9 +139,7 @@ bool swift::validateTBD(ModuleDecl *M, const llvm::Module &IRModule, const TBDGenOptions &opts, bool diagnoseExtraSymbolsInTBD) { - llvm::StringSet<> symbols; - enumeratePublicSymbols(M, symbols, opts); - + auto symbols = getPublicSymbols(TBDGenDescriptor::forModule(M, opts)); return validateSymbolSet(M->getASTContext().Diags, symbols, IRModule, diagnoseExtraSymbolsInTBD); } @@ -150,9 +148,7 @@ bool swift::validateTBD(FileUnit *file, const llvm::Module &IRModule, const TBDGenOptions &opts, bool diagnoseExtraSymbolsInTBD) { - llvm::StringSet<> symbols; - enumeratePublicSymbols(file, symbols, opts); - + auto symbols = getPublicSymbols(TBDGenDescriptor::forFile(file, opts)); return validateSymbolSet(file->getParentModule()->getASTContext().Diags, symbols, IRModule, diagnoseExtraSymbolsInTBD); diff --git a/lib/TBDGen/TBDGen.cpp b/lib/TBDGen/TBDGen.cpp index 6679eeb1b98f3..c545b61e3b68f 100644 --- a/lib/TBDGen/TBDGen.cpp +++ b/lib/TBDGen/TBDGen.cpp @@ -1190,19 +1190,9 @@ GenerateTBDRequest::evaluate(Evaluator &evaluator, return std::make_pair(std::move(file), std::move(symbols)); } -void swift::enumeratePublicSymbols(FileUnit *file, StringSet &symbols, - const TBDGenOptions &opts) { - assert(symbols.empty() && "Additive symbol enumeration not supported"); - auto &evaluator = file->getASTContext().evaluator; - auto desc = TBDGenDescriptor::forFile(file, opts); - symbols = llvm::cantFail(evaluator(GenerateTBDRequest{desc})).second; -} -void swift::enumeratePublicSymbols(ModuleDecl *M, StringSet &symbols, - const TBDGenOptions &opts) { - assert(symbols.empty() && "Additive symbol enumeration not supported"); - auto &evaluator = M->getASTContext().evaluator; - auto desc = TBDGenDescriptor::forModule(M, opts); - symbols = llvm::cantFail(evaluator(GenerateTBDRequest{desc})).second; +StringSet swift::getPublicSymbols(TBDGenDescriptor desc) { + auto &evaluator = desc.getParentModule()->getASTContext().evaluator; + return llvm::cantFail(evaluator(GenerateTBDRequest{desc})).second; } void swift::writeTBDFile(ModuleDecl *M, llvm::raw_ostream &os, const TBDGenOptions &opts) { From 24dc744ca5319e3a9916e859e71ffac30a43d944 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Mon, 13 Jul 2020 20:42:53 -0700 Subject: [PATCH 2/4] Have performParallelIRGeneration take an IRGenDescriptor --- lib/IRGen/IRGen.cpp | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/lib/IRGen/IRGen.cpp b/lib/IRGen/IRGen.cpp index 0d028b8313863..c49d51a21c0cb 100644 --- a/lib/IRGen/IRGen.cpp +++ b/lib/IRGen/IRGen.cpp @@ -1113,11 +1113,11 @@ struct LLVMCodeGenThreads { /// Generates LLVM IR, runs the LLVM passes and produces the output files. /// All this is done in multiple threads. -static void performParallelIRGeneration( - const IRGenOptions &Opts, swift::ModuleDecl *M, std::unique_ptr SILMod, - StringRef ModuleName, - ArrayRef outputFilenames, - llvm::StringSet<> *linkerDirectives) { +static void performParallelIRGeneration(IRGenDescriptor desc) { + const auto &Opts = desc.Opts; + auto outputFilenames = desc.parallelOutputFilenames; + auto SILMod = std::unique_ptr(desc.SILMod); + auto *M = desc.getParentModule(); IRGenerator irgen(Opts, *SILMod); @@ -1158,7 +1158,7 @@ static void performParallelIRGeneration( // Create the IR emitter. IRGenModule *IGM = new IRGenModule(irgen, std::move(targetMachine), nextSF, - ModuleName, *OutputIter++, nextSF->getFilename(), + desc.ModuleName, *OutputIter++, nextSF->getFilename(), nextSF->getPrivateDiscriminator().str()); IGMcreated = true; @@ -1178,7 +1178,7 @@ static void performParallelIRGeneration( } // Emit the module contents. - irgen.emitGlobalTopLevel(linkerDirectives); + irgen.emitGlobalTopLevel(desc.LinkerDirectives); for (auto *File : M->getFiles()) { if (auto *SF = dyn_cast(File)) { @@ -1311,18 +1311,17 @@ GeneratedModule swift::performIRGeneration( const PrimarySpecificPaths &PSPs, ArrayRef parallelOutputFilenames, llvm::GlobalVariable **outModuleHash, llvm::StringSet<> *LinkerDirectives) { + auto desc = IRGenDescriptor::forWholeModule( + Opts, M, std::move(SILMod), ModuleName, PSPs, parallelOutputFilenames, + outModuleHash, LinkerDirectives); + if (Opts.shouldPerformIRGenerationInParallel() && !parallelOutputFilenames.empty()) { - ::performParallelIRGeneration(Opts, M, std::move(SILMod), ModuleName, - parallelOutputFilenames, LinkerDirectives); + ::performParallelIRGeneration(desc); // TODO: Parallel LLVM compilation cannot be used if a (single) module is // needed as return value. return GeneratedModule::null(); } - - auto desc = IRGenDescriptor::forWholeModule( - Opts, M, std::move(SILMod), ModuleName, PSPs, parallelOutputFilenames, - outModuleHash, LinkerDirectives); return llvm::cantFail(M->getASTContext().evaluator(IRGenRequest{desc})); } From db7fea46656d6fd474446e40fa39b73d0cd835ac Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Mon, 13 Jul 2020 20:42:54 -0700 Subject: [PATCH 3/4] Sink linker directive computation into IRGen With an inverted pipeline, IRGen needs to be able to compute the linker directives itself, so sink it down such that it can be computed by the `IRGenDescriptor`. --- include/swift/AST/IRGenRequests.h | 42 ++++++++++++++++++------------- include/swift/Subsystems.h | 13 +++++----- lib/FrontendTool/FrontendTool.cpp | 37 ++++++--------------------- lib/IRGen/CMakeLists.txt | 3 ++- lib/IRGen/GenDecl.cpp | 9 +++---- lib/IRGen/IRGen.cpp | 28 ++++++++++----------- lib/IRGen/IRGenModule.h | 2 +- lib/IRGen/IRGenRequests.cpp | 12 +++++++++ lib/Immediate/Immediate.cpp | 5 ++-- tools/sil-llvm-gen/SILLLVMGen.cpp | 4 ++- 10 files changed, 78 insertions(+), 77 deletions(-) diff --git a/include/swift/AST/IRGenRequests.h b/include/swift/AST/IRGenRequests.h index 3887d8f8dca7c..b6ea1a40c0a1f 100644 --- a/include/swift/AST/IRGenRequests.h +++ b/include/swift/AST/IRGenRequests.h @@ -27,6 +27,7 @@ namespace swift { class SourceFile; class IRGenOptions; class SILModule; +struct TBDGenOptions; namespace irgen { class IRGenModule; @@ -113,15 +114,17 @@ class GeneratedModule final { }; struct IRGenDescriptor { - const IRGenOptions &Opts; llvm::PointerUnion Ctx; + + const IRGenOptions &Opts; + const TBDGenOptions &TBDOpts; + SILModule *SILMod; StringRef ModuleName; const PrimarySpecificPaths &PSPs; StringRef PrivateDiscriminator; ArrayRef parallelOutputFilenames; llvm::GlobalVariable **outModuleHash; - llvm::StringSet<> *LinkerDirectives; friend llvm::hash_code hash_value(const IRGenDescriptor &owner) { return llvm::hash_combine(owner.Ctx); @@ -139,38 +142,38 @@ struct IRGenDescriptor { public: static IRGenDescriptor - forFile(const IRGenOptions &Opts, SourceFile &SF, - std::unique_ptr &&SILMod, StringRef ModuleName, - const PrimarySpecificPaths &PSPs, StringRef PrivateDiscriminator, - llvm::GlobalVariable **outModuleHash, - llvm::StringSet<> *LinkerDirectives) { - return IRGenDescriptor{Opts, - &SF, + forFile(SourceFile &SF, const IRGenOptions &Opts, + const TBDGenOptions &TBDOpts, std::unique_ptr &&SILMod, + StringRef ModuleName, const PrimarySpecificPaths &PSPs, + StringRef PrivateDiscriminator, + llvm::GlobalVariable **outModuleHash) { + return IRGenDescriptor{&SF, + Opts, + TBDOpts, SILMod.release(), ModuleName, PSPs, PrivateDiscriminator, {}, - outModuleHash, - LinkerDirectives}; + outModuleHash}; } static IRGenDescriptor - forWholeModule(const IRGenOptions &Opts, swift::ModuleDecl *M, + forWholeModule(ModuleDecl *M, const IRGenOptions &Opts, + const TBDGenOptions &TBDOpts, std::unique_ptr &&SILMod, StringRef ModuleName, const PrimarySpecificPaths &PSPs, ArrayRef parallelOutputFilenames, - llvm::GlobalVariable **outModuleHash, - llvm::StringSet<> *LinkerDirectives) { - return IRGenDescriptor{Opts, - M, + llvm::GlobalVariable **outModuleHash) { + return IRGenDescriptor{M, + Opts, + TBDOpts, SILMod.release(), ModuleName, PSPs, "", parallelOutputFilenames, - outModuleHash, - LinkerDirectives}; + outModuleHash}; } /// Retrieves the files to perform IR generation for. @@ -179,6 +182,9 @@ struct IRGenDescriptor { /// For a single file, returns its parent module, otherwise returns the module /// itself. ModuleDecl *getParentModule() const; + + /// Compute the linker directives to emit. + llvm::StringSet<> getLinkerDirectives() const; }; /// Report that a request of the given kind is being evaluated, so it diff --git a/include/swift/Subsystems.h b/include/swift/Subsystems.h index 09f746dfe1d7e..2275e8735f938 100644 --- a/include/swift/Subsystems.h +++ b/include/swift/Subsystems.h @@ -62,6 +62,7 @@ namespace swift { class SourceManager; class SyntaxParseActions; class SyntaxParsingCache; + struct TBDGenOptions; class Token; class TopLevelContext; class TypeCheckerOptions; @@ -207,23 +208,23 @@ namespace swift { /// and return the generated LLVM IR module. /// If you set an outModuleHash, then you need to call performLLVM. GeneratedModule - performIRGeneration(const IRGenOptions &Opts, ModuleDecl *M, + performIRGeneration(ModuleDecl *M, const IRGenOptions &Opts, + const TBDGenOptions &TBDOpts, std::unique_ptr SILMod, StringRef ModuleName, const PrimarySpecificPaths &PSPs, ArrayRef parallelOutputFilenames, - llvm::GlobalVariable **outModuleHash = nullptr, - llvm::StringSet<> *LinkerDirectives = nullptr); + llvm::GlobalVariable **outModuleHash = nullptr); /// Turn the given Swift module into either LLVM IR or native code /// and return the generated LLVM IR module. /// If you set an outModuleHash, then you need to call performLLVM. GeneratedModule - performIRGeneration(const IRGenOptions &Opts, SourceFile &SF, + performIRGeneration(SourceFile &SF, const IRGenOptions &Opts, + const TBDGenOptions &TBDOpts, std::unique_ptr SILMod, StringRef ModuleName, const PrimarySpecificPaths &PSPs, StringRef PrivateDiscriminator, - llvm::GlobalVariable **outModuleHash = nullptr, - llvm::StringSet<> *LinkerDirectives = nullptr); + llvm::GlobalVariable **outModuleHash = nullptr); /// Given an already created LLVM module, construct a pass pipeline and run /// the Swift LLVM Pipeline upon it. This does not cause the module to be diff --git a/lib/FrontendTool/FrontendTool.cpp b/lib/FrontendTool/FrontendTool.cpp index 92f7ebfce6d84..f1400e27238d1 100644 --- a/lib/FrontendTool/FrontendTool.cpp +++ b/lib/FrontendTool/FrontendTool.cpp @@ -1501,24 +1501,21 @@ static bool serializeSIB(SILModule *SM, const PrimarySpecificPaths &PSPs, } static GeneratedModule -generateIR(const IRGenOptions &IRGenOpts, +generateIR(const IRGenOptions &IRGenOpts, const TBDGenOptions &TBDOpts, std::unique_ptr SM, const PrimarySpecificPaths &PSPs, StringRef OutputFilename, ModuleOrSourceFile MSF, llvm::GlobalVariable *&HashGlobal, - ArrayRef parallelOutputFilenames, - llvm::StringSet<> &LinkerDirectives) { + ArrayRef parallelOutputFilenames) { if (auto *SF = MSF.dyn_cast()) { - return performIRGeneration(IRGenOpts, *SF, + return performIRGeneration(*SF, IRGenOpts, TBDOpts, std::move(SM), OutputFilename, PSPs, SF->getPrivateDiscriminator().str(), - &HashGlobal, - &LinkerDirectives); + &HashGlobal); } else { - return performIRGeneration(IRGenOpts, MSF.get(), + return performIRGeneration(MSF.get(), IRGenOpts, TBDOpts, std::move(SM), OutputFilename, PSPs, - parallelOutputFilenames, - &HashGlobal, &LinkerDirectives); + parallelOutputFilenames, &HashGlobal); } } @@ -1663,19 +1660,6 @@ static bool generateCode(CompilerInstance &Instance, StringRef OutputFilename, OutputFilename, Instance.getStatsReporter()); } -static llvm::StringSet<> -collectLinkerDirectives(const CompilerInvocation &Invocation, - ModuleOrSourceFile MSF) { - auto tbdOpts = Invocation.getTBDGenOptions(); - tbdOpts.LinkerDirectivesOnly = true; - if (auto *SF = MSF.dyn_cast()) { - return getPublicSymbols(TBDGenDescriptor::forFile(SF, tbdOpts)); - } else { - return getPublicSymbols( - TBDGenDescriptor::forModule(MSF.get(), tbdOpts)); - } -} - static bool performCompileStepsPostSILGen(CompilerInstance &Instance, std::unique_ptr SM, ModuleOrSourceFile MSF, @@ -1783,18 +1767,13 @@ static bool performCompileStepsPostSILGen(CompilerInstance &Instance, return processCommandLineAndRunImmediately( Instance, std::move(SM), MSF, observer, ReturnValue); - llvm::StringSet<> LinkerDirectives; - collectLinkerDirectives(Invocation, MSF, LinkerDirectives); - // Don't proceed to IRGen if collecting linker directives failed. - if (Context.hadError()) - return true; StringRef OutputFilename = PSPs.OutputFilename; std::vector ParallelOutputFilenames = opts.InputsAndOutputs.copyOutputFilenames(); llvm::GlobalVariable *HashGlobal; auto IRModule = generateIR( - IRGenOpts, std::move(SM), PSPs, OutputFilename, MSF, HashGlobal, - ParallelOutputFilenames, LinkerDirectives); + IRGenOpts, Invocation.getTBDGenOptions(), std::move(SM), PSPs, + OutputFilename, MSF, HashGlobal, ParallelOutputFilenames); // Just because we had an AST error it doesn't mean we can't performLLVM. bool HadError = Instance.getASTContext().hadError(); diff --git a/lib/IRGen/CMakeLists.txt b/lib/IRGen/CMakeLists.txt index 1e73fff73d49b..7cfe61b987e86 100644 --- a/lib/IRGen/CMakeLists.txt +++ b/lib/IRGen/CMakeLists.txt @@ -69,4 +69,5 @@ target_link_libraries(swiftIRGen PRIVATE swiftLLVMPasses swiftSIL swiftSILGen - swiftSILOptimizer) + swiftSILOptimizer + swiftTBDGen) diff --git a/lib/IRGen/GenDecl.cpp b/lib/IRGen/GenDecl.cpp index ac7b9b10e2b70..3851f1b3d00ce 100644 --- a/lib/IRGen/GenDecl.cpp +++ b/lib/IRGen/GenDecl.cpp @@ -1030,7 +1030,8 @@ static bool hasCodeCoverageInstrumentation(SILFunction &f, SILModule &m) { return f.getProfiler() && m.getOptions().EmitProfileCoverageMapping; } -void IRGenerator::emitGlobalTopLevel(llvm::StringSet<> *linkerDirectives) { +void IRGenerator::emitGlobalTopLevel( + const llvm::StringSet<> &linkerDirectives) { // Generate order numbers for the functions in the SIL module that // correspond to definitions in the LLVM module. unsigned nextOrderNumber = 0; @@ -1050,10 +1051,8 @@ void IRGenerator::emitGlobalTopLevel(llvm::StringSet<> *linkerDirectives) { CurrentIGMPtr IGM = getGenModule(wt.getProtocol()->getDeclContext()); ensureRelativeSymbolCollocation(wt); } - if (linkerDirectives) { - for (auto &entry: *linkerDirectives) { - createLinkerDirectiveVariable(*PrimaryIGM, entry.getKey()); - } + for (auto &entry: linkerDirectives) { + createLinkerDirectiveVariable(*PrimaryIGM, entry.getKey()); } for (SILGlobalVariable &v : PrimaryIGM->getSILModule().getSILGlobals()) { Decl *decl = v.getDecl(); diff --git a/lib/IRGen/IRGen.cpp b/lib/IRGen/IRGen.cpp index c49d51a21c0cb..7d292c1ef2969 100644 --- a/lib/IRGen/IRGen.cpp +++ b/lib/IRGen/IRGen.cpp @@ -931,7 +931,7 @@ GeneratedModule IRGenRequest::evaluate(Evaluator &evaluator, FrontendStatsTracer tracer(Ctx.Stats, "IRGen"); // Emit the module contents. - irgen.emitGlobalTopLevel(desc.LinkerDirectives); + irgen.emitGlobalTopLevel(desc.getLinkerDirectives()); for (auto *file : filesToEmit) { if (auto *nextSF = dyn_cast(file)) { @@ -1178,7 +1178,7 @@ static void performParallelIRGeneration(IRGenDescriptor desc) { } // Emit the module contents. - irgen.emitGlobalTopLevel(desc.LinkerDirectives); + irgen.emitGlobalTopLevel(desc.getLinkerDirectives()); for (auto *File : M->getFiles()) { if (auto *SF = dyn_cast(File)) { @@ -1306,14 +1306,14 @@ static void performParallelIRGeneration(IRGenDescriptor desc) { } GeneratedModule swift::performIRGeneration( - const IRGenOptions &Opts, swift::ModuleDecl *M, - std::unique_ptr SILMod, StringRef ModuleName, - const PrimarySpecificPaths &PSPs, + swift::ModuleDecl *M, const IRGenOptions &Opts, + const TBDGenOptions &TBDOpts, std::unique_ptr SILMod, + StringRef ModuleName, const PrimarySpecificPaths &PSPs, ArrayRef parallelOutputFilenames, - llvm::GlobalVariable **outModuleHash, llvm::StringSet<> *LinkerDirectives) { + llvm::GlobalVariable **outModuleHash) { auto desc = IRGenDescriptor::forWholeModule( - Opts, M, std::move(SILMod), ModuleName, PSPs, parallelOutputFilenames, - outModuleHash, LinkerDirectives); + M, Opts, TBDOpts, std::move(SILMod), ModuleName, PSPs, + parallelOutputFilenames, outModuleHash); if (Opts.shouldPerformIRGenerationInParallel() && !parallelOutputFilenames.empty()) { @@ -1326,15 +1326,15 @@ GeneratedModule swift::performIRGeneration( } GeneratedModule swift:: -performIRGeneration(const IRGenOptions &Opts, SourceFile &SF, +performIRGeneration(SourceFile &SF, const IRGenOptions &Opts, + const TBDGenOptions &TBDOpts, std::unique_ptr SILMod, StringRef ModuleName, const PrimarySpecificPaths &PSPs, StringRef PrivateDiscriminator, - llvm::GlobalVariable **outModuleHash, - llvm::StringSet<> *LinkerDirectives) { - auto desc = IRGenDescriptor::forFile(Opts, SF, std::move(SILMod), ModuleName, - PSPs, PrivateDiscriminator, - outModuleHash, LinkerDirectives); + llvm::GlobalVariable **outModuleHash) { + auto desc = IRGenDescriptor::forFile(SF, Opts, TBDOpts, std::move(SILMod), + ModuleName, PSPs, PrivateDiscriminator, + outModuleHash); return llvm::cantFail(SF.getASTContext().evaluator(IRGenRequest{desc})); } diff --git a/lib/IRGen/IRGenModule.h b/lib/IRGen/IRGenModule.h index d8634e056470d..84aa7f258a881 100644 --- a/lib/IRGen/IRGenModule.h +++ b/lib/IRGen/IRGenModule.h @@ -375,7 +375,7 @@ class IRGenerator { /// Emit functions, variables and tables which are needed anyway, e.g. because /// they are externally visible. - void emitGlobalTopLevel(llvm::StringSet<> *LinkerDirectives); + void emitGlobalTopLevel(const llvm::StringSet<> &LinkerDirectives); /// Emit references to each of the protocol descriptors defined in this /// IR module. diff --git a/lib/IRGen/IRGenRequests.cpp b/lib/IRGen/IRGenRequests.cpp index 0bb8963b44984..fa60d83945ec0 100644 --- a/lib/IRGen/IRGenRequests.cpp +++ b/lib/IRGen/IRGenRequests.cpp @@ -17,6 +17,7 @@ #include "swift/AST/SourceFile.h" #include "swift/SIL/SILModule.h" #include "swift/Subsystems.h" +#include "swift/TBDGen/TBDGen.h" #include "llvm/IR/Module.h" #include "llvm/ExecutionEngine/Orc/ThreadSafeModule.h" @@ -75,6 +76,17 @@ ModuleDecl *IRGenDescriptor::getParentModule() const { return Ctx.get(); } +llvm::StringSet<> IRGenDescriptor::getLinkerDirectives() const { + auto opts = TBDOpts; + opts.LinkerDirectivesOnly = true; + if (auto *SF = Ctx.dyn_cast()) { + return getPublicSymbols(TBDGenDescriptor::forFile(SF, opts)); + } else { + auto *M = Ctx.get(); + return getPublicSymbols(TBDGenDescriptor::forModule(M, opts)); + } +} + evaluator::DependencySource IRGenRequest::readDependencySource( const evaluator::DependencyRecorder &e) const { auto &desc = std::get<0>(getStorage()); diff --git a/lib/Immediate/Immediate.cpp b/lib/Immediate/Immediate.cpp index 2728b1898e8ac..a2cead40c0707 100644 --- a/lib/Immediate/Immediate.cpp +++ b/lib/Immediate/Immediate.cpp @@ -201,9 +201,10 @@ int swift::RunImmediately(CompilerInstance &CI, // IRGen the main module. auto *swiftModule = CI.getMainModule(); const auto PSPs = CI.getPrimarySpecificPathsForAtMostOnePrimary(); + const auto &TBDOpts = CI.getInvocation().getTBDGenOptions(); auto GenModule = performIRGeneration( - IRGenOpts, swiftModule, std::move(SM), swiftModule->getName().str(), - PSPs, ArrayRef()); + swiftModule, IRGenOpts, TBDOpts, std::move(SM), + swiftModule->getName().str(), PSPs, ArrayRef()); if (Context.hadError()) return -1; diff --git a/tools/sil-llvm-gen/SILLLVMGen.cpp b/tools/sil-llvm-gen/SILLLVMGen.cpp index 80ec715c9f42b..04b505c904b62 100644 --- a/tools/sil-llvm-gen/SILLLVMGen.cpp +++ b/tools/sil-llvm-gen/SILLLVMGen.cpp @@ -196,7 +196,9 @@ int main(int argc, char **argv) { } const PrimarySpecificPaths PSPs(OutputFilename, InputFilename); - auto Mod = performIRGeneration(Opts, CI.getMainModule(), std::move(SILMod), + auto Mod = performIRGeneration(CI.getMainModule(), Opts, + CI.getInvocation().getTBDGenOptions(), + std::move(SILMod), CI.getMainModule()->getName().str(), PSPs, ArrayRef()); return CI.getASTContext().hadError(); From c354b0faa19e4862ac357013e01b10be64990e19 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Mon, 13 Jul 2020 20:42:54 -0700 Subject: [PATCH 4/4] [TBDGen] Return a vector of symbols instead of set A couple of clients are iterating over the result, so switch to a vector to ensure we don't accidentally introduce any non-determinism. --- include/swift/AST/IRGenRequests.h | 2 +- include/swift/AST/TBDGenRequests.h | 2 +- include/swift/TBDGen/TBDGen.h | 2 +- lib/FrontendTool/FrontendTool.cpp | 2 +- lib/FrontendTool/TBD.cpp | 26 ++++++++++++++------------ lib/IRGen/GenDecl.cpp | 6 +++--- lib/IRGen/IRGenModule.h | 2 +- lib/IRGen/IRGenRequests.cpp | 2 +- lib/TBDGen/TBDGen.cpp | 15 ++++++--------- lib/TBDGen/TBDGenVisitor.h | 15 ++++++++++----- test/TBD/linker-directives.swift | 2 +- 11 files changed, 40 insertions(+), 36 deletions(-) diff --git a/include/swift/AST/IRGenRequests.h b/include/swift/AST/IRGenRequests.h index b6ea1a40c0a1f..67d3117e36215 100644 --- a/include/swift/AST/IRGenRequests.h +++ b/include/swift/AST/IRGenRequests.h @@ -184,7 +184,7 @@ struct IRGenDescriptor { ModuleDecl *getParentModule() const; /// Compute the linker directives to emit. - llvm::StringSet<> getLinkerDirectives() const; + std::vector getLinkerDirectives() const; }; /// Report that a request of the given kind is being evaluated, so it diff --git a/include/swift/AST/TBDGenRequests.h b/include/swift/AST/TBDGenRequests.h index d6e781c20b639..e7c898ece5122 100644 --- a/include/swift/AST/TBDGenRequests.h +++ b/include/swift/AST/TBDGenRequests.h @@ -78,7 +78,7 @@ void simple_display(llvm::raw_ostream &out, const TBDGenDescriptor &desc); SourceLoc extractNearestSourceLoc(const TBDGenDescriptor &desc); using TBDFileAndSymbols = - std::pair>; + std::pair>; /// Computes the TBD file and public symbols for a given module or file. class GenerateTBDRequest diff --git a/include/swift/TBDGen/TBDGen.h b/include/swift/TBDGen/TBDGen.h index 67863bb949021..f4d67280b92ec 100644 --- a/include/swift/TBDGen/TBDGen.h +++ b/include/swift/TBDGen/TBDGen.h @@ -89,7 +89,7 @@ struct TBDGenOptions { } }; -llvm::StringSet<> getPublicSymbols(TBDGenDescriptor desc); +std::vector getPublicSymbols(TBDGenDescriptor desc); void writeTBDFile(ModuleDecl *M, llvm::raw_ostream &os, const TBDGenOptions &opts); diff --git a/lib/FrontendTool/FrontendTool.cpp b/lib/FrontendTool/FrontendTool.cpp index f1400e27238d1..077846c61eacc 100644 --- a/lib/FrontendTool/FrontendTool.cpp +++ b/lib/FrontendTool/FrontendTool.cpp @@ -1074,7 +1074,7 @@ static bool writeLdAddCFileIfNeeded(CompilerInstance &Instance) { llvm::raw_svector_ostream NameOS(NameBuffer); NameOS << "ldAdd_" << Idx; OS << "extern const char " << NameOS.str() << " __asm(\"" << - changeToLdAdd(S.getKey()) << "\");\n"; + changeToLdAdd(S) << "\");\n"; OS << "const char " << NameOS.str() << " = 0;\n"; ++ Idx; } diff --git a/lib/FrontendTool/TBD.cpp b/lib/FrontendTool/TBD.cpp index 5697ddf0a41e9..77a27a2152e87 100644 --- a/lib/FrontendTool/TBD.cpp +++ b/lib/FrontendTool/TBD.cpp @@ -73,10 +73,13 @@ bool swift::inputFileKindCanHaveTBDValidated(InputFileKind kind) { llvm_unreachable("unhandled kind"); } -static bool validateSymbolSet(DiagnosticEngine &diags, - llvm::StringSet<> symbols, - const llvm::Module &IRModule, - bool diagnoseExtraSymbolsInTBD) { +static bool validateSymbols(DiagnosticEngine &diags, + const std::vector &symbols, + const llvm::Module &IRModule, + bool diagnoseExtraSymbolsInTBD) { + llvm::StringSet<> symbolSet; + symbolSet.insert(symbols.begin(), symbols.end()); + auto error = false; // Diff the two sets of symbols, flagging anything outside their intersection. @@ -101,13 +104,13 @@ static bool validateSymbolSet(DiagnosticEngine &diags, GV->hasExternalLinkage() && !GV->hasHiddenVisibility(); if (!GV->isDeclaration() && externallyVisible) { // Is it listed? - if (!symbols.erase(name)) + if (!symbolSet.erase(name)) // Note: Add the unmangled name to the irNotTBD list, which is owned // by the IRModule, instead of the mangled name. irNotTBD.push_back(unmangledName); } } else { - assert(symbols.find(name) == symbols.end() && + assert(symbolSet.find(name) == symbolSet.end() && "non-global value in value symbol table"); } } @@ -121,7 +124,7 @@ static bool validateSymbolSet(DiagnosticEngine &diags, if (diagnoseExtraSymbolsInTBD) { // Look for any extra symbols. - for (auto &name : sortSymbols(symbols)) { + for (auto &name : sortSymbols(symbolSet)) { diags.diagnose(SourceLoc(), diag::symbol_in_tbd_not_in_ir, name, Demangle::demangleSymbolAsString(name)); error = true; @@ -140,8 +143,8 @@ bool swift::validateTBD(ModuleDecl *M, const TBDGenOptions &opts, bool diagnoseExtraSymbolsInTBD) { auto symbols = getPublicSymbols(TBDGenDescriptor::forModule(M, opts)); - return validateSymbolSet(M->getASTContext().Diags, symbols, IRModule, - diagnoseExtraSymbolsInTBD); + return validateSymbols(M->getASTContext().Diags, symbols, IRModule, + diagnoseExtraSymbolsInTBD); } bool swift::validateTBD(FileUnit *file, @@ -149,7 +152,6 @@ bool swift::validateTBD(FileUnit *file, const TBDGenOptions &opts, bool diagnoseExtraSymbolsInTBD) { auto symbols = getPublicSymbols(TBDGenDescriptor::forFile(file, opts)); - return validateSymbolSet(file->getParentModule()->getASTContext().Diags, - symbols, IRModule, - diagnoseExtraSymbolsInTBD); + return validateSymbols(file->getParentModule()->getASTContext().Diags, + symbols, IRModule, diagnoseExtraSymbolsInTBD); } diff --git a/lib/IRGen/GenDecl.cpp b/lib/IRGen/GenDecl.cpp index 3851f1b3d00ce..4182ea70a8367 100644 --- a/lib/IRGen/GenDecl.cpp +++ b/lib/IRGen/GenDecl.cpp @@ -1031,7 +1031,7 @@ static bool hasCodeCoverageInstrumentation(SILFunction &f, SILModule &m) { } void IRGenerator::emitGlobalTopLevel( - const llvm::StringSet<> &linkerDirectives) { + const std::vector &linkerDirectives) { // Generate order numbers for the functions in the SIL module that // correspond to definitions in the LLVM module. unsigned nextOrderNumber = 0; @@ -1051,8 +1051,8 @@ void IRGenerator::emitGlobalTopLevel( CurrentIGMPtr IGM = getGenModule(wt.getProtocol()->getDeclContext()); ensureRelativeSymbolCollocation(wt); } - for (auto &entry: linkerDirectives) { - createLinkerDirectiveVariable(*PrimaryIGM, entry.getKey()); + for (auto &directive: linkerDirectives) { + createLinkerDirectiveVariable(*PrimaryIGM, directive); } for (SILGlobalVariable &v : PrimaryIGM->getSILModule().getSILGlobals()) { Decl *decl = v.getDecl(); diff --git a/lib/IRGen/IRGenModule.h b/lib/IRGen/IRGenModule.h index 84aa7f258a881..1bad7269cba6a 100644 --- a/lib/IRGen/IRGenModule.h +++ b/lib/IRGen/IRGenModule.h @@ -375,7 +375,7 @@ class IRGenerator { /// Emit functions, variables and tables which are needed anyway, e.g. because /// they are externally visible. - void emitGlobalTopLevel(const llvm::StringSet<> &LinkerDirectives); + void emitGlobalTopLevel(const std::vector &LinkerDirectives); /// Emit references to each of the protocol descriptors defined in this /// IR module. diff --git a/lib/IRGen/IRGenRequests.cpp b/lib/IRGen/IRGenRequests.cpp index fa60d83945ec0..5e9f9a7c81851 100644 --- a/lib/IRGen/IRGenRequests.cpp +++ b/lib/IRGen/IRGenRequests.cpp @@ -76,7 +76,7 @@ ModuleDecl *IRGenDescriptor::getParentModule() const { return Ctx.get(); } -llvm::StringSet<> IRGenDescriptor::getLinkerDirectives() const { +std::vector IRGenDescriptor::getLinkerDirectives() const { auto opts = TBDOpts; opts.LinkerDirectivesOnly = true; if (auto *SF = Ctx.dyn_cast()) { diff --git a/lib/TBDGen/TBDGen.cpp b/lib/TBDGen/TBDGen.cpp index c545b61e3b68f..087418469d95f 100644 --- a/lib/TBDGen/TBDGen.cpp +++ b/lib/TBDGen/TBDGen.cpp @@ -67,11 +67,10 @@ void TBDGenVisitor::addSymbolInternal(StringRef name, if (!isLinkerDirective && Opts.LinkerDirectivesOnly) return; Symbols.addSymbol(kind, name, Targets); - if (StringSymbols && kind == SymbolKind::GlobalSymbol) { - auto isNewValue = StringSymbols->insert(name).second; - (void)isNewValue; + if (kind == SymbolKind::GlobalSymbol) { + StringSymbols.push_back(name); #ifndef NDEBUG - if (!isNewValue) { + if (!DuplicateSymbolChecker.insert(name).second) { llvm::dbgs() << "TBDGen duplicate symbol: " << name << '\n'; assert(false && "TBDGen symbol appears twice"); } @@ -1135,10 +1134,8 @@ GenerateTBDRequest::evaluate(Evaluator &evaluator, llvm::MachO::Target targetVar(*ctx.LangOpts.TargetVariant); file.addTarget(targetVar); } - StringSet symbols; auto *clang = static_cast(ctx.getClangModuleLoader()); - TBDGenVisitor visitor(file, {target}, &symbols, - clang->getTargetInfo().getDataLayout(), + TBDGenVisitor visitor(file, {target}, clang->getTargetInfo().getDataLayout(), linkInfo, M, opts); auto visitFile = [&](FileUnit *file) { @@ -1187,10 +1184,10 @@ GenerateTBDRequest::evaluate(Evaluator &evaluator, }); } - return std::make_pair(std::move(file), std::move(symbols)); + return std::make_pair(std::move(file), std::move(visitor.StringSymbols)); } -StringSet swift::getPublicSymbols(TBDGenDescriptor desc) { +std::vector swift::getPublicSymbols(TBDGenDescriptor desc) { auto &evaluator = desc.getParentModule()->getASTContext().evaluator; return llvm::cantFail(evaluator(GenerateTBDRequest{desc})).second; } diff --git a/lib/TBDGen/TBDGenVisitor.h b/lib/TBDGen/TBDGenVisitor.h index 002f0f8f02414..eb6227b2a417c 100644 --- a/lib/TBDGen/TBDGenVisitor.h +++ b/lib/TBDGen/TBDGenVisitor.h @@ -63,9 +63,14 @@ class TBDGenVisitor : public ASTVisitor { public: llvm::MachO::InterfaceFile &Symbols; llvm::MachO::TargetList Targets; - StringSet *StringSymbols; + std::vector StringSymbols; const llvm::DataLayout &DataLayout; +#ifndef NDEBUG + /// Tracks the symbols emitted to ensure we don't emit any duplicates. + llvm::StringSet<> DuplicateSymbolChecker; +#endif + const UniversalLinkageInfo &UniversalLinkInfo; ModuleDecl *SwiftModule; const TBDGenOptions &Opts; @@ -136,13 +141,13 @@ class TBDGenVisitor : public ASTVisitor { public: TBDGenVisitor(llvm::MachO::InterfaceFile &symbols, - llvm::MachO::TargetList targets, StringSet *stringSymbols, + llvm::MachO::TargetList targets, const llvm::DataLayout &dataLayout, const UniversalLinkageInfo &universalLinkInfo, ModuleDecl *swiftModule, const TBDGenOptions &opts) - : Symbols(symbols), Targets(targets), StringSymbols(stringSymbols), - DataLayout(dataLayout), UniversalLinkInfo(universalLinkInfo), - SwiftModule(swiftModule), Opts(opts), + : Symbols(symbols), Targets(targets), DataLayout(dataLayout), + UniversalLinkInfo(universalLinkInfo), SwiftModule(swiftModule), + Opts(opts), previousInstallNameMap(parsePreviousModuleInstallNameMap()) {} ~TBDGenVisitor() { assert(DeclStack.empty()); } void addMainIfNecessary(FileUnit *file) { diff --git a/test/TBD/linker-directives.swift b/test/TBD/linker-directives.swift index 3e5b9e11c88e0..6a6c5c4eaa23b 100644 --- a/test/TBD/linker-directives.swift +++ b/test/TBD/linker-directives.swift @@ -21,5 +21,5 @@ public func toast() {} // RUN: %target-swift-frontend -typecheck %s -emit-tbd -emit-tbd-path %t/linker_directives.tbd -emit-ldadd-cfile-path %t/ldAdd.c -module-name AppKit // RUN: %FileCheck %s --check-prefix CHECK-C-SYMBOL < %t/ldAdd.c +// CHECK-C-SYMBOL: $ld$add$os10.8$_$s10ToasterKit5toastyyF // CHECK-C-SYMBOL: $ld$add$os10.14$_$s10ToasterKit5toastyyF -// CHECK-C-SYMBOL: $ld$add$os10.8$_$s10ToasterKit5toastyyF \ No newline at end of file