Skip to content

Discard swift.ld in favor of portable solution and support gold linker in Swift #1157

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 2 commits into from
Feb 18, 2016
Merged

Discard swift.ld in favor of portable solution and support gold linker in Swift #1157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f889bcc
Implement -use-ld= switch in Swift
tienex Jan 19, 2016
d0d9b1d
Discard swift.ld and support gold linker
hpux735 Jan 20, 2016
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
2 changes: 1 addition & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ if("${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}")
endif()

if(SWIFT_BUILT_STANDALONE)
project(Swift)
project(Swift C CXX ASM)
endif()

if("${CMAKE_SYSTEM_NAME}" STREQUAL "")
Expand Down
63 changes: 34 additions & 29 deletions cmake/modules/AddSwift.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,15 +173,9 @@ function(_add_variant_link_flags
result)

if("${sdk}" STREQUAL "LINUX")
if("${arch}" STREQUAL "armv7")
list(APPEND result "-lpthread" "-ldl" "-Wl,-Bsymbolic")
elseif("${arch}" STREQUAL "armv6")
list(APPEND result "-lpthread" "-ldl" "-Wl,-Bsymbolic")
else()
list(APPEND result "-lpthread" "-ldl")
endif()
list(APPEND result "-lpthread" "-ldl")
elseif("${sdk}" STREQUAL "FREEBSD")
list(APPEND result "-lpthread" "-Wl,-Bsymbolic")
list(APPEND result "-lpthread")
else()
list(APPEND result "-lobjc")
endif()
Expand Down Expand Up @@ -856,6 +850,22 @@ function(_add_swift_library_single target name)
set(SWIFTLIB_SINGLE_API_NOTES "${module_name}")
endif()

# On platforms that use ELF binaries (for now that is Linux and FreeBSD)
# we add markers for metadata sections in the shared libraries using
# these object files. This wouldn't be necessary if the link was done by
# the swift binary: rdar://problem/19007002
if("${CMAKE_SYSTEM_NAME}" STREQUAL "Linux" OR
"${CMAKE_SYSTEM_NAME}" STREQUAL "FreeBSD")

if("${libkind}" STREQUAL "SHARED")
set(arch_subdir "${SWIFTLIB_DIR}/${SWIFTLIB_SINGLE_SUBDIR}")

set(SWIFT_SECTIONS_OBJECT_BEGIN "${arch_subdir}/swift_begin.o")
set(SWIFT_SECTIONS_OBJECT_END "${arch_subdir}/swift_end.o")
endif()

endif()

# FIXME: don't actually depend on the libraries in SWIFTLIB_SINGLE_LINK_LIBRARIES,
# just any swiftmodule files that are associated with them.
handle_swift_sources(
Expand All @@ -877,8 +887,21 @@ function(_add_swift_library_single target name)
INSTALL_IN_COMPONENT "${SWIFTLIB_INSTALL_IN_COMPONENT}")

add_library("${target}" ${libkind}
Copy link
Contributor

@gribozavr gribozavr Feb 2, 2016

Choose a reason for hiding this comment

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

Would this archive these object files into static archive versions of the libraries? It seems like we don't want that.

Copy link
Contributor

@tienex tienex Feb 2, 2016

Choose a reason for hiding this comment

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

The value of SWIFT_OBJECT_{BEGIN,END} is assigned only when libkind is SHARED, so they should not be present when generating static libraries targets.

${SWIFT_SECTIONS_OBJECT_BEGIN}
${SWIFTLIB_SINGLE_SOURCES}
${SWIFTLIB_SINGLE_EXTERNAL_SOURCES})
${SWIFTLIB_SINGLE_EXTERNAL_SOURCES}
${SWIFT_SECTIONS_OBJECT_END})

# The section metadata objects are generated sources, and we need to tell CMake
# not to expect to find them prior to their generation.
if("${CMAKE_SYSTEM_NAME}" STREQUAL "Linux" OR
"${CMAKE_SYSTEM_NAME}" STREQUAL "FreeBSD")
if("${libkind}" STREQUAL "SHARED")
set_source_files_properties(${SWIFT_SECTIONS_OBJECT_BEGIN} PROPERTIES GENERATED 1)
set_source_files_properties(${SWIFT_SECTIONS_OBJECT_END} PROPERTIES GENERATED 1)
add_dependencies("${target}" section_magic)
endif()
endif()

