Stabilize the ABI of libswiftSwiftOnoneSupport #23518

eeckstein · 2019-03-23T16:26:39Z

We have to make sure that the ABI of libswiftSwiftOnoneSupport does not change compared to the swift 5.0 version, i.e. excecutables, compiled with -Onone, must be linkable to older and newer versions of libswiftSwiftOnoneSupport.
This PR tries to fix this with a brute-force approach:

backward deployment: Hard-code the list of available symbols and, when compiling a -Onone executable, only use presepcializations from this list.
forward deployment: Explicitly force pre-specialization of library symbols when compiling SwiftOnoneSupport, with the help of a new @_semantics("prespecialize.") attribute. Also add a check at the end of the SIL compilation pipeline, which lets the compiler issue and error if not all required symbols are prespecialized (when compiling SwiftOnoneSupport).

This fix is not perfect but it should work for now. We might want to replace it by something nicer in the future, e.g. specialize-attributes on library functions.

rdar://problem/48924409

eeckstein · 2019-03-23T16:27:35Z

@swift-ci test

swift-ci · 2019-03-23T17:03:04Z

Build failed
Swift Test Linux Platform
Git Sha - c0fafaff26042cb63d75fd58193aef884433b814

swift-ci · 2019-03-23T18:02:28Z

Build failed
Swift Test OS X Platform
Git Sha - c0fafaff26042cb63d75fd58193aef884433b814

eeckstein · 2019-03-25T19:36:56Z

@swift-ci test

eeckstein · 2019-03-25T19:37:18Z

@swift-ci test

bob-wilson

I have a few minor comments. Beyond those, I don't see the connection between this change and adding the demangler -strip-specialization option. What's the connection there?

bob-wilson · 2019-03-25T20:00:12Z

include/swift/Option/FrontendOptions.td

@@ -416,6 +416,9 @@ def import_module : Separate<["-"], "import-module">,
 def print_stats : Flag<["-"], "print-stats">,
  HelpText<"Print various statistics">;

+def check_onone_completeness : Flag<["-"], "check-onone-completness">,


There's a typo here: missing an "e" in "check-onone-completeness"

bob-wilson · 2019-03-25T20:06:27Z

stdlib/public/SwiftOnoneSupport/CMakeLists.txt

