Skip to content

SIL: Remove special meaning for @_semantics("stdlib_binary_only") #12237

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.

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 1 commit into from
Oct 3, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 7 additions & 9 deletions docs/HighLevelSILOptimizations.rst
Original file line number Diff line number Diff line change
Expand Up @@ -117,15 +117,13 @@ Cloning code from the standard library
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The Swift compiler can copy code from the standard library into the
application. This allows the optimizer to inline calls from stdlib and improve
the performance of code that uses common operators such as '++' or basic
containers such as Array. However, importing code from the standard library can
increase the binary size. Marking functions with @_semantics("stdlib_binary_only")
will prevent the copying of the marked function from the standard library into the
user program.

Notice that this annotation is similar to the resilient annotation that will
disallow the cloning of code into the user application.
application for functions marked @_inlineable. This allows the optimizer to
inline calls from the stdlib and improve the performance of code that uses
common operators such as '+=' or basic containers such as Array. However,
importing code from the standard library can increase the binary size.

To prevent copying of functions from the standard library into the user
program, make sure the function in question is not marked @_inlineable.

Array
~~~~~
Expand Down
29 changes: 0 additions & 29 deletions lib/SIL/Linker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,26 +26,6 @@ using namespace Lowering;

STATISTIC(NumFuncLinked, "Number of SIL functions linked");

//===----------------------------------------------------------------------===//
// Utility
//===----------------------------------------------------------------------===//

/// \return True if the function \p F should be imported into the current
/// module.
static bool shouldImportFunction(SILFunction *F) {
// Skip functions that are marked with the 'no import' tag. These
// are functions that we don't want to copy from the module.
if (F->hasSemanticsAttr("stdlib_binary_only")) {
// If we are importing a function declaration mark it as external since we
// are not importing the body.
if (F->isExternalDeclaration())
F->setLinkage(SILLinkage::PublicExternal);
return false;
}

return true;
}