if (dtrace_dependency_targets)
add_dependencies("${target}" ${dtrace_dependency_targets})
Expand Down Expand Up @@ -1089,16 +1112,8 @@ function(_add_swift_library_single target name)
"${analyze_code_coverage}"
link_flags)

# Handle gold linker flags for shared libraries.
if(SWIFT_ENABLE_GOLD_LINKER AND SWIFTLIB_SINGLE_SHARED)
if("${SWIFTLIB_SINGLE_SDK}" STREQUAL "LINUX")
# Extend the link_flags for the gold linker so long as this
# isn't the standard library. The standard library uses a
# linker script that isn't supported by the gold linker.
if(NOT SWIFTLIB_SINGLE_IS_STDLIB)
list(APPEND link_flags "-fuse-ld=gold")
endif()
endif()
if(SWIFT_ENABLE_GOLD_LINKER)
list(APPEND link_flags "-fuse-ld=gold")
endif()

# Configure plist creation for OS X.
Expand Down Expand Up @@ -1133,16 +1148,6 @@ function(_add_swift_library_single target name)
set(PLIST_INFO_BUILD_VERSION)
endif()

# On Linux and FreeBSD add the linker script that coalesces protocol
# conformance sections. This wouldn't be necessary if the link was done by
# the swift binary: rdar://problem/19007002
if("${CMAKE_SYSTEM_NAME}" STREQUAL "Linux" OR
"${CMAKE_SYSTEM_NAME}" STREQUAL "FreeBSD")
list(APPEND link_flags
"-Xlinker" "-T"
"-Xlinker" "${SWIFTLIB_DIR}/${SWIFTLIB_SINGLE_SUBDIR}/swift.ld")
endif()

# Convert variables to space-separated strings.
_list_escape_for_shell("${c_compile_flags}" c_compile_flags)
_list_escape_for_shell("${link_flags}" link_flags)
Expand Down
4 changes: 4 additions & 0 deletions include/swift/Option/Options.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,10 @@ def no_link_objc_runtime : Flag<["-"], "no-link-objc-runtime">,
Flags<[HelpHidden, DoesNotAffectIncrementalBuild]>,
HelpText<"Don't link in additions to the Objective-C runtime">;

def use_ld : Joined<["-"], "use-ld=">,
Flags<[DoesNotAffectIncrementalBuild]>,
HelpText<"Specifies the linker to be used">;

def Xlinker : Separate<["-"], "Xlinker">,
Flags<[DoesNotAffectIncrementalBuild]>,
HelpText<"Specifies an option which should be passed to the linker">;
Expand Down
19 changes: 1 addition & 18 deletions lib/Driver/Driver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2009,24 +2009,7 @@ void Driver::printHelp(bool ShowHidden) const {
}

static llvm::Triple computeTargetTriple(StringRef DefaultTargetTriple) {
llvm::Triple triple = llvm::Triple(DefaultTargetTriple);

// armv6l and armv7l (which come from linux) are mapped to armv6 and
// armv7 (respectively) within llvm. When a Triple is created by llvm,
// the string is preserved, which keeps the 'l'. This extra character
// causes problems later down the line.
// By explicitly setting the architecture to the subtype that it aliases to,
// we remove that extra character while not introducing other side effects.
if (triple.getOS() == llvm::Triple::Linux) {
if (triple.getSubArch() == llvm::Triple::SubArchType::ARMSubArch_v7) {
triple.setArchName("armv7");
}
if (triple.getSubArch() == llvm::Triple::SubArchType::ARMSubArch_v6) {
triple.setArchName("armv6");
}
}

return triple;
return llvm::Triple(DefaultTargetTriple);
}