@@ -6,7 +6,7 @@ add_swift_target_library(swiftSwiftOnoneSupport ${SWIFT_STDLIB_LIBRARY_BUILD_TYP
  # _explicitly_ special-cased to result in extra symbols generated by the
  # optimizer, meaning TBDGen can't (and shouldn't: it has to run
  # pre-optimization for performance) list them.
-  SWIFT_COMPILE_FLAGS "-parse-stdlib" "-Xllvm" "-sil-inline-generics=false" "-Xfrontend" "-validate-tbd-against-ir=none" "${SWIFT_RUNTIME_SWIFT_COMPILE_FLAGS}"
+  SWIFT_COMPILE_FLAGS "-parse-stdlib" "-Xllvm" "-sil-inline-generics=false" "-Xfrontend" "-validate-tbd-against-ir=none" "-Xfrontend" "-check-onone-completness" "${SWIFT_RUNTIME_SWIFT_COMPILE_FLAGS}"


....and this use of the new option has the same typo as the definition

bob-wilson · 2019-03-25T20:07:45Z

lib/SILOptimizer/Utils/Generics.cpp

@@ -2287,6 +2291,69 @@ SILArgument *ReabstractionThunkGenerator::convertReabstractionThunkArguments(
  return ReturnValueAddr;
 }

+/// Create a pre-specializations of the library function with


"pre-specializations" should be singular in this comment

swift-ci · 2019-03-25T20:16:42Z

Build failed
Swift Test Linux Platform
Git Sha - aa7c4187fe2cdffefc26209b14f485b2e3bf431a

… compiler. Add the list of symbols from the swift-5.0 release and only use that symbols to refer to pre-specializations in a Onone build. This ensures that future Onone executables can link to an old swift 5.0 libswiftSwiftOnoneSupport library. rdar://problem/48924409

…ls for OnoneSupport. When compiling the OnoneSupport library, the compiler checks for @_semantics("prespecialize.X") attributes to pre-specialize function X. rdar://problem/48924409

…oneSupport It's very unlikely that those functions are really referenced by Onone executables, but we want to be ABI compatible. Also, there is no guarantee that this version of SwiftOnoneSupport will always trigger all symbol to be pre-specialized. If the optimizer changes, we might need to add other explicit pre-specializations of internal stdlib symbols. rdar://problem/48924409

…l name of the origin of a specialized function.

When compiling SwiftOnoneSupport, issue errors for missing functions which are expected in the module. This ensures ABI compatibility. rdar://problem/48924409

eeckstein · 2019-03-25T22:58:27Z

@bob-wilson Thanks, I fixed the typos. Regarding -strip-specialization: I needed that for the work on this PR. The change is in a separate commit anyway.

eeckstein · 2019-03-25T22:58:34Z

@swift-ci test

eeckstein · 2019-03-25T22:58:42Z

@swift-ci test

swift-ci · 2019-03-25T23:02:20Z

Build failed
Swift Test OS X Platform
Git Sha - aa7c4187fe2cdffefc26209b14f485b2e3bf431a

eeckstein · 2019-03-26T01:40:28Z

@swift-ci test macOS

swift-ci · 2019-03-26T02:39:13Z

Build failed
Swift Test OS X Platform
Git Sha - 00599ee

rintaro · 2019-03-26T03:51:50Z

@swift-ci test macOS

slavapestov

This looks good, thanks for the quick fix. I only have a couple of questions to understand the new code better.

slavapestov · 2019-03-26T18:21:50Z

stdlib/public/SwiftOnoneSupport/SwiftOnoneSupport.swift

@@ -67,6 +132,14 @@ internal enum _Prespecialize {
      // force specialization of append.
      a.append(a[0])

+      #if _runtime(_ObjC)


Why is this specific to ObjC?

Because we compile different code without ObjC, so the symbols in the stdlib core are different.

slavapestov · 2019-03-26T18:26:22Z

stdlib/public/SwiftOnoneSupport/SwiftOnoneSupport.swift

@@ -67,6 +132,14 @@ internal enum _Prespecialize {
      // force specialization of append.
      a.append(a[0])

+      #if _runtime(_ObjC)
+      // Explicitly specialize some private functions.


Which ones are actually private? We should definitely not be emitting public pre-specializations of any; I would suggest removing any private functions from the list and only adding them back if we know we broke something.

However looking at a few, eg the _ArrayBuffer members, I don't think they're actually private at the SIL level. _ArrayBuffer is @usableFromInline internal and it has a bunch of @inlinable internal members. This means that those members are not visible in the source language but can be linked against publicly from SIL that has been deserialized into the client binary. So they're emitted with public linkage in the stdlib.

Do we know why we need special code to ensure that they're pre-specialized here?

You are right, theses are not real "private" symbols, but @inlinable internal. Otherwise we wouldn't be able to specialize them.

We eagerly specialize a whole call tree, starting from a public function (referenced from the SwiftOnoneSupport.swift code). But because we compile the stdlib with generic inlining, it's not guaranteed that we see the complete (not-inlined) original call tree when specializing.

It's still not entirely clear to me why we need two lists of symbols in two different places though. Perhaps we can just resolve this if you update the comment with your explanation, making it clear that we're not actually specializing private symbols (which would impact future library evolution).

ok, I'll update the comment.

eeckstein requested review from slavapestov and bob-wilson March 23, 2019 16:26

eeckstein force-pushed the onone-support-abi branch from c0fafaf to aa7c418 Compare March 25, 2019 19:36

bob-wilson requested changes Mar 25, 2019

View reviewed changes

eeckstein added 5 commits March 25, 2019 14:57

SILOptimizer: add a mechanism to pre-specialize internal stdlib symbo…

17aa1c0

…ls for OnoneSupport. When compiling the OnoneSupport library, the compiler checks for @_semantics("prespecialize.X") attributes to pre-specialize function X. rdar://problem/48924409

swift-demangler: Add an option -strip-specialization to get the symbo…

a8a18c0

…l name of the origin of a specialized function.

Check for completeness of the SwiftOnoneSupport library

00599ee

When compiling SwiftOnoneSupport, issue errors for missing functions which are expected in the module. This ensures ABI compatibility. rdar://problem/48924409

eeckstein force-pushed the onone-support-abi branch from aa7c418 to 00599ee Compare March 25, 2019 22:56

eeckstein requested a review from bob-wilson March 25, 2019 22:57

bob-wilson approved these changes Mar 26, 2019

View reviewed changes

eeckstein merged commit ba5e344 into swiftlang:master Mar 26, 2019

eeckstein deleted the onone-support-abi branch March 26, 2019 18:18

slavapestov reviewed Mar 26, 2019

View reviewed changes

Stabilize the ABI of libswiftSwiftOnoneSupport #23518

Stabilize the ABI of libswiftSwiftOnoneSupport #23518

Uh oh!

Conversation

eeckstein commented Mar 23, 2019

Uh oh!

eeckstein commented Mar 23, 2019

Uh oh!

swift-ci commented Mar 23, 2019

Uh oh!

swift-ci commented Mar 23, 2019

Uh oh!

eeckstein commented Mar 25, 2019

Uh oh!

eeckstein commented Mar 25, 2019

Uh oh!

bob-wilson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

swift-ci commented Mar 25, 2019

Uh oh!

eeckstein commented Mar 25, 2019

Uh oh!

eeckstein commented Mar 25, 2019

Uh oh!

eeckstein commented Mar 25, 2019

Uh oh!

swift-ci commented Mar 25, 2019

Uh oh!

eeckstein commented Mar 26, 2019

Uh oh!

swift-ci commented Mar 26, 2019

Uh oh!

rintaro commented Mar 26, 2019

Uh oh!

slavapestov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!