Skip to content

Preserve SIL when merging modules #8388

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
Sep 15, 2017
Merged

Preserve SIL when merging modules #8388

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
07fe6ae
Add test case for imported conformance usage after Sema tear-down
slavapestov Sep 14, 2017
ef58349
Driver: Pass the new -sil-merge-partial-modules flag
slavapestov Apr 1, 2017
cf9e266
AST: Use getSuperclassForDecl() in one spot
slavapestov Sep 14, 2017
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
6 changes: 2 additions & 4 deletions lib/AST/Module.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -663,13 +663,11 @@ ModuleDecl::lookupConformance(Type type, ProtocolDecl *protocol) {
if (auto inherited = dyn_cast<InheritedProtocolConformance>(conformance)) {
// Dig out the conforming nominal type.
auto rootConformance = inherited->getRootNormalConformance();
auto conformingNominal
auto conformingClass
= rootConformance->getType()->getClassOrBoundGenericClass();

// Map up to our superclass's type.
Type superclassTy = type->getSuperclass();
while (superclassTy->getAnyNominal() != conformingNominal)
superclassTy = superclassTy->getSuperclass();
auto superclassTy = type->getSuperclassForDecl(conformingClass);

// Compute the conformance for the inherited type.
auto inheritedConformance = lookupConformance(superclassTy, protocol);
Expand Down
8 changes: 8 additions & 0 deletions lib/Driver/ToolChains.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -687,6 +687,14 @@ ToolChain::constructInvocation(const MergeModuleJobAction &job,
// serialized ASTs.
Arguments.push_back("-parse-as-library");

// Merge serialized SIL from partial modules.
Arguments.push_back("-sil-merge-partial-modules");

// Disable SIL optimization passes; we've already optimized the code in each
// partial mode.
Arguments.push_back("-disable-diagnostic-passes");
Copy link
Contributor

Choose a reason for hiding this comment

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

*sigh* I wish this was inferred, but meanwhile this seems reasonable.

Arguments.push_back("-disable-sil-perf-optzns");
Copy link
Contributor

Choose a reason for hiding this comment

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

We should change this to also disable running the Onone passes.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jrose-apple I think in such a case in our pipeline, we need a better separation conceptually in between the "diagnostic passes" and the "mandatory optimizations". Those should be conceptually two different groups of passes. I would rather have a separate flag. Perhaps:

-disable-sil-mandatory-optzns.

And then a third flag that just disables /all/ swift optzns, i.e.:

-disable-sil-optzns

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm talking about SILPassPipelinePlan::getOnonePassPipeline. That's not mandatory optimizations.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, "mandatory optimizations" is an oxymoron, and therefore I wouldn't expect -disable-sil-optzns to disable the mandatory passes. Skipping the diagnostic passes and the mandatory passes should just be automatic from seeing the SIL stage.

Copy link
Contributor

Choose a reason for hiding this comment

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

I talked with @jrose-apple offline. Turns out my memory was wrong and the -Onone pass pipeline is not what I thought it was. The -Onone pass pipeline is a pass pipeline that includes extra optimizations run at -Onone.


addCommonFrontendArgs(*this, context.OI, context.Output, context.Args,
Arguments);
context.Args.AddLastArg(Arguments, options::OPT_import_objc_header);
Expand Down
8 changes: 8 additions & 0 deletions test/multifile/imported-conformance/option-set/library.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// RUN: true

import Foundation

@inline(__always)
public func replace(rgx: NSRegularExpression, in: String, with: String, x: NSRange) -> String {
return rgx.stringByReplacingMatches(in: `in`, options: [], range: x, withTemplate: with)
}
15 changes: 15 additions & 0 deletions test/multifile/imported-conformance/option-set/main.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// RUN: %empty-directory(%t)

// RUN: %empty-directory(%t/linker)
// RUN: %target-build-swift -emit-module -emit-library %S/library.swift -o %t/linker/liblibrary.%target-dylib-extension -emit-module-path %t/linker/library.swiftmodule -module-name library
// RUN: %target-build-swift %S/main.swift -I %t/linker/ -L %t/linker/ -llibrary -o %t/linker/main

// REQUIRES: executable_test
// REQUIRES: objc_interop

import Foundation
import library

public func rreplace(rgx: NSRegularExpression, in: String, with: String, x: NSRange) -> String {
return replace(rgx: rgx, in: `in`, with: with, x: x)
}
5 changes: 1 addition & 4 deletions validation-test/Evolution/test_function_change_transparent_body.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
// RUN: %target-resilience-test-wmo
// RUN: %target-resilience-test
// REQUIRES: executable_test

// FIXME: shouldn't need -whole-module-optimization here; we need to fix the
// frontend to merge serialized SIL functions from different translation units

import StdlibUnittest
import function_change_transparent_body

Expand Down