const ToolChain *Driver::getToolChain(const ArgList &Args) const {
Expand Down
84 changes: 64 additions & 20 deletions lib/Driver/ToolChains.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1091,6 +1091,28 @@ toolchains::GenericUnix::constructInvocation(const AutolinkExtractJobAction &job
return {"swift-autolink-extract", Arguments};
}

// This function maps triples to the architecture component of the path
// where the swift_begin.o and swift_end.o objects can be found. This
// is a stop-gap until full Triple support (ala Clang) exists within swiftc.
StringRef
getSectionMagicArch(const llvm::Triple &Triple) {
if (Triple.isOSLinux()) {
switch(Triple.getSubArch()) {
default:
return Triple.getArchName();
break;
case llvm::Triple::SubArchType::ARMSubArch_v7:
return "armv7";
break;
case llvm::Triple::SubArchType::ARMSubArch_v6:
return "armv6";
break;
}
} else {
return Triple.getArchName();
}
}

ToolChain::InvocationInfo
toolchains::GenericUnix::constructInvocation(const LinkJobAction &job,
const JobContext &context) const {
Expand All @@ -1109,25 +1131,32 @@ toolchains::GenericUnix::constructInvocation(const LinkJobAction &job,
break;
case LinkKind::DynamicLibrary:
Arguments.push_back("-shared");
if (getTriple().getOS() == llvm::Triple::Linux) {
if (getTriple().getSubArch() == llvm::Triple::SubArchType::ARMSubArch_v7 ||
getTriple().getSubArch() == llvm::Triple::SubArchType::ARMSubArch_v6) {
Arguments.push_back("-Wl,-Bsymbolic");
}
}
break;
}

addPrimaryInputsOfType(Arguments, context.Inputs, types::TY_Object);
addInputsOfType(Arguments, context.InputActions, types::TY_Object);
// Select the linker to use
StringRef Linker;

context.Args.AddAllArgs(Arguments, options::OPT_Xlinker);
context.Args.AddAllArgs(Arguments, options::OPT_linker_option_Group);
context.Args.AddAllArgs(Arguments, options::OPT_F);
if (const Arg *A = context.Args.getLastArg(options::OPT_use_ld)) {
Linker = A->getValue();
} else {
switch(getTriple().getArch()) {
default:
break;
case llvm::Triple::arm:
case llvm::Triple::armeb:
case llvm::Triple::thumb:
case llvm::Triple::thumbeb:
// BFD linker has issues wrt relocation of the protocol conformance
// section on these targets, it also generates COPY relocations for
// final executables, as such, unless specified, we default to gold
// linker.
Linker = "gold";
}
}

if (!context.OI.SDKPath.empty()) {
Arguments.push_back("--sysroot");
Arguments.push_back(context.Args.MakeArgString(context.OI.SDKPath));
if (!Linker.empty()) {
Arguments.push_back(context.Args.MakeArgString("-fuse-ld=" + Linker));
}

// Add the runtime library link path, which is platform-specific and found
Expand All @@ -1147,9 +1176,27 @@ toolchains::GenericUnix::constructInvocation(const LinkJobAction &job,
}
llvm::sys::path::append(RuntimeLibPath,
getPlatformNameForTriple(getTriple()));

// On Linux and FreeBSD (really, ELF binaries) we need to add objects
// to provide markers and size for the metadata sections.
Arguments.push_back(context.Args.MakeArgString(
Twine(RuntimeLibPath) + "/" + getSectionMagicArch(getTriple()) + "/swift_begin.o"));
addPrimaryInputsOfType(Arguments, context.Inputs, types::TY_Object);
addInputsOfType(Arguments, context.InputActions, types::TY_Object);

context.Args.AddAllArgs(Arguments, options::OPT_Xlinker);
context.Args.AddAllArgs(Arguments, options::OPT_linker_option_Group);
context.Args.AddAllArgs(Arguments, options::OPT_F);

if (!context.OI.SDKPath.empty()) {
Arguments.push_back("--sysroot");
Arguments.push_back(context.Args.MakeArgString(context.OI.SDKPath));
}

Arguments.push_back("-L");
Arguments.push_back(context.Args.MakeArgString(RuntimeLibPath));

// Explicitly pass the target to the linker
Arguments.push_back(context.Args.MakeArgString("--target=" + getTriple().str()));

if (context.Args.hasArg(options::OPT_profile_generate)) {
Expand Down Expand Up @@ -1182,13 +1229,10 @@ toolchains::GenericUnix::constructInvocation(const LinkJobAction &job,
Twine("@") + OutputInfo.getPrimaryOutputFilename()));
}

// Add the linker script that coalesces protocol conformance sections.
Arguments.push_back("-Xlinker");
Arguments.push_back("-T");

// FIXME: This should also query the abi type (i.e. gnueabihf)
// It is important that swift_end.o be the last object on the link line
// therefore, it is included just before the output filename.
Arguments.push_back(context.Args.MakeArgString(
Twine(RuntimeLibPath) + "/" + getTriple().getArchName() + "/swift.ld"));
Twine(RuntimeLibPath) + "/" + getSectionMagicArch(getTriple()) + "/swift_end.o"));

