Skip to content

Commit f458d9b

Browse files
authored
Fix unnecessary one-time recompile of stdlib with -enable-ossa-flag (#39516)
* Fix unnecessary one-time recompile of stdlib with -enable-ossa-flag This includes a bit in the module format to represent if the module was compiled with -enable-ossa-modules flag. When compiling a client module with -enable-ossa-modules flag, all dependent modules are checked for this bit, if not on, recompilation is triggered with -enable-ossa-modules. * Updated tests
1 parent b341624 commit f458d9b

36 files changed

+213
-173
lines changed

include/swift/AST/ASTContext.h

+8-5
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "swift/AST/GenericSignature.h"
2323
#include "swift/AST/Identifier.h"
2424
#include "swift/AST/Import.h"
25+
#include "swift/AST/SILOptions.h"
2526
#include "swift/AST/SearchPathOptions.h"
2627
#include "swift/AST/Type.h"
2728
#include "swift/AST/TypeAlignments.h"
@@ -38,8 +39,8 @@
3839
#include "llvm/ADT/PointerIntPair.h"
3940
#include "llvm/ADT/SetVector.h"
4041
#include "llvm/ADT/SmallPtrSet.h"
41-
#include "llvm/ADT/StringSet.h"
4242
#include "llvm/ADT/StringMap.h"
43+
#include "llvm/ADT/StringSet.h"
4344
#include "llvm/ADT/TinyPtrVector.h"
4445
#include "llvm/Support/Allocator.h"
4546
#include "llvm/Support/DataTypes.h"
@@ -221,11 +222,10 @@ class ASTContext final {
221222
void operator=(const ASTContext&) = delete;
222223

223224
ASTContext(LangOptions &langOpts, TypeCheckerOptions &typeckOpts,
224-
SearchPathOptions &SearchPathOpts,
225+
SILOptions &silOpts, SearchPathOptions &SearchPathOpts,
225226
ClangImporterOptions &ClangImporterOpts,
226227
symbolgraphgen::SymbolGraphOptions &SymbolGraphOpts,
227-
SourceManager &SourceMgr,
228-
DiagnosticEngine &Diags);
228+
SourceManager &SourceMgr, DiagnosticEngine &Diags);
229229

230230
public:
231231
// Members that should only be used by ASTContext.cpp.
@@ -237,7 +237,7 @@ class ASTContext final {
237237
void operator delete(void *Data) throw();
238238

239239
static ASTContext *get(LangOptions &langOpts, TypeCheckerOptions &typeckOpts,
240-
SearchPathOptions &SearchPathOpts,
240+
SILOptions &silOpts, SearchPathOptions &SearchPathOpts,
241241
ClangImporterOptions &ClangImporterOpts,
242242
symbolgraphgen::SymbolGraphOptions &SymbolGraphOpts,
243243
SourceManager &SourceMgr, DiagnosticEngine &Diags);
@@ -255,6 +255,9 @@ class ASTContext final {
255255
/// The type checker options.
256256
const TypeCheckerOptions &TypeCheckerOpts;
257257

258+
/// Options for SIL.
259+
const SILOptions &SILOpts;
260+
258261
/// The search path options used by this AST context.
259262
SearchPathOptions &SearchPathOpts;
260263

include/swift/Serialization/SerializationOptions.h

+1
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@ namespace swift {
141141
bool IsSIB = false;
142142
bool DisableCrossModuleIncrementalInfo = false;
143143
bool StaticLibrary = false;
144+
bool IsOSSA = false;
144145
};
145146

146147
} // end namespace swift

include/swift/Serialization/SerializedModuleLoader.h

+2
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,8 @@ class SerializedModuleLoaderBase : public ModuleLoader {
161161
std::unique_ptr<llvm::MemoryBuffer> moduleSourceInfoInputBuffer,
162162
bool isFramework);
163163

164+
bool isRequiredOSSAModules() const;
165+
164166
/// Check whether the module with a given name can be imported without
165167
/// importing it.
166168
///

include/swift/Serialization/Validation.h

+7-1
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,9 @@ enum class Status {
4242
/// The precise revision version doesn't match.
4343
RevisionIncompatible,
4444

45+
/// The module is required to be in OSSA, but is not.
46+
NotInOSSA,
47+
4548
/// The module file depends on another module that can't be loaded.
4649
MissingDependency,
4750

@@ -186,13 +189,16 @@ class ExtendedValidationInfo {
186189
///
187190
/// \param data A buffer containing the serialized AST. Result information
188191
/// refers directly into this buffer.
192+
/// \param requiresOSSAModules If true, necessitates the module to be
193+
/// compiled with -enable-ossa-modules.
189194
/// \param[out] extendedInfo If present, will be populated with additional
190195
/// compilation options serialized into the AST at build time that may be
191196
/// necessary to load it properly.
192197
/// \param[out] dependencies If present, will be populated with list of
193198
/// input files the module depends on, if present in INPUT_BLOCK.
194199
ValidationInfo validateSerializedAST(
195-
StringRef data, ExtendedValidationInfo *extendedInfo = nullptr,
200+
StringRef data, bool requiresOSSAModules,
201+
ExtendedValidationInfo *extendedInfo = nullptr,
196202
SmallVectorImpl<SerializationOptions::FileDependency> *dependencies =
197203
nullptr);
198204

include/swift/Subsystems.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ namespace swift {
5252
class GenericParamList;
5353
class IRGenOptions;
5454
class LangOptions;
55+
class SILOptions;
5556
class ModuleDecl;
5657
/// A opaque syntax node created by a \c SyntaxParseAction, whose contents
5758
/// must be interpreted by the \c SyntaxParseAction which created it.
@@ -283,7 +284,7 @@ namespace swift {
283284
public:
284285
ParserUnit(SourceManager &SM, SourceFileKind SFKind, unsigned BufferID,
285286
const LangOptions &LangOpts, const TypeCheckerOptions &TyOpts,
286-
StringRef ModuleName,
287+
const SILOptions &SILOpts, StringRef ModuleName,
287288
std::shared_ptr<SyntaxParseActions> spActions = nullptr,
288289
SyntaxParsingCache *SyntaxCache = nullptr);
289290
ParserUnit(SourceManager &SM, SourceFileKind SFKind, unsigned BufferID);

lib/AST/ASTContext.cpp

+18-24
Original file line numberDiff line numberDiff line change
@@ -583,12 +583,11 @@ void ASTContext::operator delete(void *Data) throw() {
583583
}
584584

585585
ASTContext *ASTContext::get(LangOptions &langOpts,
586-
TypeCheckerOptions &typeckOpts,
586+
TypeCheckerOptions &typeckOpts, SILOptions &silOpts,
587587
SearchPathOptions &SearchPathOpts,
588588
ClangImporterOptions &ClangImporterOpts,
589589
symbolgraphgen::SymbolGraphOptions &SymbolGraphOpts,
590-
SourceManager &SourceMgr,
591-
DiagnosticEngine &Diags) {
590+
SourceManager &SourceMgr, DiagnosticEngine &Diags) {
592591
// If more than two data structures are concatentated, then the aggregate
593592
// size math needs to become more complicated due to per-struct alignment
594593
// constraints.
@@ -600,33 +599,28 @@ ASTContext *ASTContext::get(LangOptions &langOpts,
600599
llvm::alignAddr(impl, llvm::Align(alignof(Implementation))));
601600
new (impl) Implementation();
602601
return new (mem)
603-
ASTContext(langOpts, typeckOpts, SearchPathOpts, ClangImporterOpts,
604-
SymbolGraphOpts, SourceMgr, Diags);
602+
ASTContext(langOpts, typeckOpts, silOpts, SearchPathOpts,
603+
ClangImporterOpts, SymbolGraphOpts, SourceMgr, Diags);
605604
}
606605

607606
ASTContext::ASTContext(LangOptions &langOpts, TypeCheckerOptions &typeckOpts,
608-
SearchPathOptions &SearchPathOpts,
607+
SILOptions &silOpts, SearchPathOptions &SearchPathOpts,
609608
ClangImporterOptions &ClangImporterOpts,
610609
symbolgraphgen::SymbolGraphOptions &SymbolGraphOpts,
611610
SourceManager &SourceMgr, DiagnosticEngine &Diags)
612-
: LangOpts(langOpts),
613-
TypeCheckerOpts(typeckOpts),
614-
SearchPathOpts(SearchPathOpts),
615-
ClangImporterOpts(ClangImporterOpts),
616-
SymbolGraphOpts(SymbolGraphOpts),
617-
SourceMgr(SourceMgr), Diags(Diags),
618-
evaluator(Diags, langOpts),
619-
TheBuiltinModule(createBuiltinModule(*this)),
620-
StdlibModuleName(getIdentifier(STDLIB_NAME)),
621-
SwiftShimsModuleName(getIdentifier(SWIFT_SHIMS_NAME)),
622-
TheErrorType(
623-
new (*this, AllocationArena::Permanent)
624-
ErrorType(*this, Type(), RecursiveTypeProperties::HasError)),
625-
TheUnresolvedType(new (*this, AllocationArena::Permanent)
626-
UnresolvedType(*this)),
627-
TheEmptyTupleType(TupleType::get(ArrayRef<TupleTypeElt>(), *this)),
628-
TheAnyType(ProtocolCompositionType::get(*this, ArrayRef<Type>(),
629-
/*HasExplicitAnyObject=*/false)),
611+
: LangOpts(langOpts), TypeCheckerOpts(typeckOpts), SILOpts(silOpts),
612+
SearchPathOpts(SearchPathOpts), ClangImporterOpts(ClangImporterOpts),
613+
SymbolGraphOpts(SymbolGraphOpts), SourceMgr(SourceMgr), Diags(Diags),
614+
evaluator(Diags, langOpts), TheBuiltinModule(createBuiltinModule(*this)),
615+
StdlibModuleName(getIdentifier(STDLIB_NAME)),
616+
SwiftShimsModuleName(getIdentifier(SWIFT_SHIMS_NAME)),
617+
TheErrorType(new (*this, AllocationArena::Permanent) ErrorType(
618+
*this, Type(), RecursiveTypeProperties::HasError)),
619+
TheUnresolvedType(new (*this, AllocationArena::Permanent)
620+
UnresolvedType(*this)),
621+
TheEmptyTupleType(TupleType::get(ArrayRef<TupleTypeElt>(), *this)),
622+
TheAnyType(ProtocolCompositionType::get(*this, ArrayRef<Type>(),
623+
/*HasExplicitAnyObject=*/false)),
630624
#define SINGLETON_TYPE(SHORT_ID, ID) \
631625
The##SHORT_ID##Type(new (*this, AllocationArena::Permanent) \
632626
ID##Type(*this)),

lib/ASTSectionImporter/ASTSectionImporter.cpp

+3-1
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
#include "swift/ASTSectionImporter/ASTSectionImporter.h"
1919
#include "../Serialization/ModuleFormat.h"
20+
#include "swift/AST/ASTContext.h"
2021
#include "swift/Basic/Dwarf.h"
2122
#include "swift/Serialization/SerializedModuleLoader.h"
2223
#include "swift/Serialization/Validation.h"
@@ -34,7 +35,8 @@ bool swift::parseASTSection(MemoryBufferSerializedModuleLoader &Loader,
3435
// An AST section consists of one or more AST modules, optionally with
3536
// headers. Iterate over all AST modules.
3637
while (!buf.empty()) {
37-
auto info = serialization::validateSerializedAST(buf);
38+
auto info = serialization::validateSerializedAST(
39+
buf, Loader.isRequiredOSSAModules());
3840

3941
assert(info.name.size() < (2 << 10) && "name failed sanity check");
4042

lib/DriverTool/modulewrap_main.cpp

+4-4
Original file line numberDiff line numberDiff line change
@@ -175,20 +175,20 @@ int modulewrap_main(ArrayRef<const char *> Args, const char *Argv0,
175175
SearchPathOpts.RuntimeResourcePath = std::string(RuntimeResourcePath.str());
176176

177177
SourceManager SrcMgr;
178+
SILOptions SILOpts;
178179
TypeCheckerOptions TypeCheckOpts;
179180
LangOptions LangOpts;
180181
ClangImporterOptions ClangImporterOpts;
181182
symbolgraphgen::SymbolGraphOptions SymbolGraphOpts;
182183
LangOpts.Target = Invocation.getTargetTriple();
183-
ASTContext &ASTCtx = *ASTContext::get(LangOpts, TypeCheckOpts, SearchPathOpts,
184-
ClangImporterOpts, SymbolGraphOpts, SrcMgr,
185-
Instance.getDiags());
184+
ASTContext &ASTCtx = *ASTContext::get(
185+
LangOpts, TypeCheckOpts, SILOpts, SearchPathOpts, ClangImporterOpts,
186+
SymbolGraphOpts, SrcMgr, Instance.getDiags());
186187
registerParseRequestFunctions(ASTCtx.evaluator);
187188
registerTypeCheckerRequestFunctions(ASTCtx.evaluator);
188189

189190
ASTCtx.addModuleLoader(ClangImporter::create(ASTCtx, ""), true);
190191
ModuleDecl *M = ModuleDecl::create(ASTCtx.getIdentifier("swiftmodule"), ASTCtx);
191-
SILOptions SILOpts;
192192
std::unique_ptr<Lowering::TypeConverter> TC(new Lowering::TypeConverter(*M));
193193
std::unique_ptr<SILModule> SM = SILModule::createEmptyModule(M, *TC, SILOpts);
194194
createSwiftModuleObjectFile(*SM, (*ErrOrBuf)->getBuffer(),

lib/DriverTool/swift_indent_main.cpp

+4-4
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,10 @@ class FormatterDocument {
5959

6060
void updateCode(std::unique_ptr<llvm::MemoryBuffer> Buffer) {
6161
BufferID = SM.addNewSourceBuffer(std::move(Buffer));
62-
Parser.reset(new ParserUnit(SM, SourceFileKind::Main,
63-
BufferID, CompInv.getLangOptions(),
64-
CompInv.getTypeCheckerOptions(),
65-
CompInv.getModuleName()));
62+
Parser.reset(new ParserUnit(
63+
SM, SourceFileKind::Main, BufferID, CompInv.getLangOptions(),
64+
CompInv.getTypeCheckerOptions(), CompInv.getSILOptions(),
65+
CompInv.getModuleName()));
6666
Parser->getDiagnosticEngine().addConsumer(DiagConsumer);
6767
Parser->parse();
6868
}

lib/Frontend/CompilerInvocation.cpp

+4-3
Original file line numberDiff line numberDiff line change
@@ -2151,8 +2151,8 @@ bool CompilerInvocation::parseArgs(
21512151
serialization::Status
21522152
CompilerInvocation::loadFromSerializedAST(StringRef data) {
21532153
serialization::ExtendedValidationInfo extendedInfo;
2154-
serialization::ValidationInfo info =
2155-
serialization::validateSerializedAST(data, &extendedInfo);
2154+
serialization::ValidationInfo info = serialization::validateSerializedAST(
2155+
data, getSILOptions().EnableOSSAModules, &extendedInfo);
21562156

21572157
if (info.status != serialization::Status::Valid)
21582158
return info.status;
@@ -2187,7 +2187,8 @@ CompilerInvocation::setUpInputForSILTool(
21872187
InputFile(inputFilename, bePrimary, fileBufOrErr.get().get(), file_types::TY_SIL));
21882188

21892189
auto result = serialization::validateSerializedAST(
2190-
fileBufOrErr.get()->getBuffer(), &extendedInfo);
2190+
fileBufOrErr.get()->getBuffer(), getSILOptions().EnableOSSAModules,
2191+
&extendedInfo);
21912192
bool hasSerializedAST = result.status == serialization::Status::Valid;
21922193

21932194
if (hasSerializedAST) {

lib/Frontend/Frontend.cpp

+4-3
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,8 @@ SerializationOptions CompilerInvocation::computeSerializationOptions(
181181

182182
serializationOpts.StaticLibrary = opts.Static;
183183

184+
serializationOpts.IsOSSA = getSILOptions().EnableOSSAModules;
185+
184186
return serializationOpts;
185187
}
186188

@@ -215,9 +217,8 @@ bool CompilerInstance::setUpASTContextIfNeeded() {
215217

216218
Context.reset(ASTContext::get(
217219
Invocation.getLangOptions(), Invocation.getTypeCheckerOptions(),
218-
Invocation.getSearchPathOptions(),
219-
Invocation.getClangImporterOptions(),
220-
Invocation.getSymbolGraphOptions(),
220+
Invocation.getSILOptions(), Invocation.getSearchPathOptions(),
221+
Invocation.getClangImporterOptions(), Invocation.getSymbolGraphOptions(),
221222
SourceMgr, Diagnostics));
222223
if (!Invocation.getFrontendOptions().ModuleAliasMap.empty())
223224
Context->setModuleAliases(Invocation.getFrontendOptions().ModuleAliasMap);

lib/Frontend/ModuleInterfaceBuilder.cpp

+2
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,8 @@ bool ModuleInterfaceBuilder::buildSwiftModuleInternal(
265265
}
266266
if (ShouldSerializeDeps)
267267
SerializationOpts.Dependencies = Deps;
268+
SerializationOpts.IsOSSA = SILOpts.EnableOSSAModules;
269+
268270
SILMod->setSerializeSILAction([&]() {
269271
if (isTypeChecking)
270272
return;

lib/Frontend/ModuleInterfaceLoader.cpp

+7-10
Original file line numberDiff line numberDiff line change
@@ -198,8 +198,10 @@ class DiscoveredModule {
198198

199199
namespace path = llvm::sys::path;
200200

201-
static bool serializedASTLooksValid(const llvm::MemoryBuffer &buf) {
202-
auto VI = serialization::validateSerializedAST(buf.getBuffer());
201+
static bool serializedASTLooksValid(const llvm::MemoryBuffer &buf,
202+
bool requiresOSSAModules) {
203+
auto VI = serialization::validateSerializedAST(buf.getBuffer(),
204+
requiresOSSAModules);
203205
return VI.status == serialization::Status::Valid;
204206
}
205207

@@ -497,7 +499,8 @@ class ModuleInterfaceLoaderImpl {
497499

498500
LLVM_DEBUG(llvm::dbgs() << "Validating deps of " << path << "\n");
499501
auto validationInfo = serialization::validateSerializedAST(
500-
buf.getBuffer(), /*ExtendedValidationInfo=*/nullptr, &allDeps);
502+
buf.getBuffer(), requiresOSSAModules,
503+
/*ExtendedValidationInfo=*/nullptr, &allDeps);
501504

502505
if (validationInfo.status != serialization::Status::Valid) {
503506
rebuildInfo.setSerializationStatus(path, validationInfo.status);
@@ -538,7 +541,7 @@ class ModuleInterfaceLoaderImpl {
538541

539542
// First, make sure the underlying module path exists and is valid.
540543
auto modBuf = fs.getBufferForFile(fwd.underlyingModulePath);
541-
if (!modBuf || !serializedASTLooksValid(*modBuf.get()))
544+
if (!modBuf || !serializedASTLooksValid(*modBuf.get(), requiresOSSAModules))
542545
return false;
543546

544547
// Next, check the dependencies in the forwarding file.
@@ -646,12 +649,6 @@ class ModuleInterfaceLoaderImpl {
646649
}
647650

648651
std::pair<std::string, std::string> getCompiledModuleCandidates() {
649-
// If we require ossa modules, then we /always/ rebuild the module interface
650-
// regardless of the module loading mode.
651-
if (requiresOSSAModules) {
652-
return {};
653-
}
654-
655652
std::pair<std::string, std::string> result;
656653
// Keep track of whether we should attempt to load a .swiftmodule adjacent
657654
// to the .swiftinterface.

lib/IDE/CompletionInstance.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -328,12 +328,13 @@ bool CompletionInstance::performCachedOperationIfPossible(
328328

329329
LangOptions langOpts = CI.getASTContext().LangOpts;
330330
TypeCheckerOptions typeckOpts = CI.getASTContext().TypeCheckerOpts;
331+
SILOptions silOpts = CI.getASTContext().SILOpts;
331332
SearchPathOptions searchPathOpts = CI.getASTContext().SearchPathOpts;
332333
DiagnosticEngine tmpDiags(tmpSM);
333334
ClangImporterOptions clangOpts;
334335
symbolgraphgen::SymbolGraphOptions symbolOpts;
335336
std::unique_ptr<ASTContext> tmpCtx(
336-
ASTContext::get(langOpts, typeckOpts, searchPathOpts, clangOpts,
337+
ASTContext::get(langOpts, typeckOpts, silOpts, searchPathOpts, clangOpts,
337338
symbolOpts, tmpSM, tmpDiags));
338339
registerParseRequestFunctions(tmpCtx->evaluator);
339340
registerIDERequestFunctions(tmpCtx->evaluator);

lib/Parse/ParseDecl.cpp

+2-4
Original file line numberDiff line numberDiff line change
@@ -1522,10 +1522,8 @@ void Parser::parseAllAvailabilityMacroArguments() {
15221522
// Create temporary parser.
15231523
int bufferID = SM.addMemBufferCopy(macro,
15241524
"-define-availability argument");
1525-
swift::ParserUnit PU(SM,
1526-
SourceFileKind::Main, bufferID,
1527-
LangOpts,
1528-
TypeCheckerOptions(), "unknown");
1525+
swift::ParserUnit PU(SM, SourceFileKind::Main, bufferID, LangOpts,
1526+
TypeCheckerOptions(), SILOptions(), "unknown");
15291527

15301528
ForwardingDiagnosticConsumer PDC(Context.Diags);
15311529
PU.getDiagnosticEngine().addConsumer(PDC);

0 commit comments

Comments
 (0)