//===----------------------------------------------------------------------===//
// Linker Helpers
//===----------------------------------------------------------------------===//
Expand All @@ -55,9 +35,6 @@ bool SILLinkerVisitor::processFunction(SILFunction *F) {
if (Mode == LinkingMode::LinkNone)
return false;

if (!shouldImportFunction(F))
return false;

// If F is a declaration, first deserialize it.
if (F->isExternalDeclaration()) {
auto *NewFn = Loader->lookupSILFunction(F);
Expand Down Expand Up @@ -334,9 +311,6 @@ bool SILLinkerVisitor::process() {
while (!Worklist.empty()) {
auto *Fn = Worklist.pop_back_val();

if (!shouldImportFunction(Fn))
continue;

DEBUG(llvm::dbgs() << "Process imports in function: "
<< Fn->getName() << "\n");

Expand All @@ -346,9 +320,6 @@ bool SILLinkerVisitor::process() {
if (visit(&I)) {
for (auto *F : FunctionDeserializationWorklist) {

if (!shouldImportFunction(F))
continue;

DEBUG(llvm::dbgs() << "Imported function: "
<< F->getName() << "\n");
F->setBare(IsBare);
Expand Down
9 changes: 4 additions & 5 deletions lib/SILOptimizer/IPO/GlobalOpt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1353,11 +1353,9 @@ void SILGlobalOpt::replaceFindStringCall(ApplyInst *FindStringCall) {
if (!FD)
return;

std::string Mangled = SILDeclRef(FD, SILDeclRef::Kind::Func).mangle();
SILFunction *replacementFunc = Module->findFunction(Mangled,
SILLinkage::PublicExternal);
if (!replacementFunc)
return;
SILDeclRef declRef(FD, SILDeclRef::Kind::Func);
SILFunction *replacementFunc = Module->getOrCreateFunction(
FindStringCall->getLoc(), declRef, NotForDefinition);

SILFunctionType *FTy = replacementFunc->getLoweredFunctionType();
if (FTy->getNumParameters() != 3)
Expand All @@ -1367,6 +1365,7 @@ void SILGlobalOpt::replaceFindStringCall(ApplyInst *FindStringCall) {
NominalTypeDecl *cacheDecl = cacheType.getNominalOrBoundGenericNominal();
if (!cacheDecl)
return;

SILType wordTy = cacheType.getFieldType(
cacheDecl->getStoredProperties().front(), *Module);

Expand Down
6 changes: 1 addition & 5 deletions lib/SILOptimizer/Utils/Local.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -423,11 +423,7 @@ SILLinkage swift::getSpecializedLinkage(SILFunction *F, SILLinkage L) {
return SILLinkage::Private;
}

// Treat stdlib_binary_only specially. We don't serialize the body of
// stdlib_binary_only functions so we can't mark them as Shared (making
// their visibility in the dylib hidden).
return F->hasSemanticsAttr("stdlib_binary_only") ? SILLinkage::Public
: SILLinkage::Shared;
return SILLinkage::Shared;
}

/// Remove all instructions in the body of \p BB in safe manner by using
Expand Down
2 changes: 1 addition & 1 deletion stdlib/private/StdlibUnittest/StdlibUnittest.swift.gyb
Original file line number Diff line number Diff line change
Expand Up @@ -627,7 +627,7 @@ func _stdlib_installTrapInterceptor()
#endif

// Avoid serializing references to objc_setUncaughtExceptionHandler in SIL.
@inline(never) @_semantics("stdlib_binary_only")
@inline(never)
func _childProcess() {
_stdlib_installTrapInterceptor()

Expand Down
12 changes: 3 additions & 9 deletions stdlib/public/core/AssertCommon.swift
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,8 @@ func _fatalErrorFlags() -> UInt32 {
///
/// This function should not be inlined because it is cold and inlining just
/// bloats code.
@_inlineable // FIXME(sil-serialize-all)
@_versioned
@_versioned // FIXME(sil-serialize-all)
@inline(never)
@_semantics("stdlib_binary_only")
internal func _assertionFailure(
_ prefix: StaticString, _ message: StaticString,
file: StaticString, line: UInt,
Expand Down Expand Up @@ -114,10 +112,8 @@ internal func _assertionFailure(
///
/// This function should not be inlined because it is cold and inlining just
/// bloats code.
@_inlineable // FIXME(sil-serialize-all)
@_versioned
@_versioned // FIXME(sil-serialize-all)
@inline(never)
@_semantics("stdlib_binary_only")
internal func _assertionFailure(
_ prefix: StaticString, _ message: String,
file: StaticString, line: UInt,
Expand Down Expand Up @@ -146,10 +142,8 @@ internal func _assertionFailure(
///
/// This function should not be inlined because it is cold and it inlining just
/// bloats code.
@_inlineable // FIXME(sil-serialize-all)
@_versioned
@_versioned // FIXME(sil-serialize-all)
@inline(never)
@_semantics("stdlib_binary_only")
@_semantics("arc.programtermination_point")
internal func _fatalErrorMessage(
_ prefix: StaticString, _ message: StaticString,
Expand Down
6 changes: 2 additions & 4 deletions stdlib/public/core/DebuggerSupport.swift
Original file line number Diff line number Diff line change
Expand Up @@ -305,10 +305,8 @@ public enum _DebuggerSupport {
}

// LLDB uses this function in expressions, and if it is inlined the resulting
// LLVM IR is enormous. As a result, to improve LLDB performance we have made
// this stdlib_binary_only, which prevents inlining.
@_inlineable // FIXME(sil-serialize-all)
@_semantics("stdlib_binary_only")
// LLVM IR is enormous. As a result, to improve LLDB performance we are not
// making it @_inlineable.
public static func stringForPrintObject(_ value: Any) -> String {
var maxItemCounter = Int.max
var refs = Set<ObjectIdentifier>()
Expand Down
2 changes: 0 additions & 2 deletions stdlib/public/core/HashedCollections.swift.gyb
Original file line number Diff line number Diff line change
Expand Up @@ -2862,9 +2862,7 @@ public func _dictionaryUpCast<DerivedKey, DerivedValue, BaseKey, BaseValue>(
///
/// - Precondition: `SwiftKey` and `SwiftValue` are bridged to Objective-C,
/// and at least one of them requires non-trivial bridging.
@_inlineable // FIXME(sil-serialize-all)
@inline(never)
@_semantics("stdlib_binary_only")
public func _dictionaryBridgeToObjectiveC<
SwiftKey, SwiftValue, ObjCKey, ObjCValue
>(
Expand Down
2 changes: 0 additions & 2 deletions stdlib/public/core/OutputStream.swift
Original file line number Diff line number Diff line change
Expand Up @@ -343,11 +343,9 @@ internal func _adHocPrint_unlocked<T, TargetStream : TextOutputStream>(
}
}

@_inlineable // FIXME(sil-serialize-all)
@_versioned
@inline(never)
@_semantics("optimize.sil.specialize.generic.never")
@_semantics("stdlib_binary_only")
internal func _print_unlocked<T, TargetStream : TextOutputStream>(
_ value: T, _ target: inout TargetStream
) {
Expand Down
8 changes: 0 additions & 8 deletions stdlib/public/core/Print.swift
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,7 @@
/// space (`" "`).
/// - terminator: The string to print after all items have been printed. The
/// default is a newline (`"\n"`).
@_inlineable // FIXME(sil-serialize-all)
@inline(never)
@_semantics("stdlib_binary_only")
public func print(
_ items: Any...,
separator: String = " ",
Expand Down Expand Up @@ -110,9 +108,7 @@ public func print(
/// space (`" "`).
/// - terminator: The string to print after all items have been printed. The
/// default is a newline (`"\n"`).
@_inlineable // FIXME(sil-serialize-all)
@inline(never)
@_semantics("stdlib_binary_only")
public func debugPrint(
_ items: Any...,
separator: String = " ",
Expand Down Expand Up @@ -228,10 +224,8 @@ public func debugPrint<Target : TextOutputStream>(
items, separator: separator, terminator: terminator, to: &output)
}

@_inlineable // FIXME(sil-serialize-all)
@_versioned
@inline(never)
@_semantics("stdlib_binary_only")
internal func _print<Target : TextOutputStream>(
_ items: [Any],
separator: String = " ",
Expand All @@ -249,10 +243,8 @@ internal func _print<Target : TextOutputStream>(
output.write(terminator)
}

@_inlineable // FIXME(sil-serialize-all)
@_versioned
@inline(never)
@_semantics("stdlib_binary_only")
internal func _debugPrint<Target : TextOutputStream>(
_ items: [Any],
separator: String = " ",
Expand Down
2 changes: 0 additions & 2 deletions stdlib/public/core/REPL.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@ func _replPrintLiteralString(_ text: String) {
}

/// Print the debug representation of `value`, followed by a newline.
@_inlineable // FIXME(sil-serialize-all)
@inline(never)
@_semantics("stdlib_binary_only")
public // COMPILER_INTRINSIC
func _replDebugPrintln<T>(_ value: T) {
debugPrint(value)
Expand Down
23 changes: 8 additions & 15 deletions stdlib/public/core/StringBridge.swift
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,8 @@ func _stdlib_binary_CFStringGetCharactersPtr(

/// Bridges `source` to `Swift.String`, assuming that `source` has non-ASCII
/// characters (does not apply ASCII optimizations).
@_inlineable // FIXME(sil-serialize-all)
@_versioned // FIXME(sil-serialize-all)
@inline(never) @_semantics("stdlib_binary_only") // Hide the CF dependency
@inline(never) // Hide the CF dependency
Copy link
Member

Choose a reason for hiding this comment

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

IIRC The comment is solely from the lack of @_inlineable. I don't think it goes on @inline(never), but I don't know where to put it. Perhaps under the new paradigm it's just not an interesting distinction. Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the comment is still useful for future maintainers in case they decide to put @_inlineable back in?

Copy link
Contributor

Choose a reason for hiding this comment

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

In theory we shouldn't need the inline-never anymore either, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Without inline-never it can be inlined inside the stdlib itself.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, but that should be fine now.

Copy link
Member

Choose a reason for hiding this comment

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

Outline-and-inline-never is still a useful, albeit niche, tool inside the stdlib when Builtin.expect is insufficient.

func _cocoaStringToSwiftString_NonASCII(
_ source: _CocoaString
) -> String {
Expand All @@ -70,9 +69,8 @@ func _cocoaStringToSwiftString_NonASCII(

/// Produces a `_StringBuffer` from a given subrange of a source
/// `_CocoaString`, having the given minimum capacity.
@_inlineable // FIXME(sil-serialize-all)
@_versioned // FIXME(sil-serialize-all)
@inline(never) @_semantics("stdlib_binary_only") // Hide the CF dependency
@inline(never) // Hide the CF dependency
internal func _cocoaStringToContiguous(
source: _CocoaString, range: Range<Int>, minimumCapacity: Int
) -> _StringBuffer {
Expand All @@ -94,9 +92,8 @@ internal func _cocoaStringToContiguous(

/// Reads the entire contents of a _CocoaString into contiguous
/// storage of sufficient capacity.
@_inlineable // FIXME(sil-serialize-all)
@_versioned // FIXME(sil-serialize-all)
@inline(never) @_semantics("stdlib_binary_only") // Hide the CF dependency
@inline(never) // Hide the CF dependency
internal func _cocoaStringReadAll(
_ source: _CocoaString, _ destination: UnsafeMutablePointer<UTF16.CodeUnit>
) {
Expand All @@ -105,9 +102,8 @@ internal func _cocoaStringReadAll(
location: 0, length: _swift_stdlib_CFStringGetLength(source)), destination)
}

@_inlineable // FIXME(sil-serialize-all)
@_versioned // FIXME(sil-serialize-all)
@inline(never) @_semantics("stdlib_binary_only") // Hide the CF dependency
@inline(never) // Hide the CF dependency
internal func _cocoaStringSlice(
_ target: _StringCore, _ bounds: Range<Int>
) -> _StringCore {
Expand All @@ -126,9 +122,8 @@ internal func _cocoaStringSlice(
return String(_cocoaString: cfResult)._core
}

@_inlineable // FIXME(sil-serialize-all)
@_versioned
@inline(never) @_semantics("stdlib_binary_only") // Hide the CF dependency
@_versioned // FIXME(sil-serialize-all)
@inline(never) // Hide the CF dependency
internal func _cocoaStringSubscript(
_ target: _StringCore, _ position: Int
) -> UTF16.CodeUnit {
Expand All @@ -151,8 +146,7 @@ internal var kCFStringEncodingASCII : _swift_shims_CFStringEncoding {
}

extension String {
@_inlineable // FIXME(sil-serialize-all)
@inline(never) @_semantics("stdlib_binary_only") // Hide the CF dependency
@inline(never) // Hide the CF dependency
public // SPI(Foundation)
init(_cocoaString: AnyObject) {
if let wrapped = _cocoaString as? _NSContiguousString {
Expand Down Expand Up @@ -374,8 +368,7 @@ extension String {
return _NSContiguousString(_core)
}

@_inlineable // FIXME(sil-serialize-all)
@inline(never) @_semantics("stdlib_binary_only") // Hide the CF dependency
@inline(never) // Hide the CF dependency
public func _bridgeToObjectiveCImpl() -> AnyObject {
return _stdlib_binary_bridgeToObjectiveCImpl()
}
Expand Down
4 changes: 1 addition & 3 deletions stdlib/public/core/StringComparable.swift
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,7 @@ extension String {
#endif

/// Compares two strings with the Unicode Collation Algorithm.
@_inlineable // FIXME(sil-serialize-all)
@inline(never)
@_semantics("stdlib_binary_only") // Hide the CF/ICU dependency
@inline(never) // Hide the CF/ICU dependency
public // @testable
func _compareDeterministicUnicodeCollation(_ rhs: String) -> Int {
// Note: this operation should be consistent with equality comparison of
Expand Down
4 changes: 1 addition & 3 deletions stdlib/public/core/StringHashable.swift
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,8 @@ extension Unicode {
}
}

// FIXME: cannot be marked @_inlineable. See <rdar://problem/34438258>
// @_inlineable // FIXME(sil-serialize-all)
@_versioned // FIXME(sil-serialize-all)
@inline(never) @_semantics("stdlib_binary_only") // Hide the CF dependency
@inline(never) // Hide the CF dependency
internal func _hashString(_ string: String) -> Int {
let core = string._core
#if _runtime(_ObjC)
Expand Down
4 changes: 0 additions & 4 deletions stdlib/public/core/StringSwitch.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@

/// The compiler intrinsic which is called to lookup a string in a table
/// of static string case values.
@_inlineable // FIXME(sil-serialize-all)
@_semantics("stdlib_binary_only")
@_semantics("findStringSwitchCase")
public // COMPILER_INTRINSIC
func _findStringSwitchCase(
Expand Down Expand Up @@ -68,8 +66,6 @@ internal struct _StringSwitchContext {
/// in \p cache. Consecutive calls use the cache for faster lookup.
/// The \p cases array must not change between subsequent calls with the
/// same \p cache.
@_inlineable // FIXME(sil-serialize-all)
@_semantics("stdlib_binary_only")
@_semantics("findStringSwitchCaseWithCache")
public // COMPILER_INTRINSIC
func _findStringSwitchCaseWithCache(
Expand Down
9 changes: 6 additions & 3 deletions test/SILOptimizer/Inputs/linker_pass_input.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,26 @@
@_silgen_name("unknown")
public func unknown() -> ()

@_inlineable
public func doSomething() {
unknown()
}

@_semantics("stdlib_binary_only")
public func doSomething2() {
unknown()
}

@inline(never)
@_semantics("stdlib_binary_only")
public func doSomething3<T>(_ a:T) {
unknown()
}

struct A {}
@_versioned struct A {
@_versioned init() {}
}

@inline(never)
@_inlineable
Copy link
Contributor

Choose a reason for hiding this comment

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

Wait, I thought @inline(never) and @_inlineable don't play well together, and already wanted to ask for a warning.

Copy link
Member

Choose a reason for hiding this comment

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

@_inlineable is not about inlining, so they are actually orthogonal.

Copy link
Contributor

Choose a reason for hiding this comment

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

But, @swiftix told me that @inline(__always) implies @_inlineable. I'm confused now...

Copy link
Contributor

Choose a reason for hiding this comment

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

@moiseev IIRC, the current rules in the compiler work in such a way that @inline(__always) and @_transparent implies @_inlineable.

@slavapestov That's how it works today, or?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@swiftix and @milseman are correct. I know it's a bit confusing but this is where we are at today:

  • @_transparent and @inline(__always) imply @_inlineable, but only if the function is public.
  • @_inlineable just marks the SIL function body for serialization, it's ignored by the optimizer.
  • @inline(never) is an optimizer directive and is orthogonal to @_inlineable.

public func callDoSomething3() {
doSomething3(A())
}
Loading