// This should be the last option, for convenience in checking output.
Arguments.push_back("-o");
Expand Down
22 changes: 21 additions & 1 deletion lib/Frontend/CompilerInvocation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,27 @@ static void updateRuntimeLibraryPath(SearchPathOptions &SearchPathOpts,
llvm::sys::path::append(LibPath, getPlatformNameForTriple(Triple));
SearchPathOpts.RuntimeLibraryPath = LibPath.str();

llvm::sys::path::append(LibPath, Triple.getArchName());
// The linux provided triple for ARM contains a trailing 'l'
// denoting little-endian. This is not used in the path for
// libraries. LLVM matches these SubArchTypes to the generic
// ARMSubArch_v7 (for example) type. If that is the case,
// use the base of the architecture type in the library path.
if (Triple.isOSLinux()) {
Copy link
Contributor Author

@hpux735 hpux735 Feb 10, 2016

Choose a reason for hiding this comment

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

@jrose-apple Your comment got lost again in the I've pasted it here from my email:

I'm a little concerned about this function mutating Triple. It should probably be passed const.

LLVM does have logic to canonicalize a triple. If you do that early on in the Driver, does everything work?

And I think that's a good point. I refactored it a bit, and I think that this solution really gets at the essence of what I'm trying to achieve here. The goal is to get the library path using the architecture type. In many other places in the project, "armv7" and "armv6" are used for this path according to the architecture type (not the least of which are the build and install scripts). It probably makes sense to decouple the assumption that llvm uses the same string to achieve this, or the unnecessary setting of llvm::Triple's string to match. I think it makes more sense to ask llvm for the architecture we're using, then use that to select the explicit string that we're really looking for, and is hard-coded elsewhere.

With regard to the normalization logic, I did some experiments with that. It preserves the 'l' provided by linux, so it won't do what we want here.

Copy link
Contributor

@jrose-apple jrose-apple Feb 10, 2016

Choose a reason for hiding this comment

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

Thanks for looking into this further. This version looks good to me.

Copy link
Contributor

@jrose-apple jrose-apple Feb 10, 2016

Choose a reason for hiding this comment

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

…although another possible answer would be to actually use armv7l as the name of the standard library, in case you ever wanted it to coexist with an armv7b.

Copy link
Contributor Author

@hpux735 hpux735 Feb 10, 2016

Choose a reason for hiding this comment

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

Hmmm... interesting point. Thoughts @tienex ?

Copy link
Contributor

@tienex tienex Feb 10, 2016

Choose a reason for hiding this comment

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

I still think we'll need to normalize somehow... assuming we want it to behave like clang, we should know how to translate armv7- (with no endian notion) to armv7([bl])-, the same happens for thumbv7([bl])?- which should use armv7 libraries (which effectively should be the default on linux/armv7 being the preferred ISA).

Copy link
Contributor Author

@hpux735 hpux735 Feb 10, 2016

Choose a reason for hiding this comment

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

Because the fact that there are still many showstopper issues with big endian (which were brought up in the discussion of the c-based implementation of the conformance objects) it may not make sense to put the cart before the horse on this one. I doubt this would pose undue complication to a future effort supporting big-endian arm cores.

Copy link
Contributor

@jrose-apple jrose-apple Feb 12, 2016

Choose a reason for hiding this comment

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

👍

switch(Triple.getSubArch()) {
default:
llvm::sys::path::append(LibPath, Triple.getArchName());
break;
case llvm::Triple::SubArchType::ARMSubArch_v7:
llvm::sys::path::append(LibPath, "armv7");
break;
case llvm::Triple::SubArchType::ARMSubArch_v6:
llvm::sys::path::append(LibPath, "armv6");
break;
}
} else {
llvm::sys::path::append(LibPath, Triple.getArchName());
}

SearchPathOpts.RuntimeLibraryImportPath = LibPath.str();
}

