From 5792a72b95d9287c18f4c018be4852184796f2ad Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Tue, 23 Jun 2020 16:05:48 -0700 Subject: [PATCH 1/5] Always link SIL for partial modules in SILGen Previously we would only link `.sib` partial modules in SILGen, with other kinds of serialized ASTs being linked just before the SILOptimizer if `-sil-merge-partial-modules` was specified. However linking them always seems to be the desired behaviour, so adjust SILGen to link SIL for all serialized AST inputs. --- include/swift/SIL/SILModule.h | 4 ---- lib/Frontend/Frontend.cpp | 17 ++++++----------- lib/SIL/IR/SILModule.cpp | 5 ----- lib/SILGen/SILGen.cpp | 12 ++++++------ .../sil-func-extractor/SILFunctionExtractor.cpp | 12 +++--------- tools/sil-opt/SILOpt.cpp | 11 +++-------- 6 files changed, 18 insertions(+), 43 deletions(-) diff --git a/include/swift/SIL/SILModule.h b/include/swift/SIL/SILModule.h index a5c38959ad889..988fe9fe61ece 100644 --- a/include/swift/SIL/SILModule.h +++ b/include/swift/SIL/SILModule.h @@ -561,10 +561,6 @@ class SILModule { /// i.e. it can be linked by linkFunction. bool hasFunction(StringRef Name); - /// Link all definitions in all segments that are logically part of - /// the same AST module. - void linkAllFromCurrentModule(); - /// Look up the SILWitnessTable representing the lowering of a protocol /// conformance, and collect the substitutions to apply to the referenced /// witnesses, if any. diff --git a/lib/Frontend/Frontend.cpp b/lib/Frontend/Frontend.cpp index 08cea263b3544..fdbf61bc7fe99 100644 --- a/lib/Frontend/Frontend.cpp +++ b/lib/Frontend/Frontend.cpp @@ -982,22 +982,17 @@ void CompilerInstance::freeASTContext() { /// Perform "stable" optimizations that are invariant across compiler versions. static bool performMandatorySILPasses(CompilerInvocation &Invocation, SILModule *SM) { + // Don't run diagnostic passes at all when merging modules. if (Invocation.getFrontendOptions().RequestedAction == FrontendOptions::ActionType::MergeModules) { - // Don't run diagnostic passes at all. - } else if (!Invocation.getDiagnosticOptions().SkipDiagnosticPasses) { - if (runSILDiagnosticPasses(*SM)) - return true; - } else { + return false; + } + if (Invocation.getDiagnosticOptions().SkipDiagnosticPasses) { // Even if we are not supposed to run the diagnostic passes, we still need // to run the ownership evaluator. - if (runSILOwnershipEliminatorPass(*SM)) - return true; + return runSILOwnershipEliminatorPass(*SM); } - - if (Invocation.getSILOptions().MergePartialModules) - SM->linkAllFromCurrentModule(); - return false; + return runSILDiagnosticPasses(*SM); } /// Perform SIL optimization passes if optimizations haven't been disabled. diff --git a/lib/SIL/IR/SILModule.cpp b/lib/SIL/IR/SILModule.cpp index 68d8f2a54dd40..e5c0dd5a3f842 100644 --- a/lib/SIL/IR/SILModule.cpp +++ b/lib/SIL/IR/SILModule.cpp @@ -427,11 +427,6 @@ bool SILModule::hasFunction(StringRef Name) { return getSILLoader()->hasSILFunction(Name); } -void SILModule::linkAllFromCurrentModule() { - getSILLoader()->getAllForModule(getSwiftModule()->getName(), - /*PrimaryFile=*/nullptr); -} - void SILModule::invalidateSILLoaderCaches() { getSILLoader()->invalidateCaches(); } diff --git a/lib/SILGen/SILGen.cpp b/lib/SILGen/SILGen.cpp index 01a7a175dd4bb..7f2b690297c55 100644 --- a/lib/SILGen/SILGen.cpp +++ b/lib/SILGen/SILGen.cpp @@ -1928,12 +1928,12 @@ ASTLoweringRequest::evaluate(Evaluator &evaluator, } // Also make sure to process any intermediate files that may contain SIL. - bool hasSIB = llvm::any_of(desc.getFiles(), [](const FileUnit *File) -> bool { - auto *SASTF = dyn_cast(File); - return SASTF && SASTF->isSIB(); - }); - if (hasSIB) { - auto primary = desc.context.dyn_cast(); + bool shouldDeserialize = + llvm::any_of(desc.getFiles(), [](const FileUnit *File) -> bool { + return isa(File); + }); + if (shouldDeserialize) { + auto *primary = desc.context.dyn_cast(); silMod->getSILLoader()->getAllForModule(silMod->getSwiftModule()->getName(), primary); } diff --git a/tools/sil-func-extractor/SILFunctionExtractor.cpp b/tools/sil-func-extractor/SILFunctionExtractor.cpp index 5630dbf244a01..9e4b49d5deef0 100644 --- a/tools/sil-func-extractor/SILFunctionExtractor.cpp +++ b/tools/sil-func-extractor/SILFunctionExtractor.cpp @@ -275,16 +275,10 @@ int main(int argc, char **argv) { auto SILMod = performASTLowering(CI.getMainModule(), CI.getSILTypes(), CI.getSILOptions()); - // Load the SIL if we have a non-SIB serialized module. SILGen handles SIB for - // us. + // Load in all the SIL if we're allowed to. if (Invocation.hasSerializedAST() && !extendedInfo.isSIB()) { - auto SL = SerializedSILLoader::create( - CI.getASTContext(), SILMod.get(), nullptr); - - if (DisableSILLinking) - SL->getAllForModule(CI.getMainModule()->getName(), nullptr); - else - SL->getAll(); + if (!DisableSILLinking) + SILMod->getSILLoader()->getAll(); } if (CommandLineFunctionNames.empty() && FunctionNameFile.empty()) diff --git a/tools/sil-opt/SILOpt.cpp b/tools/sil-opt/SILOpt.cpp index 26f1f1c8f505d..1ef2ffcb2dd61 100644 --- a/tools/sil-opt/SILOpt.cpp +++ b/tools/sil-opt/SILOpt.cpp @@ -464,15 +464,10 @@ int main(int argc, char **argv) { } SILMod->setSerializeSILAction([]{}); - // Load the SIL if we have a non-SIB serialized module. SILGen handles SIB for - // us. + // Load in all the SIL if we're allowed to. if (Invocation.hasSerializedAST() && !extendedInfo.isSIB()) { - auto SL = SerializedSILLoader::create( - CI.getASTContext(), SILMod.get(), nullptr); - if (DisableSILLinking) - SL->getAllForModule(CI.getMainModule()->getName(), nullptr); - else - SL->getAll(); + if (!DisableSILLinking) + SILMod->getSILLoader()->getAll(); } if (!RemarksFilename.empty()) { From b3dcef9796ceedba0dc59bfd3f72a7c9d15b5204 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Tue, 23 Jun 2020 16:05:49 -0700 Subject: [PATCH 2/5] Avoid unnecessary linking in SIL tools Previously we were linking in all SIL entities if the input was a serialized non-SIB AST, and `-disable-sil-linking` wasn't specified. However none of the tests appear to want this behaviour. Stop calling `SerializedSILLoader::getAll`, and remove the `-disable-sil-linking` option, as this is now the default behaviour. --- include/swift/Serialization/SerializedSILLoader.h | 4 ---- lib/Serialization/SerializedSILLoader.cpp | 5 ----- test/AutoDiff/Serialization/derivative_attr.swift | 2 +- test/AutoDiff/Serialization/differentiable_attr.swift | 2 +- .../Serialization/differentiable_function.swift | 2 +- test/AutoDiff/Serialization/transpose_attr.swift | 2 +- test/Frontend/sil-merge-partial-modules.swift | 2 +- .../ModuleCache/SerializedSIL.swiftinterface | 2 +- test/SIL/Parser/stage.swift | 2 +- test/SIL/Serialization/dynamically_replaceable.sil | 2 +- test/SIL/Serialization/globals.sil | 2 +- test/SILOptimizer/switch_enum_resilient.swift | 8 ++++---- test/Serialization/alignment.swift | 2 +- .../default-witness-table-deserialization.swift | 2 +- test/Serialization/global_init.swift | 2 +- test/Serialization/serialize_attr.swift | 2 +- tools/sil-func-extractor/SILFunctionExtractor.cpp | 11 ----------- tools/sil-llvm-gen/SILLLVMGen.cpp | 8 -------- tools/sil-nm/SILNM.cpp | 9 --------- tools/sil-opt/SILOpt.cpp | 10 ---------- 20 files changed, 17 insertions(+), 64 deletions(-) diff --git a/include/swift/Serialization/SerializedSILLoader.h b/include/swift/Serialization/SerializedSILLoader.h index 86e97a220427d..1707f546d8cae 100644 --- a/include/swift/Serialization/SerializedSILLoader.h +++ b/include/swift/Serialization/SerializedSILLoader.h @@ -74,10 +74,6 @@ class SerializedSILLoader { bool invalidateFunction(SILFunction *F); - /// Deserialize all SILFunctions, VTables, and WitnessTables in all - /// SILModules. - void getAll(); - /// Deserialize all SILFunctions, VTables, and WitnessTables for /// a given Module. /// diff --git a/lib/Serialization/SerializedSILLoader.cpp b/lib/Serialization/SerializedSILLoader.cpp index 2ddeedcca5304..b0e495690adc2 100644 --- a/lib/Serialization/SerializedSILLoader.cpp +++ b/lib/Serialization/SerializedSILLoader.cpp @@ -156,11 +156,6 @@ bool SerializedSILLoader::invalidateFunction(SILFunction *F) { return false; } -void SerializedSILLoader::getAll() { - for (auto &Des : LoadedSILSections) - Des->getAll(); -} - // FIXME: Not the best interface. We know exactly which FileUnits may have SIL // those in the main module. void SerializedSILLoader::getAllForModule(Identifier Mod, diff --git a/test/AutoDiff/Serialization/derivative_attr.swift b/test/AutoDiff/Serialization/derivative_attr.swift index 7b28ddff7d9e9..0a2094f4bb360 100644 --- a/test/AutoDiff/Serialization/derivative_attr.swift +++ b/test/AutoDiff/Serialization/derivative_attr.swift @@ -1,7 +1,7 @@ // RUN: %empty-directory(%t) // RUN: %target-swift-frontend %s -emit-module -parse-as-library -o %t // RUN: llvm-bcanalyzer %t/derivative_attr.swiftmodule | %FileCheck %s -check-prefix=BCANALYZER -// RUN: %target-sil-opt -disable-sil-linking -enable-sil-verify-all %t/derivative_attr.swiftmodule -o - | %FileCheck %s +// RUN: %target-sil-opt -enable-sil-verify-all %t/derivative_attr.swiftmodule -o - | %FileCheck %s // BCANALYZER-NOT: UnknownCode diff --git a/test/AutoDiff/Serialization/differentiable_attr.swift b/test/AutoDiff/Serialization/differentiable_attr.swift index d410f7a79ca6c..38fc0fbbba09e 100644 --- a/test/AutoDiff/Serialization/differentiable_attr.swift +++ b/test/AutoDiff/Serialization/differentiable_attr.swift @@ -1,7 +1,7 @@ // RUN: %empty-directory(%t) // RUN: %target-swift-frontend %s -emit-module -parse-as-library -o %t // RUN: llvm-bcanalyzer %t/differentiable_attr.swiftmodule | %FileCheck %s -check-prefix=BCANALYZER -// RUN: %target-sil-opt -disable-sil-linking -enable-sil-verify-all %t/differentiable_attr.swiftmodule -o - | %FileCheck %s +// RUN: %target-sil-opt -enable-sil-verify-all %t/differentiable_attr.swiftmodule -o - | %FileCheck %s // BCANALYZER-NOT: UnknownCode diff --git a/test/AutoDiff/Serialization/differentiable_function.swift b/test/AutoDiff/Serialization/differentiable_function.swift index 7431b6ffb210b..fd7c435af198e 100644 --- a/test/AutoDiff/Serialization/differentiable_function.swift +++ b/test/AutoDiff/Serialization/differentiable_function.swift @@ -1,7 +1,7 @@ // RUN: %empty-directory(%t) // RUN: %target-swift-frontend %s -emit-module -parse-as-library -o %t // RUN: llvm-bcanalyzer %t/differentiable_function.swiftmodule | %FileCheck %s -check-prefix=BCANALYZER -// RUN: %target-sil-opt -disable-sil-linking -enable-sil-verify-all %t/differentiable_function.swiftmodule -o - | %FileCheck %s +// RUN: %target-sil-opt -enable-sil-verify-all %t/differentiable_function.swiftmodule -o - | %FileCheck %s // BCANALYZER-NOT: UnknownCode diff --git a/test/AutoDiff/Serialization/transpose_attr.swift b/test/AutoDiff/Serialization/transpose_attr.swift index e4c5e44d30e84..2d5c3470af40a 100644 --- a/test/AutoDiff/Serialization/transpose_attr.swift +++ b/test/AutoDiff/Serialization/transpose_attr.swift @@ -1,7 +1,7 @@ // RUN: %empty-directory(%t) // RUN: %target-swift-frontend %s -emit-module -parse-as-library -o %t // RUN: llvm-bcanalyzer %t/transpose_attr.swiftmodule | %FileCheck %s -check-prefix=BCANALYZER -// RUN: %target-sil-opt -disable-sil-linking -enable-sil-verify-all %t/transpose_attr.swiftmodule -o - | %FileCheck %s +// RUN: %target-sil-opt -enable-sil-verify-all %t/transpose_attr.swiftmodule -o - | %FileCheck %s // BCANALYZER-NOT: UnknownCode diff --git a/test/Frontend/sil-merge-partial-modules.swift b/test/Frontend/sil-merge-partial-modules.swift index 2e0da856eefa0..49dda24b23270 100644 --- a/test/Frontend/sil-merge-partial-modules.swift +++ b/test/Frontend/sil-merge-partial-modules.swift @@ -5,7 +5,7 @@ // RUN: %target-swift-frontend -emit-module %t/partial.a.swiftmodule %t/partial.b.swiftmodule -module-name test -enable-library-evolution -sil-merge-partial-modules -disable-diagnostic-passes -disable-sil-perf-optzns -o %t/test.swiftmodule -// RUN: %target-sil-opt %t/test.swiftmodule -disable-sil-linking -emit-sorted-sil > %t/dump.sil +// RUN: %target-sil-opt %t/test.swiftmodule -emit-sorted-sil > %t/dump.sil // RUN: %FileCheck %s < %t/dump.sil // RUN: %FileCheck %s --check-prefix=NEGATIVE < %t/dump.sil diff --git a/test/ModuleInterface/ModuleCache/SerializedSIL.swiftinterface b/test/ModuleInterface/ModuleCache/SerializedSIL.swiftinterface index d7dd3611b3282..ef7700fca6839 100644 --- a/test/ModuleInterface/ModuleCache/SerializedSIL.swiftinterface +++ b/test/ModuleInterface/ModuleCache/SerializedSIL.swiftinterface @@ -3,7 +3,7 @@ // RUN: %empty-directory(%t) // RUN: echo 'import SerializedSIL' | %target-swift-frontend -typecheck -module-cache-path %t -I %S - -// RUN: %target-sil-opt -disable-sil-linking %t/SerializedSIL-*.swiftmodule -module-name SerializedSIL > %t/SerializedSIL.sil +// RUN: %target-sil-opt %t/SerializedSIL-*.swiftmodule -module-name SerializedSIL > %t/SerializedSIL.sil // RUN: %FileCheck %s < %t/SerializedSIL.sil // RUN: %FileCheck -check-prefix NEGATIVE %s < %t/SerializedSIL.sil diff --git a/test/SIL/Parser/stage.swift b/test/SIL/Parser/stage.swift index be8186a10d547..bde9bad406ed0 100644 --- a/test/SIL/Parser/stage.swift +++ b/test/SIL/Parser/stage.swift @@ -1,6 +1,6 @@ // RUN: %empty-directory(%t) // RUN: %target-swift-frontend -emit-module %s -O -parse-stdlib -parse-as-library -emit-module -o %t/stage.swiftmodule -// RUN: %target-sil-opt %t/stage.swiftmodule -disable-sil-linking -sil-disable-ast-dump -o %t/stage.sil +// RUN: %target-sil-opt %t/stage.swiftmodule -sil-disable-ast-dump -o %t/stage.sil // RUN: %target-sil-opt %t/stage.sil -o - | %FileCheck %s // FIXME: We create all SIL modules in the 'raw' stage regardless of the input diff --git a/test/SIL/Serialization/dynamically_replaceable.sil b/test/SIL/Serialization/dynamically_replaceable.sil index c052c04316eae..47482338d628c 100644 --- a/test/SIL/Serialization/dynamically_replaceable.sil +++ b/test/SIL/Serialization/dynamically_replaceable.sil @@ -1,6 +1,6 @@ // RUN: %empty-directory(%t) // RUN: %target-swift-frontend %s -emit-module -o %t/tmp.swiftmodule -// RUN: %target-sil-opt %t/tmp.swiftmodule -disable-sil-linking -emit-sorted-sil | %FileCheck %s +// RUN: %target-sil-opt %t/tmp.swiftmodule -emit-sorted-sil | %FileCheck %s // CHECK-DAG-LABEL: sil [serialized] [dynamically_replacable] [canonical] @test_dynamically_replaceable diff --git a/test/SIL/Serialization/globals.sil b/test/SIL/Serialization/globals.sil index 5dadb7b1d7a4b..a5bff2029b89b 100644 --- a/test/SIL/Serialization/globals.sil +++ b/test/SIL/Serialization/globals.sil @@ -1,6 +1,6 @@ // RUN: %empty-directory(%t) // RUN: %target-swift-frontend %s -emit-module -o %t/tmp.swiftmodule -// RUN: %target-sil-opt %t/tmp.swiftmodule -disable-sil-linking | %FileCheck %s +// RUN: %target-sil-opt %t/tmp.swiftmodule | %FileCheck %s sil_stage canonical diff --git a/test/SILOptimizer/switch_enum_resilient.swift b/test/SILOptimizer/switch_enum_resilient.swift index d61fced4a2334..ba21ed02f2ac0 100644 --- a/test/SILOptimizer/switch_enum_resilient.swift +++ b/test/SILOptimizer/switch_enum_resilient.swift @@ -4,16 +4,16 @@ // module vs. inlinable SIL. // RUN: %target-swift-frontend -emit-sil -O -primary-file %s -emit-module-path %t/O-fragile.swiftmodule | %FileCheck -check-prefix CHECK-ALL -check-prefix CHECK-NOINLINE -check-prefix CHECK-FRAGILE -check-prefix CHECK-FRAGILE-NOINLINE %s -// RUN: %target-sil-opt %t/O-fragile.swiftmodule -module-name switch_enum_resilient -emit-sorted-sil -disable-sil-linking | %FileCheck -check-prefix CHECK-ALL -check-prefix CHECK-FRAGILE %s +// RUN: %target-sil-opt %t/O-fragile.swiftmodule -module-name switch_enum_resilient -emit-sorted-sil | %FileCheck -check-prefix CHECK-ALL -check-prefix CHECK-FRAGILE %s // RUN: %target-swift-frontend -emit-sil -Osize -primary-file %s -emit-module-path %t/Osize-fragile.swiftmodule | %FileCheck -check-prefix CHECK-ALL -check-prefix CHECK-NOINLINE -check-prefix CHECK-FRAGILE -check-prefix CHECK-FRAGILE-NOINLINE %s -// RUN: %target-sil-opt %t/Osize-fragile.swiftmodule -module-name switch_enum_resilient -emit-sorted-sil -disable-sil-linking | %FileCheck -check-prefix CHECK-ALL -check-prefix CHECK-FRAGILE %s +// RUN: %target-sil-opt %t/Osize-fragile.swiftmodule -module-name switch_enum_resilient -emit-sorted-sil | %FileCheck -check-prefix CHECK-ALL -check-prefix CHECK-FRAGILE %s // RUN: %target-swift-frontend -emit-sil -enable-library-evolution -O -primary-file %s -emit-module-path %t/O-resilient.swiftmodule | %FileCheck -check-prefix CHECK-ALL -check-prefix CHECK-NOINLINE -check-prefix=CHECK-RESILIENT -check-prefix=CHECK-RESILIENT-NOINLINE %s -// RUN: %target-sil-opt %t/O-resilient.swiftmodule -module-name switch_enum_resilient -emit-sorted-sil -disable-sil-linking | %FileCheck -check-prefix CHECK-ALL -check-prefix=CHECK-RESILIENT -check-prefix CHECK-RESILIENT-INLINE %s +// RUN: %target-sil-opt %t/O-resilient.swiftmodule -module-name switch_enum_resilient -emit-sorted-sil | %FileCheck -check-prefix CHECK-ALL -check-prefix=CHECK-RESILIENT -check-prefix CHECK-RESILIENT-INLINE %s // RUN: %target-swift-frontend -emit-sil -enable-library-evolution -Osize -primary-file %s -emit-module-path %t/Osize-resilient.swiftmodule | %FileCheck -check-prefix CHECK-ALL -check-prefix CHECK-NOINLINE -check-prefix=CHECK-RESILIENT -check-prefix=CHECK-RESILIENT-NOINLINE %s -// RUN: %target-sil-opt %t/Osize-resilient.swiftmodule -module-name switch_enum_resilient -emit-sorted-sil -disable-sil-linking | %FileCheck -check-prefix CHECK-ALL -check-prefix=CHECK-RESILIENT -check-prefix CHECK-RESILIENT-INLINE %s +// RUN: %target-sil-opt %t/Osize-resilient.swiftmodule -module-name switch_enum_resilient -emit-sorted-sil | %FileCheck -check-prefix CHECK-ALL -check-prefix=CHECK-RESILIENT -check-prefix CHECK-RESILIENT-INLINE %s public enum Alpha : Int { case a, b, c, d, e diff --git a/test/Serialization/alignment.swift b/test/Serialization/alignment.swift index 62d8f193df3fd..a39b9c8f5455a 100644 --- a/test/Serialization/alignment.swift +++ b/test/Serialization/alignment.swift @@ -1,6 +1,6 @@ // RUN: %empty-directory(%t) // RUN: %target-swift-frontend %s -emit-module -parse-as-library -o %t -// RUN: %target-sil-opt -disable-sil-linking -enable-sil-verify-all %t/alignment.swiftmodule -o - | %FileCheck %s +// RUN: %target-sil-opt -enable-sil-verify-all %t/alignment.swiftmodule -o - | %FileCheck %s //CHECK: @_alignment(16) struct Foo { @_alignment(16) struct Foo {} diff --git a/test/Serialization/default-witness-table-deserialization.swift b/test/Serialization/default-witness-table-deserialization.swift index b0934b33cd09e..bf18e827a4971 100644 --- a/test/Serialization/default-witness-table-deserialization.swift +++ b/test/Serialization/default-witness-table-deserialization.swift @@ -1,4 +1,4 @@ -// RUN: %target-swift-frontend %s -module-name Test -emit-module -emit-module-path - -o /dev/null | %target-sil-opt -enable-sil-verify-all -disable-sil-linking -module-name="Test" | %FileCheck %s +// RUN: %target-swift-frontend %s -module-name Test -emit-module -emit-module-path - -o /dev/null | %target-sil-opt -enable-sil-verify-all -module-name="Test" | %FileCheck %s // Check that default witness tables are properly deserialized. // rdar://problem/29173229 diff --git a/test/Serialization/global_init.swift b/test/Serialization/global_init.swift index abc6500149d79..d00d1f56b7183 100644 --- a/test/Serialization/global_init.swift +++ b/test/Serialization/global_init.swift @@ -1,7 +1,7 @@ // RUN: %empty-directory(%t) // RUN: %target-swift-frontend -emit-module -parse-as-library -o %t %s // RUN: llvm-bcanalyzer %t/global_init.swiftmodule | %FileCheck %s -check-prefix=BCANALYZER -// RUN: %target-sil-opt -enable-sil-verify-all -disable-sil-linking %t/global_init.swiftmodule | %FileCheck %s +// RUN: %target-sil-opt -enable-sil-verify-all %t/global_init.swiftmodule | %FileCheck %s // BCANALYZER-NOT: UnknownCode diff --git a/test/Serialization/serialize_attr.swift b/test/Serialization/serialize_attr.swift index c5b48d90b497a..ea7c44e0b4547 100644 --- a/test/Serialization/serialize_attr.swift +++ b/test/Serialization/serialize_attr.swift @@ -2,7 +2,7 @@ // RUN: %empty-directory(%t) // RUN: %target-swift-frontend -module-name serialize_attr -emit-module -parse-as-library -o %t %s // RUN: llvm-bcanalyzer %t/serialize_attr.swiftmodule | %FileCheck %s -check-prefix=BCANALYZER -// RUN: %target-sil-opt -enable-sil-verify-all -disable-sil-linking %t/serialize_attr.swiftmodule | %FileCheck %s +// RUN: %target-sil-opt -enable-sil-verify-all %t/serialize_attr.swiftmodule | %FileCheck %s // BCANALYZER-NOT: UnknownCode diff --git a/tools/sil-func-extractor/SILFunctionExtractor.cpp b/tools/sil-func-extractor/SILFunctionExtractor.cpp index 9e4b49d5deef0..ee3711f4bb6f8 100644 --- a/tools/sil-func-extractor/SILFunctionExtractor.cpp +++ b/tools/sil-func-extractor/SILFunctionExtractor.cpp @@ -111,11 +111,6 @@ DisableASTDump("sil-disable-ast-dump", llvm::cl::Hidden, llvm::cl::init(false), llvm::cl::desc("Do not dump AST.")); -static llvm::cl::opt -DisableSILLinking("disable-sil-linking", - llvm::cl::init(true), - llvm::cl::desc("Disable SIL linking")); - // This function isn't referenced outside its translation unit, but it // can't use the "static" keyword because its address is used for // getMainExecutable (since some platforms don't support taking the @@ -275,12 +270,6 @@ int main(int argc, char **argv) { auto SILMod = performASTLowering(CI.getMainModule(), CI.getSILTypes(), CI.getSILOptions()); - // Load in all the SIL if we're allowed to. - if (Invocation.hasSerializedAST() && !extendedInfo.isSIB()) { - if (!DisableSILLinking) - SILMod->getSILLoader()->getAll(); - } - if (CommandLineFunctionNames.empty() && FunctionNameFile.empty()) return CI.getASTContext().hadError(); diff --git a/tools/sil-llvm-gen/SILLLVMGen.cpp b/tools/sil-llvm-gen/SILLLVMGen.cpp index 88f7a897292e1..80ec715c9f42b 100644 --- a/tools/sil-llvm-gen/SILLLVMGen.cpp +++ b/tools/sil-llvm-gen/SILLLVMGen.cpp @@ -195,14 +195,6 @@ int main(int argc, char **argv) { CI.getSILOptions()); } - // Load the SIL if we have a non-SIB serialized module. SILGen handles SIB for - // us. - if (Invocation.hasSerializedAST() && !extendedInfo.isSIB()) { - auto SL = SerializedSILLoader::create( - CI.getASTContext(), SILMod.get(), nullptr); - SL->getAll(); - } - const PrimarySpecificPaths PSPs(OutputFilename, InputFilename); auto Mod = performIRGeneration(Opts, CI.getMainModule(), std::move(SILMod), CI.getMainModule()->getName().str(), PSPs, diff --git a/tools/sil-nm/SILNM.cpp b/tools/sil-nm/SILNM.cpp index f8c0810ed5285..bd5fbbece62cd 100644 --- a/tools/sil-nm/SILNM.cpp +++ b/tools/sil-nm/SILNM.cpp @@ -188,15 +188,6 @@ int main(int argc, char **argv) { auto SILMod = performASTLowering(CI.getMainModule(), CI.getSILTypes(), CI.getSILOptions()); - - // Load the SIL if we have a non-SIB serialized module. SILGen handles SIB for - // us. - if (Invocation.hasSerializedAST() && !extendedInfo.isSIB()) { - auto SL = SerializedSILLoader::create( - CI.getASTContext(), SILMod.get(), nullptr); - SL->getAll(); - } - nmModule(SILMod.get()); return CI.getASTContext().hadError(); diff --git a/tools/sil-opt/SILOpt.cpp b/tools/sil-opt/SILOpt.cpp index 1ef2ffcb2dd61..490bf85550abb 100644 --- a/tools/sil-opt/SILOpt.cpp +++ b/tools/sil-opt/SILOpt.cpp @@ -172,10 +172,6 @@ static llvm::cl::opt AssertConfId("assert-conf-id", llvm::cl::Hidden, llvm::cl::init(0)); -static llvm::cl::opt -DisableSILLinking("disable-sil-linking", - llvm::cl::desc("Disable SIL linking")); - static llvm::cl::opt SILInlineThreshold("sil-inline-threshold", llvm::cl::Hidden, llvm::cl::init(-1)); @@ -464,12 +460,6 @@ int main(int argc, char **argv) { } SILMod->setSerializeSILAction([]{}); - // Load in all the SIL if we're allowed to. - if (Invocation.hasSerializedAST() && !extendedInfo.isSIB()) { - if (!DisableSILLinking) - SILMod->getSILLoader()->getAll(); - } - if (!RemarksFilename.empty()) { llvm::Expected formatOrErr = llvm::remarks::parseFormat(RemarksFormat); From 0c9c606d2814e98bc42edb69e837c77d433a187f Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Tue, 23 Jun 2020 16:05:49 -0700 Subject: [PATCH 3/5] Remove MergePartialModules from SILOptions Its use in deserialization can be replaced with a more general check for whether we're deserializing into the same module. Its use in the SILVerifier is subsumed by the check for whether the SILModule is canonical, which it isn't during merge-modules. --- include/swift/AST/SILOptions.h | 5 ----- lib/Frontend/CompilerInvocation.cpp | 3 --- lib/SIL/Verifier/SILVerifier.cpp | 14 ++++---------- lib/Serialization/DeserializeSIL.cpp | 8 +++++++- 4 files changed, 11 insertions(+), 19 deletions(-) diff --git a/include/swift/AST/SILOptions.h b/include/swift/AST/SILOptions.h index 1ff15e19922e2..af8d8c29f2c69 100644 --- a/include/swift/AST/SILOptions.h +++ b/include/swift/AST/SILOptions.h @@ -40,11 +40,6 @@ class SILOptions { /// Controls the aggressiveness of the loop unroller. int UnrollThreshold = 250; - /// Controls whether to pull in SIL from partial modules during the - /// merge modules step. Could perhaps be merged with the link mode - /// above but the interactions between all the flags are tricky. - bool MergePartialModules = false; - /// Remove all runtime assertions during optimizations. bool RemoveRuntimeAsserts = false; diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp index fa1ee67f03b91..ded883e643baa 100644 --- a/lib/Frontend/CompilerInvocation.cpp +++ b/lib/Frontend/CompilerInvocation.cpp @@ -1049,9 +1049,6 @@ static bool ParseSILArgs(SILOptions &Opts, ArgList &Args, if (FEOpts.RequestedAction == FrontendOptions::ActionType::EmitModuleOnly) Opts.StopOptimizationAfterSerialization = true; - if (Args.hasArg(OPT_sil_merge_partial_modules)) - Opts.MergePartialModules = true; - // Propagate the typechecker's understanding of // -experimental-skip-non-inlinable-function-bodies to SIL. Opts.SkipNonInlinableFunctionBodies = TCOpts.SkipNonInlinableFunctionBodies; diff --git a/lib/SIL/Verifier/SILVerifier.cpp b/lib/SIL/Verifier/SILVerifier.cpp index 84e74fb9db4ee..cca9546938961 100644 --- a/lib/SIL/Verifier/SILVerifier.cpp +++ b/lib/SIL/Verifier/SILVerifier.cpp @@ -1751,8 +1751,9 @@ class SILVerifier : public SILVerifierBase { "[dynamically_replaceable] function"); // In canonical SIL, direct reference to a shared_external declaration - // is an error; we should have deserialized a body. In raw SIL, we may - // not have deserialized the body yet. + // is an error; we should have deserialized a body. In raw SIL, including + // the merge-modules phase, we may not have deserialized the body yet as we + // may not have run the SILLinker pass. if (F.getModule().getStage() >= SILStage::Canonical) { if (RefF->isExternalDeclaration()) { require(SingleFunction || @@ -5560,20 +5561,13 @@ void SILModule::verify() const { // Uniquing set to catch symbol name collisions. llvm::DenseSet symbolNames; - // When merging partial modules, we only link functions from the current - // module, without enabling "LinkAll" mode or running the SILLinker pass; - // in this case, we need to relax some of the checks. - bool SingleFunction = false; - if (getOptions().MergePartialModules) - SingleFunction = true; - // Check all functions. for (const SILFunction &f : *this) { if (!symbolNames.insert(f.getName()).second) { llvm::errs() << "Symbol redefined: " << f.getName() << "!\n"; assert(false && "triggering standard assertion failure routine"); } - f.verify(SingleFunction); + f.verify(/*singleFunction*/ false); } // Check all globals. diff --git a/lib/Serialization/DeserializeSIL.cpp b/lib/Serialization/DeserializeSIL.cpp index 6cc6d6f7e8a15..74874e75a792e 100644 --- a/lib/Serialization/DeserializeSIL.cpp +++ b/lib/Serialization/DeserializeSIL.cpp @@ -597,7 +597,13 @@ SILDeserializer::readSILFunctionChecked(DeclID FID, SILFunction *existingFn, fn->setSerialized(IsSerialized_t(isSerialized)); - if (SILMod.getOptions().MergePartialModules) + // If the serialized function comes from the same module, we're merging + // modules, and can update the the linkage directly. This is needed to + // correctly update the linkage for forward declarations to entities defined + // in another file of the same module – we want to ensure the linkage + // reflects the fact that the entity isn't really external and shouldn't be + // dropped from the resulting merged module. + if (getFile()->getParentModule() == SILMod.getSwiftModule()) fn->setLinkage(linkage); // Don't override the transparency or linkage of a function with From 9144ff57c06c136caf6a811671589ec1c03c49a2 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Tue, 23 Jun 2020 16:05:49 -0700 Subject: [PATCH 4/5] Alias -sil-merge-partial-modules to -merge-modules The option now has no additional meaning. Eventually it should be removed, but for now just alias it to `-merge-modules`. --- include/swift/Option/FrontendOptions.td | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/swift/Option/FrontendOptions.td b/include/swift/Option/FrontendOptions.td index 2da46f6f1c087..b9f4adbceb59a 100644 --- a/include/swift/Option/FrontendOptions.td +++ b/include/swift/Option/FrontendOptions.td @@ -554,8 +554,9 @@ def sil_unroll_threshold : Separate<["-"], "sil-unroll-threshold">, MetaVarName<"<250>">, HelpText<"Controls the aggressiveness of loop unrolling">; +// FIXME: This option is now redundant and should eventually be removed. def sil_merge_partial_modules : Flag<["-"], "sil-merge-partial-modules">, - HelpText<"Merge SIL from all partial swiftmodules into the final module">; + Alias; def sil_verify_all : Flag<["-"], "sil-verify-all">, HelpText<"Verify SIL after each transform">; From 751e68177c5165601575087cbc3336063b83bf4d Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Tue, 23 Jun 2020 16:05:50 -0700 Subject: [PATCH 5/5] [test] Remove uses of -sil-merge-partial-modules Replace with -merge-modules, which means we can also remove the now redundant `-disable-diagnostic-passes` & `-disable-sil-perf-optzns` options. --- test/Frontend/sil-merge-partial-modules-stdlib.swift | 2 +- test/Frontend/sil-merge-partial-modules.swift | 2 +- test/SPI/private_swiftinterface.swift | 2 +- test/Sema/composition_extension.swift | 2 +- test/Serialization/property_wrappers.swift | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/Frontend/sil-merge-partial-modules-stdlib.swift b/test/Frontend/sil-merge-partial-modules-stdlib.swift index 2e7cd4f7cd1a4..1a9c48ae2d408 100644 --- a/test/Frontend/sil-merge-partial-modules-stdlib.swift +++ b/test/Frontend/sil-merge-partial-modules-stdlib.swift @@ -2,7 +2,7 @@ // RUN: %target-swift-frontend -emit-module -primary-file %s -module-name test -o %t/partial.swiftmodule -O -// RUN: %target-swift-frontend -emit-module %t/partial.swiftmodule -module-name test -sil-merge-partial-modules -disable-diagnostic-passes -disable-sil-perf-optzns -o %t/test.swiftmodule +// RUN: %target-swift-frontend -merge-modules -emit-module %t/partial.swiftmodule -module-name test -o %t/test.swiftmodule public func makeMirror(object x: Any) -> Mirror { return Mirror(reflecting: x) diff --git a/test/Frontend/sil-merge-partial-modules.swift b/test/Frontend/sil-merge-partial-modules.swift index 49dda24b23270..e4dbe7a237f5b 100644 --- a/test/Frontend/sil-merge-partial-modules.swift +++ b/test/Frontend/sil-merge-partial-modules.swift @@ -3,7 +3,7 @@ // RUN: %target-swift-frontend -emit-module -primary-file %s %S/Inputs/sil-merge-partial-modules-other.swift -module-name test -enable-library-evolution -o %t/partial.a.swiftmodule // RUN: %target-swift-frontend -emit-module %s -primary-file %S/Inputs/sil-merge-partial-modules-other.swift -module-name test -enable-library-evolution -o %t/partial.b.swiftmodule -// RUN: %target-swift-frontend -emit-module %t/partial.a.swiftmodule %t/partial.b.swiftmodule -module-name test -enable-library-evolution -sil-merge-partial-modules -disable-diagnostic-passes -disable-sil-perf-optzns -o %t/test.swiftmodule +// RUN: %target-swift-frontend -merge-modules -emit-module %t/partial.a.swiftmodule %t/partial.b.swiftmodule -module-name test -enable-library-evolution -o %t/test.swiftmodule // RUN: %target-sil-opt %t/test.swiftmodule -emit-sorted-sil > %t/dump.sil // RUN: %FileCheck %s < %t/dump.sil diff --git a/test/SPI/private_swiftinterface.swift b/test/SPI/private_swiftinterface.swift index 8be3d34925410..cc9d21f42928d 100644 --- a/test/SPI/private_swiftinterface.swift +++ b/test/SPI/private_swiftinterface.swift @@ -17,7 +17,7 @@ /// Serialize and deserialize this module, then print. // RUN: %target-swift-frontend -emit-module %s -emit-module-path %t/merged-partial.swiftmodule -swift-version 5 -I %t -module-name merged -enable-library-evolution -// RUN: %target-swift-frontend -merge-modules %t/merged-partial.swiftmodule -module-name merged -sil-merge-partial-modules -emit-module -emit-module-path %t/merged.swiftmodule -I %t -emit-module-interface-path %t/merged.swiftinterface -emit-private-module-interface-path %t/merged.private.swiftinterface -enable-library-evolution -swift-version 5 -I %t +// RUN: %target-swift-frontend -merge-modules %t/merged-partial.swiftmodule -module-name merged -emit-module -emit-module-path %t/merged.swiftmodule -I %t -emit-module-interface-path %t/merged.swiftinterface -emit-private-module-interface-path %t/merged.private.swiftinterface -enable-library-evolution -swift-version 5 -I %t // RUN: %FileCheck -check-prefix=CHECK-PUBLIC %s < %t/merged.swiftinterface // RUN: %FileCheck -check-prefix=CHECK-PRIVATE %s < %t/merged.private.swiftinterface diff --git a/test/Sema/composition_extension.swift b/test/Sema/composition_extension.swift index 933a06436e29d..1476d3e2197d7 100644 --- a/test/Sema/composition_extension.swift +++ b/test/Sema/composition_extension.swift @@ -2,7 +2,7 @@ // RUN: %empty-directory(%t) // RUN: %target-swift-frontend -primary-file %s %S/Inputs/composition_extension_usage.swift -emit-module-path %t/P-partial.swiftmodule -module-name SR11227 -enable-testing // RUN: %target-swift-frontend -primary-file %S/Inputs/composition_extension_usage.swift %s -emit-module-path %t/S-partial.swiftmodule -module-name SR11227 -enable-testing -// RUN: %target-swift-frontend -sil-merge-partial-modules %t/P-partial.swiftmodule %t/S-partial.swiftmodule -emit-module -o %t/SR11227.swiftmodule +// RUN: %target-swift-frontend -merge-modules -emit-module %t/P-partial.swiftmodule %t/S-partial.swiftmodule -o %t/SR11227.swiftmodule protocol P1 {} diff --git a/test/Serialization/property_wrappers.swift b/test/Serialization/property_wrappers.swift index a03a2dba38a6e..0167eaf3a5bc3 100644 --- a/test/Serialization/property_wrappers.swift +++ b/test/Serialization/property_wrappers.swift @@ -1,7 +1,7 @@ // RUN: %empty-directory(%t) // RUN: %empty-directory(%t-scratch) // RUN: %target-swift-frontend -emit-module -o %t-scratch/def_property_wrappers~partial.swiftmodule -primary-file %S/Inputs/def_property_wrappers.swift -module-name def_property_wrappers -enable-testing -// RUN: %target-swift-frontend -merge-modules -emit-module -parse-as-library -sil-merge-partial-modules -disable-diagnostic-passes -disable-sil-perf-optzns -enable-testing %t-scratch/def_property_wrappers~partial.swiftmodule -module-name def_property_wrappers -o %t/def_property_wrappers.swiftmodule +// RUN: %target-swift-frontend -merge-modules -emit-module -parse-as-library -enable-testing %t-scratch/def_property_wrappers~partial.swiftmodule -module-name def_property_wrappers -o %t/def_property_wrappers.swiftmodule // RUN: %target-swift-frontend -typecheck -I%t -verify %s -verify-ignore-unknown @testable import def_property_wrappers