Skip to content

[Driver] Always link compiler_rt on Darwin #17843

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 4 commits into from
Jul 12, 2018
Merged

[Driver] Always link compiler_rt on Darwin #17843

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
64b3d88
[Driver] Always link compiler_rt on Darwin
jrose-apple Jul 9, 2018
d466883
[Driver] Only link to compiler_rt if present for the target platform
jrose-apple Jul 10, 2018
d11704b
Add swift::getNonSimulatorPlatform for DarwinPlatformKind
jrose-apple Jul 11, 2018
51f6e9a
[test] Tidy up newly-added clang_rt Driver tests to be a bit clearer
jrose-apple Jul 11, 2018
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
5 changes: 5 additions & 0 deletions include/swift/Basic/Platform.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@ namespace swift {
/// Returns the platform Kind for Darwin triples.
DarwinPlatformKind getDarwinPlatformKind(const llvm::Triple &triple);

/// Maps an arbitrary platform to its non-simulator equivalent.
///
/// If \p platform is not a simulator platform, it will be returned as is.
DarwinPlatformKind getNonSimulatorPlatform(DarwinPlatformKind platform);

/// Returns the architecture component of the path for a given target triple.
///
/// Typically this is used for mapping the architecture component of the
Expand Down
16 changes: 16 additions & 0 deletions lib/Basic/Platform.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,22 @@ DarwinPlatformKind swift::getDarwinPlatformKind(const llvm::Triple &triple) {
llvm_unreachable("Unsupported Darwin platform");
}

DarwinPlatformKind swift::getNonSimulatorPlatform(DarwinPlatformKind platform) {
switch (platform) {
case DarwinPlatformKind::MacOS:
return DarwinPlatformKind::MacOS;
case DarwinPlatformKind::IPhoneOS:
case DarwinPlatformKind::IPhoneOSSimulator:
return DarwinPlatformKind::IPhoneOS;
case DarwinPlatformKind::TvOS:
case DarwinPlatformKind::TvOSSimulator:
return DarwinPlatformKind::TvOS;
case DarwinPlatformKind::WatchOS:
case DarwinPlatformKind::WatchOSSimulator:
return DarwinPlatformKind::WatchOS;
}
}

static StringRef getPlatformNameForDarwin(const DarwinPlatformKind platform) {
switch (platform) {
case DarwinPlatformKind::MacOS:
Expand Down
30 changes: 24 additions & 6 deletions lib/Driver/DarwinToolChains.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,12 @@ toolchains::Darwin::constructInvocation(const InterpretJobAction &job,
}

static StringRef
getDarwinLibraryNameSuffixForTriple(const llvm::Triple &triple) {
switch (getDarwinPlatformKind(triple)) {
getDarwinLibraryNameSuffixForTriple(const llvm::Triple &triple,
bool distinguishSimulator = true) {
const DarwinPlatformKind kind = getDarwinPlatformKind(triple);
const DarwinPlatformKind effectiveKind =
distinguishSimulator ? kind : getNonSimulatorPlatform(kind);
switch (effectiveKind) {
case DarwinPlatformKind::MacOS:
return "osx";
case DarwinPlatformKind::IPhoneOS:
Expand Down Expand Up @@ -262,7 +266,22 @@ toolchains::Darwin::constructInvocation(const LinkJobAction &job,
assert(Triple.isOSDarwin());

// FIXME: If we used Clang as a linker instead of going straight to ld,
// we wouldn't have to replicate Clang's logic here.
// we wouldn't have to replicate a bunch of Clang's logic here.

// Always link the regular compiler_rt if it's present.
//
// Note: Normally we'd just add this unconditionally, but it's valid to build
// Swift and use it as a linker without building compiler_rt.
SmallString<128> CompilerRTPath;
getClangLibraryPath(context.Args, CompilerRTPath);
llvm::sys::path::append(
CompilerRTPath,
Twine("libclang_rt.") +
getDarwinLibraryNameSuffixForTriple(Triple, /*simulator*/false) +
".a");
if (llvm::sys::fs::exists(CompilerRTPath))
Arguments.push_back(context.Args.MakeArgString(CompilerRTPath));

Copy link
Contributor

Choose a reason for hiding this comment

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

The routine this is in, constructInvocation is already too long for me to understand without breaking it down. At the least, it would be great to factor this paragraph out into a separate function. At best, in order to ease the way for others to work on this code, it would help a lot to break this constructInvocation function down into smaller pieces.

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 definitely don't want to do that in a cherry-pick to 4.2, and I don't want to do it without a guiding principle for grouping arguments, which I don't think we have today.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh, yes, I understand about cherry-picking. Sigh. Will check the other box.

bool wantsObjCRuntime = false;
if (Triple.isiOS())
wantsObjCRuntime = Triple.isOSVersionLT(9);
Expand Down Expand Up @@ -375,9 +394,8 @@ toolchains::Darwin::constructInvocation(const LinkJobAction &job,
}

if (context.Args.hasArg(options::OPT_profile_generate)) {
SmallString<128> LibProfile(RuntimeLibPath);
llvm::sys::path::remove_filename(LibProfile); // remove platform name
llvm::sys::path::append(LibProfile, "clang", "lib", "darwin");
SmallString<128> LibProfile;
getClangLibraryPath(context.Args, LibProfile);

StringRef RT;
if (Triple.isiOS()) {
Expand Down
38 changes: 38 additions & 0 deletions test/Driver/linker-clang_rt.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Make sure that the platform-appropriate clang_rt library (found relative to
// the compiler) is included when using Swift as a linker (with Apple targets).

// We use hard links to make sure the Swift driver really thinks it's been
// moved.

// RUN: rm -rf %t
// RUN: %empty-directory(%t/bin)
// RUN: %hardlink-or-copy(from: %swift_driver_plain, to: %t/bin/swiftc)
// RUN: %empty-directory(%t/lib/clang/darwin/)

// RUN: %t/bin/swiftc -driver-print-jobs -target x86_64-apple-macosx10.9 %S/../Inputs/empty.swift | %FileCheck -check-prefix CHECK -check-prefix CHECK-NO-RUNTIME %s

// RUN: touch %t/lib/clang/darwin/libclang_rt.osx.a %t/lib/clang/darwin/libclang_rt.ios.a %t/lib/clang/darwin/libclang_rt.tvos.a %t/lib/clang/darwin/libclang_rt.watchos.a

// RUN: %t/bin/swiftc -driver-print-jobs -target x86_64-apple-macosx10.9 %S/../Inputs/empty.swift | %FileCheck -check-prefix CHECK -check-prefix MACOS %s

// RUN: %t/bin/swiftc -driver-print-jobs -target i386-apple-ios7 %S/../Inputs/empty.swift | %FileCheck -check-prefix CHECK -check-prefix IOS %s
// RUN: %t/bin/swiftc -driver-print-jobs -target x86_64-apple-ios7 %S/../Inputs/empty.swift | %FileCheck -check-prefix CHECK -check-prefix IOS %s
// RUN: %t/bin/swiftc -driver-print-jobs -target armv7s-apple-ios7 %S/../Inputs/empty.swift | %FileCheck -check-prefix CHECK -check-prefix IOS %s
// RUN: %t/bin/swiftc -driver-print-jobs -target arm64-apple-ios7 %S/../Inputs/empty.swift | %FileCheck -check-prefix CHECK -check-prefix IOS %s

// RUN: %t/bin/swiftc -driver-print-jobs -target x86_64-apple-ios9 %S/../Inputs/empty.swift | %FileCheck -check-prefix CHECK -check-prefix TVOS %s
// RUN: %t/bin/swiftc -driver-print-jobs -target arm64-apple-tvos9 %S/../Inputs/empty.swift | %FileCheck -check-prefix CHECK -check-prefix TVOS %s

// RUN: %t/bin/swiftc -driver-print-jobs -target i386-apple-watchos2 %S/../Inputs/empty.swift | %FileCheck -check-prefix CHECK -check-prefix WATCHOS %s
// RUN: %t/bin/swiftc -driver-print-jobs -target armv7k-apple-watchos2 %S/../Inputs/empty.swift | %FileCheck -check-prefix CHECK -check-prefix WATCHOS %s

// Clean up the test executable because hard links are expensive.
// RUN: rm -f %t/bin/swiftc

// CHECK: bin/ld{{"? }}
// CHECK-NO-RUNTIME-NOT: libclang_rt
// CHECK-MACOS-SAME: {{[^ ]+/lib/clang/darwin/libclang_rt.osx.a}}
// CHECK-IOS-SAME: {{[^ ]+/lib/clang/darwin/libclang_rt.ios.a}}
// CHECK-TVOS-SAME: {{[^ ]+/lib/clang/darwin/libclang_rt.tvos.a}}
// CHECK-WATCHOS-SAME: {{[^ ]+/lib/clang/darwin/libclang_rt.watchos.a}}
// CHECK-SAME: -o {{[^ ]+}}
10 changes: 5 additions & 5 deletions test/Driver/linker.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@

// SIMPLE: bin/ld{{"? }}
// SIMPLE-NOT: -syslibroot
// SIMPLE-DAG: -macosx_version_min 10.{{[0-9]+}}.{{[0-9]+}}
// SIMPLE: -macosx_version_min 10.{{[0-9]+}}.{{[0-9]+}}
// SIMPLE-NOT: -syslibroot
// SIMPLE: -o linker

Expand Down Expand Up @@ -312,11 +312,11 @@


// FILELIST: bin/ld{{"? }}
// FILELIST-NOT: .o
// FILELIST-NOT: .o{{"? }}
// FILELIST: -filelist {{"?[^-]}}
// FILELIST-NOT: .o
// FILELIST: /a.o
// FILELIST-NOT: .o
// FILELIST-NOT: .o{{"? }}
// FILELIST: /a.o{{"? }}
// FILELIST-NOT: .o{{"? }}
// FILELIST: -o linker


Expand Down
16 changes: 14 additions & 2 deletions test/lit.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -945,8 +945,9 @@ def source_compiler_rt_libs(path):
if lib.startswith('libclang_rt.')
and config.compiler_rt_platform in lib])

source_compiler_rt_libs(make_path(test_resource_dir, 'clang', 'lib',
platform.system().lower()))
compiler_rt_dir = make_path(test_resource_dir, 'clang', 'lib',
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be possible to add a comment to explain the why here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Explain what why?

Copy link
Contributor

Choose a reason for hiding this comment

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

LOL! I need to add a comment to explain my request for a comment!
My thought was to add comments to this file to explain what is going on w.r.t. the new feature being added. (I know nothing about lit.cfg files and thought maybe there were breadcrumbs you could add.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This feels like another case where there's not a reasonable comment. This part didn't change, and the part below builds a path and tests if it exists.

platform.system().lower())
source_compiler_rt_libs(compiler_rt_dir)

def check_runtime_libs(features_to_check):
for lib in config.compiler_rt_libs:
Expand All @@ -967,6 +968,17 @@ if run_ptrsize != "32":

check_runtime_libs(runtime_libs)

# From https://stackoverflow.com/a/2393022
def strip_right(text, suffix):
if not text.endswith(suffix):
return text
return text[:len(text)-len(suffix)]

base_runtime_lib_name = (
'libclang_rt.' + strip_right(config.compiler_rt_platform, 'sim') + '.a')
if os.path.exists(make_path(compiler_rt_dir, base_runtime_lib_name)):
config.available_features.add('c_runtime')

if not getattr(config, 'target_run_simple_swift', None):
config.target_run_simple_swift = (
'%%empty-directory(%%t) && '
Expand Down
9 changes: 9 additions & 0 deletions validation-test/Driver/Inputs/clang_rt-helper.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#include <stdbool.h>

static inline bool isRunningOnFairlyRecentOS() {
if (__builtin_available(macOS 10.12, iOS 10, tvOS 10, watchOS 3, *)) {
return true;
} else {
return false;
}
}
11 changes: 11 additions & 0 deletions validation-test/Driver/clang_rt.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// RUN: %target-build-swift -emit-executable %s -import-objc-header %S/Inputs/clang_rt-helper.h -o %t

// REQUIRES: c_runtime

// Just make sure we can build and link successfully.

if isRunningOnFairlyRecentOS() {
print("new!")
} else {
print("old...")
}