Skip to content

BuildDescription: Set output path for bc files #6611

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
May 26, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,17 @@ public final class ClangTargetBuildDescription {
// User arguments (from -Xcxx) should follow generated arguments to allow user overrides
args += self.buildParameters.flags.cxxCompilerFlags
}

// Enable the correct lto mode if requested.
switch self.buildParameters.linkTimeOptimizationMode {
case nil:
break
case .full:
args += ["-flto=full"]
case .thin:
args += ["-flto=thin"]
}

return args
}

Expand Down
59 changes: 43 additions & 16 deletions Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,19 @@ public final class SwiftTargetBuildDescription {
self.target.underlyingTarget.resources + self.pluginDerivedResources
}

/// The objects in this target.
/// The objects in this target, containing either machine code or bitcode
/// depending on the build parameters used.
public var objects: [AbsolutePath] {
get throws {
let relativePaths = self.target.sources.relativePaths + self.derivedSources.relativePaths + self
.pluginDerivedSources.relativePaths
return try relativePaths.map {
try AbsolutePath(validating: "\($0.pathString).o", relativeTo: tempsPath)
let relativeSources = self.target.sources.relativePaths
+ self.derivedSources.relativePaths
+ self.pluginDerivedSources.relativePaths
let ltoEnabled = self.buildParameters.linkTimeOptimizationMode != nil
let objectFileExtension = ltoEnabled ? "bc" : "o"
return try relativeSources.map {
try AbsolutePath(
validating: "\($0.pathString).\(objectFileExtension)",
relativeTo: self.tempsPath)
}
}
}
Expand Down Expand Up @@ -512,6 +518,16 @@ public final class SwiftTargetBuildDescription {
// // User arguments (from -Xcxx) should follow generated arguments to allow user overrides
// args += self.buildParameters.flags.cxxCompilerFlags.asSwiftcCXXCompilerFlags()

// Enable the correct lto mode if requested.
switch self.buildParameters.linkTimeOptimizationMode {
case nil:
break
case .full:
args += ["-lto=llvm-full"]
case .thin:
args += ["-lto=llvm-thin"]
}

// suppress warnings if the package is remote
if self.package.isRemote {
args += ["-suppress-warnings"]
Expand Down Expand Up @@ -680,6 +696,16 @@ public final class SwiftTargetBuildDescription {
// // User arguments (from -Xcxx) should follow generated arguments to allow user overrides
// result += self.buildParameters.flags.cxxCompilerFlags.asSwiftcCXXCompilerFlags()

// Enable the correct lto mode if requested.
switch self.buildParameters.linkTimeOptimizationMode {
case nil:
break
case .full:
result += ["-lto=llvm-full"]
case .thin:
result += ["-lto=llvm-thin"]
}

result += try self.macroArguments()
return result
}
Expand Down Expand Up @@ -729,14 +755,18 @@ public final class SwiftTargetBuildDescription {


// Write out the entries for each source file.
let sources = self.target.sources.paths + self.derivedSources.paths + self.pluginDerivedSources.paths
for (idx, source) in sources.enumerated() {
let object = try objects[idx]
let objectDir = object.parentDirectory
let sources = self.sources
let objects = try self.objects
let ltoEnabled = self.buildParameters.linkTimeOptimizationMode != nil
let objectKey = ltoEnabled ? "llvm-bc" : "object"

let sourceFileName = source.basenameWithoutExt
for idx in 0..<sources.count {
let source = sources[idx]
let object = objects[idx]

let swiftDepsPath = objectDir.appending(component: sourceFileName + ".swiftdeps")
let sourceFileName = source.basenameWithoutExt
let partialModulePath = self.tempsPath.appending(component: sourceFileName + "~partial.swiftmodule")
let swiftDepsPath = self.tempsPath.appending(component: sourceFileName + ".swiftdeps")

content +=
#"""
Expand All @@ -745,7 +775,7 @@ public final class SwiftTargetBuildDescription {
"""#

if !self.buildParameters.useWholeModuleOptimization {
let depsPath = objectDir.appending(component: sourceFileName + ".d")
let depsPath = self.tempsPath.appending(component: sourceFileName + ".d")
content +=
#"""
"dependencies": "\#(depsPath._nativePathString(escaped: true))",
Expand All @@ -754,12 +784,9 @@ public final class SwiftTargetBuildDescription {
// FIXME: Need to record this deps file for processing it later.
}


let partialModulePath = objectDir.appending(component: sourceFileName + "~partial.swiftmodule")

content +=
#"""
"object": "\#(object._nativePathString(escaped: true))",
"\#(objectKey)": "\#(object._nativePathString(escaped: true))",
"swiftmodule": "\#(partialModulePath._nativePathString(escaped: true))",
"swift-dependencies": "\#(swiftDepsPath._nativePathString(escaped: true))"
}\#((idx + 1) < sources.count ? "," : "")
Expand Down
15 changes: 15 additions & 0 deletions Sources/CoreCommands/Options.swift
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,13 @@ public struct BuildOptions: ParsableArguments {
)
public var testEntryPointPath: AbsolutePath?

/// The lto mode to use if any.
@Option(
name: .customLong("experimental-lto-mode"),
help: .hidden
)
public var linkTimeOptimizationMode: LinkTimeOptimizationMode?
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this use that enum directly instead of creating a proxy enum, as they seem identical to each other?

Suggested change
public var linkTimeOptimizationMode: LinkTimeOptimizationMode?
public var linkTimeOptimizationMode: BuildParameters.LinkTimeOptimizationMode?

Copy link
Member Author

Choose a reason for hiding this comment

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

@neonichu suggested discrete enums, I'm happy to go either way.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think they should be separate because one is for argument parsing and one is for the build. They just happen to be the same right now, but that's coincidental not inherent.


// @Flag works best when there is a default value present
// if true, false aren't enough and a third state is needed
// nil should not be the goto. Instead create an enum
Expand All @@ -484,6 +491,14 @@ public struct BuildOptions: ParsableArguments {
case warn
case error
}

/// See `BuildParameters.LinkTimeOptimizationMode` for details.
public enum LinkTimeOptimizationMode: String, Codable, ExpressibleByArgument {
/// See `BuildParameters.LinkTimeOptimizationMode.full` for details.
case full
/// See `BuildParameters.LinkTimeOptimizationMode.thin` for details.
case thin
}
}

public struct LinkerOptions: ParsableArguments {
Expand Down
14 changes: 13 additions & 1 deletion Sources/CoreCommands/SwiftTool.swift
Original file line number Diff line number Diff line change
Expand Up @@ -703,7 +703,8 @@ public final class SwiftTool {
testEntryPointPath: options.build.testEntryPointPath,
explicitTargetDependencyImportCheckingMode: options.build.explicitTargetDependencyImportCheck.modeParameter,
linkerDeadStrip: options.linker.linkerDeadStrip,
verboseOutput: self.logLevel <= .info
verboseOutput: self.logLevel <= .info,
linkTimeOptimizationMode: options.build.linkTimeOptimizationMode?.buildParameter
)
})
}()
Expand Down Expand Up @@ -966,6 +967,17 @@ extension BuildOptions.TargetDependencyImportCheckingMode {
}
}

extension BuildOptions.LinkTimeOptimizationMode {
fileprivate var buildParameter: BuildParameters.LinkTimeOptimizationMode? {
switch self {
case .full:
return .full
case .thin:
return .thin
}
}
}

extension Basics.Diagnostic {
public static func mutuallyExclusiveArgumentsError(arguments: [String]) -> Self {
.error(arguments.map { "'\($0)'" }.spm_localizedJoin(type: .conjunction) + " are mutually exclusive")
Expand Down
35 changes: 33 additions & 2 deletions Sources/SPMBuildCore/BuildParameters.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,32 @@ public struct BuildParameters: Encodable {
case auto
}

/// An optional intermodule optimization to run at link time.
///
/// When using Link Time Optimization (LTO for short) the swift and clang
/// compilers produce objects containing containing a higher level
/// representation of the program bitcode instead of machine code. The
/// linker combines these objects together performing additional
/// optimizations with visibility into each module/object, resulting in a
/// further optimized version of the executable.
///
/// Using LTO can have significant impact on compile times, however can be
/// used to dramatically reduce code-size in some cases.
///
/// Note: Bitcode objects and machine code objects can be linked together.
public enum LinkTimeOptimizationMode: String, Encodable {
/// The "standard" LTO mode designed to produce minimal code sign.
///
/// Full LTO can lead to large link times. Consider using thin LTO if
/// build time is more important than minimizing binary size.
case full
/// An LTO mode designed to scale better with input size.
///
/// Thin LTO typically results in faster link times than traditional LTO.
/// However, thin LTO may not result in binary as small as full LTO.
case thin
}

/// Represents the debugging strategy.
///
/// Swift binaries requires the swiftmodule files in order for lldb to work.
Expand Down Expand Up @@ -219,6 +245,8 @@ public struct BuildParameters: Encodable {

public var verboseOutput: Bool

public var linkTimeOptimizationMode: LinkTimeOptimizationMode?

public init(
dataPath: AbsolutePath,
configuration: BuildConfiguration,
Expand Down Expand Up @@ -247,7 +275,8 @@ public struct BuildParameters: Encodable {
explicitTargetDependencyImportCheckingMode: TargetDependencyImportCheckingMode = .none,
linkerDeadStrip: Bool = true,
colorizedOutput: Bool = false,
verboseOutput: Bool = false
verboseOutput: Bool = false,
linkTimeOptimizationMode: LinkTimeOptimizationMode? = nil
) throws {
let triple = try destinationTriple ?? .getHostTriple(usingSwiftCompiler: toolchain.swiftCompilerPath)

Expand Down Expand Up @@ -288,6 +317,7 @@ public struct BuildParameters: Encodable {
self.linkerDeadStrip = linkerDeadStrip
self.colorizedOutput = colorizedOutput
self.verboseOutput = verboseOutput
self.linkTimeOptimizationMode = linkTimeOptimizationMode
}

public func withDestination(_ destinationTriple: Triple) throws -> BuildParameters {
Expand Down Expand Up @@ -330,7 +360,8 @@ public struct BuildParameters: Encodable {
explicitTargetDependencyImportCheckingMode: self.explicitTargetDependencyImportCheckingMode,
linkerDeadStrip: self.linkerDeadStrip,
colorizedOutput: self.colorizedOutput,
verboseOutput: self.verboseOutput
verboseOutput: self.verboseOutput,
linkTimeOptimizationMode: self.linkTimeOptimizationMode
)
}

Expand Down
58 changes: 58 additions & 0 deletions Tests/BuildTests/BuildPlanTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -4690,4 +4690,62 @@ final class BuildPlanTests: XCTestCase {

XCTAssertMatch(try result.buildProduct(for: "exe").linkArguments(), ["-sanitize=\(expectedName)"])
}

func testBuildParameterLTOMode() throws {
let fileSystem = InMemoryFileSystem(emptyFiles:
"/Pkg/Sources/exe/main.swift",
"/Pkg/Sources/cLib/cLib.c",
"/Pkg/Sources/cLib/include/cLib.h"
)

let observability = ObservabilitySystem.makeForTesting()
let graph = try loadPackageGraph(
fileSystem: fileSystem,
manifests: [
Manifest.createRootManifest(
displayName: "Pkg",
path: "/Pkg",
targets: [
TargetDescription(name: "exe", dependencies: ["cLib"]),
TargetDescription(name: "cLib", dependencies: []),
]),
],
observabilityScope: observability.topScope
)
XCTAssertNoDiagnostics(observability.diagnostics)

let toolchain = try UserToolchain.default
let buildParameters = mockBuildParameters(
toolchain: toolchain,
linkTimeOptimizationMode: .full)
let result = try BuildPlanResult(plan: BuildPlan(
buildParameters: buildParameters,
graph: graph,
fileSystem: fileSystem,
observabilityScope: observability.topScope))
result.checkProductsCount(1)
result.checkTargetsCount(2)

// Compile C Target
let cLibCompileArguments = try result.target(for: "cLib").clangTarget().basicArguments(isCXX: false)
let cLibCompileArgumentsPattern: [StringPattern] = ["-flto=full"]
XCTAssertMatch(cLibCompileArguments, cLibCompileArgumentsPattern)

// Compile Swift Target
let exeCompileArguments = try result.target(for: "exe").swiftTarget().compileArguments()
let exeCompileArgumentsPattern: [StringPattern] = ["-lto=llvm-full"]
XCTAssertMatch(exeCompileArguments, exeCompileArgumentsPattern)

// Assert the objects built by the Swift Target are actually bitcode
// files, indicated by the "bc" extension.
let exeCompileObjects = try result.target(for: "exe").swiftTarget().objects
XCTAssert(exeCompileObjects.allSatisfy { $0.extension == "bc"})

// Assert the objects getting linked contain all the bitcode objects
// built by the Swift Target
let exeProduct = try result.buildProduct(for: "exe")
for exeCompileObject in exeCompileObjects {
XCTAssertTrue(exeProduct.objects.contains(exeCompileObject))
}
}
}
6 changes: 4 additions & 2 deletions Tests/BuildTests/MockBuildTestHelper.swift
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ func mockBuildParameters(
destinationTriple: Basics.Triple = hostTriple,
indexStoreMode: BuildParameters.IndexStoreMode = .off,
useExplicitModuleBuild: Bool = false,
linkerDeadStrip: Bool = true
linkerDeadStrip: Bool = true,
linkTimeOptimizationMode: BuildParameters.LinkTimeOptimizationMode? = nil
) -> BuildParameters {
return try! BuildParameters(
dataPath: buildPath,
Expand All @@ -87,7 +88,8 @@ func mockBuildParameters(
canRenameEntrypointFunctionName: canRenameEntrypointFunctionName,
indexStoreMode: indexStoreMode,
useExplicitModuleBuild: useExplicitModuleBuild,
linkerDeadStrip: linkerDeadStrip
linkerDeadStrip: linkerDeadStrip,
linkTimeOptimizationMode: linkTimeOptimizationMode
)
}

Expand Down