Expand Down
1 change: 1 addition & 0 deletions stdlib/public/core/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ add_swift_library(swiftCore SHARED IS_STDLIB IS_STDLIB_CORE
# and the generated directory as dependencies.
FILE_DEPENDS
copy_shim_headers "${SWIFTLIB_DIR}/shims"
section_magic
LINK_FLAGS ${swift_core_link_flags}
PRIVATE_LINK_LIBRARIES ${swift_core_private_link_libraries}
FRAMEWORK_DEPENDS ${swift_core_framework_depends}
Expand Down
31 changes: 26 additions & 5 deletions stdlib/public/runtime/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ endif()

# Acknowledge that the following sources are known.
set(LLVM_OPTIONAL_SOURCES
Remangle.cpp)
Remangle.cpp
swift_sections.S)

set(swift_runtime_objc_sources)
set(swift_runtime_unicode_normalization_sources)
Expand Down Expand Up @@ -52,20 +53,40 @@ add_swift_library(swiftRuntime IS_STDLIB IS_STDLIB_CORE
C_COMPILE_FLAGS ${swift_runtime_compile_flags}
INSTALL_IN_COMPONENT stdlib)

set(object_target_list)
foreach(sdk ${SWIFT_CONFIGURED_SDKS})
if("${sdk}" STREQUAL "LINUX" OR "${sdk}" STREQUAL "FREEBSD")
foreach(arch ${SWIFT_SDK_${sdk}_ARCHITECTURES})
set(arch_subdir "${SWIFT_SDK_${sdk}_LIB_SUBDIR}/${arch}")

# FIXME: We will need a different linker script for 32-bit builds.
configure_file(
"swift.ld" "${SWIFTLIB_DIR}/${arch_subdir}/swift.ld" COPYONLY)
set(section_magic_begin_name "section_magic_begin_${SWIFT_SDK_${sdk}_ARCH_${arch}_TRIPLE}")
set(section_magic_end_name "section_magic_end_${SWIFT_SDK_${sdk}_ARCH_${arch}_TRIPLE}")

add_library(${section_magic_begin_name} STATIC swift_sections.S)
set_target_properties(${section_magic_begin_name} PROPERTIES COMPILE_FLAGS "-DSWIFT_BEGIN")

add_library(${section_magic_end_name} STATIC swift_sections.S)
set_target_properties(${section_magic_end_name} PROPERTIES COMPILE_FLAGS "-DSWIFT_END")

add_custom_command_target(${section_magic_begin_name}_begin
OUTPUT "${SWIFTLIB_DIR}/${arch_subdir}/swift_begin.o"
COMMAND "${CMAKE_COMMAND}" -E copy "${CMAKE_CURRENT_BINARY_DIR}/CMakeFiles/${section_magic_begin_name}.dir/swift_sections.S${CMAKE_C_OUTPUT_EXTENSION}" "${SWIFTLIB_DIR}/${arch_subdir}/swift_begin.o"
DEPENDS ${section_magic_begin_name})

add_custom_command_target(${section_magic_begin_name}_end
OUTPUT "${SWIFTLIB_DIR}/${arch_subdir}/swift_end.o"
COMMAND "${CMAKE_COMMAND}" -E copy "${CMAKE_CURRENT_BINARY_DIR}/CMakeFiles/${section_magic_end_name}.dir/swift_sections.S${CMAKE_C_OUTPUT_EXTENSION}" "${SWIFTLIB_DIR}/${arch_subdir}/swift_end.o"
DEPENDS ${section_magic_end_name})

list(APPEND object_target_list "${SWIFTLIB_DIR}/${arch_subdir}/swift_begin.o" "${SWIFTLIB_DIR}/${arch_subdir}/swift_end.o")

swift_install_in_component(compiler
FILES "swift.ld"
FILES "${SWIFTLIB_DIR}/${arch_subdir}/swift_begin.o" "${SWIFTLIB_DIR}/${arch_subdir}/swift_end.o"
DESTINATION "lib/swift/${arch_subdir}")

endforeach()
endif()
endforeach()

add_custom_target(section_magic ALL DEPENDS ${object_target_list})

20 changes: 0 additions & 20 deletions stdlib/public/runtime/swift.ld
View file
Open in desktop

This file was deleted.

Loading