Skip to content

optimize flag computation #31125

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 7 commits into from
May 1, 2020
Merged

optimize flag computation #31125

merged 7 commits into from
May 1, 2020

Conversation

compnerd
Copy link
Member

Replace this paragraph with a description of your changes and rationale. Provide links to external references/discussions if appropriate.

Resolves SR-NNNN.

@compnerd
Copy link
Member Author

CC: @drexin @gottesmm

@compnerd
Copy link
Member Author

@swift-ci please clean test

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - e033243e7955321e4c0fa396e2f65de636416ce5

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - e033243e7955321e4c0fa396e2f65de636416ce5

Apply constant value propagation to the host flag variant operations.
This reduces the parameters needed for the invocations and enables
further cleaning in the hopes that we can apply these per-target using
CMake instead.
Perform some constant folding, constant value propagation, dead argument
elimination over the flag computation methods.  This reduces the
unnecessary parameters and more clearly and succinctly describes what is
happening.
Convert the out parameters for `_add_host_variant_link_flags` to use the
target as an in-parameter.  Doing so allows us to set the properties on
the target directly rather than providing them as out parameters which
then get set subsequently.
Convert the out parameters for `_add_host_variant_c_compile_flags` to
use the target as an in-parameter.  Doing so allows us to set the
properties on the target directly rather than providing them as out
parameters which then get set subsequently.
The host tools do not use ICU, only the standard library does.  Remove
the special handling in the host tool path for ICU.  This simplifies the
flag computation and allows the ICU handling to be sunk entirely into
the target specific paths.
The two invocations here both had a single parameter passed to it.
Replace it with `add_dependencies` which already actually supports
multiple dependencies specified.  Sink the custom wrapper into the
usage location in `AddSwiftStdlib.cmake`.
Use a newly introduced `swift_gyb_target_sources` to gyb and use the
generated sources when building.  Let CMake figure out when to run the
command, let it invoke the command properly, and indicate that the
sources being added to the target are generated.
@compnerd
Copy link
Member Author

@swift-ci please clean test

@compnerd
Copy link
Member Author

@swift-ci please test windows platform

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - e6f2e62

@compnerd
Copy link
Member Author

@swift-ci please smoke test Linux platform

@compnerd
Copy link
Member Author

@swift-ci please test Linux platform

@drexin drexin merged commit 6290d8c into swiftlang:master May 1, 2020
@compnerd compnerd deleted the opt-flags branch May 1, 2020 17:35
@rintaro
Copy link
Member

rintaro commented May 7, 2020

edymtt added a commit to edymtt/swift that referenced this pull request May 14, 2020
Following swiftlang#31125 and swiftlang#31612, `-target` is not added automatically to
linker flags when that's needed (e.g. when building for Apple SDKs) --
mimic the logic used to add it for compiler flags.

Addresses rdar://63138761
edymtt added a commit that referenced this pull request May 18, 2020
Following #31125 and #31612, `-target` is not added automatically to
linker flags when that's needed (e.g. when building for Apple SDKs) --
mimic the logic used to add it for compiler flags.

Addresses rdar://63138761
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants