diff options
4 files changed, 68 insertions, 36 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java index b68e2e3c5f..b21536e797 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java @@ -89,7 +89,8 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { NestedSet<Artifact> filesToBuild, Iterable<Artifact> fakeLinkerInputs, boolean fake, - ImmutableSet<CppSource> cAndCppSources) { + ImmutableSet<CppSource> cAndCppSources, + boolean linkCompileOutputSeparately) { Runfiles.Builder builder = new Runfiles.Builder( context.getWorkspaceName(), context.getConfiguration().legacyExternalRunfiles()); Function<TransitiveInfoCollection, Runfiles> runfilesMapping = @@ -104,12 +105,11 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { // Add the C++ runtime libraries if linking them dynamically. if (linkStaticness == LinkStaticness.DYNAMIC) { builder.addTransitiveArtifacts(toolchain.getDynamicRuntimeLinkInputs()); - CppConfiguration cppConfiguration = context.getFragment(CppConfiguration.class); - if (cppConfiguration.getLinkDynamicBinariesSeparately()) { - builder.addArtifacts( - LinkerInputs.toLibraryArtifacts( - info.getCcLinkingOutputs().getExecutionDynamicLibraries())); - } + } + if (linkCompileOutputSeparately) { + builder.addArtifacts( + LinkerInputs.toLibraryArtifacts( + info.getCcLinkingOutputs().getExecutionDynamicLibraries())); } // For cc_binary and cc_test rules, there is an implicit dependency on // the malloc library package, which is specified by the "malloc" attribute. @@ -172,6 +172,15 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { if (ruleContext.hasErrors()) { return null; } + + List<String> linkopts = common.getLinkopts(); + LinkStaticness linkStaticness = getLinkStaticness(ruleContext, linkopts, cppConfiguration); + + // We currently only want link the dynamic library generated for test code separately. + boolean linkCompileOutputSeparately = + ruleContext.isTestTarget() + && cppConfiguration.getLinkCompileOutputSeparately() + && linkStaticness == LinkStaticness.DYNAMIC; CcLibraryHelper helper = new CcLibraryHelper(ruleContext, semantics, featureConfiguration) @@ -181,9 +190,19 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { .setFake(fake) .addPrecompiledFiles(precompiledFiles) .enableInterfaceSharedObjects(); - // Always treat test targets as static libraries so that their code can be compiled into a - // dynamic library and profit from optimizations like interface so. - helper.setLinkType(ruleContext.isTestTarget() ? LinkTargetType.STATIC_LIBRARY : linkType); + // When linking the object files directly into the resulting binary, we do not need + // library-level link outputs; thus, we do not let CcLibraryHelper produce link outputs + // (either shared object files or archives) for a non-library link type [*], and add + // the object files explicitly in determineLinkerArguments. + // + // When linking the object files into their own library, we want CcLibraryHelper to + // take care of creating the library link outputs for us, so we need to set the link + // type to STATIC_LIBRARY. + // + // [*] The only library link type is STATIC_LIBRARY. EXECUTABLE specifies a normal + // cc_binary output, while DYNAMIC_LIBRARY is a cc_binary rules that produces an + // output matching a shared object, for example cc_binary(name="foo.so", ...) on linux. + helper.setLinkType(linkCompileOutputSeparately ? LinkTargetType.STATIC_LIBRARY : linkType); CcLibraryHelper.Info info = helper.build(); CppCompilationContext cppCompilationContext = info.getCppCompilationContext(); @@ -206,9 +225,6 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { return null; } - List<String> linkopts = common.getLinkopts(); - LinkStaticness linkStaticness = getLinkStaticness(ruleContext, linkopts, cppConfiguration); - CppLinkActionBuilder linkActionBuilder = determineLinkerArguments( ruleContext, @@ -219,7 +235,8 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { fake, binary, linkStaticness, - linkopts); + linkopts, + linkCompileOutputSeparately); linkActionBuilder.setUseTestOnlyFlags(ruleContext.isTestTarget()); CcToolchainProvider ccToolchain = CppHelper.getToolchain(ruleContext); @@ -333,7 +350,8 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { filesToBuild, fakeLinkerInputs, fake, - helper.getCompilationUnitSources()); + helper.getCompilationUnitSources(), + linkCompileOutputSeparately); RunfilesSupport runfilesSupport = RunfilesSupport.withExecutable( ruleContext, runfiles, executable, ruleContext.getConfiguration().buildRunfiles()); @@ -409,7 +427,8 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { boolean fake, Artifact binary, LinkStaticness linkStaticness, - List<String> linkopts) + List<String> linkopts, + boolean linkCompileOutputSeparately) throws InterruptedException { CppLinkActionBuilder builder = new CppLinkActionBuilder(context, binary) @@ -418,9 +437,7 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { // Either link in the .o files generated for the sources of this target or link in the // generated dynamic library they are compiled into. - CppConfiguration cppConfiguration = context.getFragment(CppConfiguration.class); - if (cppConfiguration.getLinkDynamicBinariesSeparately() - && linkStaticness == LinkStaticness.DYNAMIC) { + if (linkCompileOutputSeparately) { for (LibraryToLink library : info.getCcLinkingOutputs().getDynamicLibraries()) { builder.addLibrary(library); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java index fd205d8951..b1228148c0 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java @@ -1598,8 +1598,8 @@ public class CppConfiguration extends BuildConfiguration.Fragment { return dynamicMode; } - public boolean getLinkDynamicBinariesSeparately() { - return cppOptions.linkDynamicBinariesSeparately; + public boolean getLinkCompileOutputSeparately() { + return cppOptions.linkCompileOutputSeparately; } /* diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java index ecbe6bb70b..7382b00ce2 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java @@ -229,7 +229,7 @@ public class CppOptions extends FragmentOptions { public DynamicModeFlag dynamicMode; @Option( - name = "experimental_link_dynamic_binaries_separately", + name = "experimental_link_compile_output_separately", defaultValue = "false", category = "semantics", help = @@ -237,7 +237,7 @@ public class CppOptions extends FragmentOptions { + "If true, dynamically linked binary targets will build and link their own srcs as " + "a dynamic library instead of directly linking it in." ) - public boolean linkDynamicBinariesSeparately; + public boolean linkCompileOutputSeparately; @Option( name = "force_pic", @@ -550,9 +550,7 @@ public class CppOptions extends FragmentOptions { name = "experimental_skip_unused_modules", defaultValue = "false", category = "experimental", - help = - "If enabled, not all transitive modules automatically become an action's inputs. Instead," - + " input discovery adds just the required ones." + help = "Deprecated. No effect." ) public boolean skipUnusedModules; @@ -560,11 +558,19 @@ public class CppOptions extends FragmentOptions { name = "experimental_prune_more_modules", defaultValue = "false", category = "experimental", - help = "If enabled, modules pruning is used when building modules themselves." + help = "Deprecated. No effect." ) public boolean pruneMoreModules; @Option( + name = "prune_cpp_modules", + defaultValue = "true", + category = "strategy", + help = "If enabled, use the results of input discovery to reduce the number of used modules." + ) + public boolean pruneCppModules; + + @Option( name = "experimental_omitfp", defaultValue = "false", category = "semantics", diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java index 8dfb6a7860..75e7cd912b 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java @@ -165,24 +165,33 @@ public class CppLinkActionTest extends BuildViewTestCase { // its own suite with a TestSpec? return; } - scratch.file("x/BUILD", "cc_test(name = 'a', srcs = ['a.cc'])"); + scratch.file( + "x/BUILD", + "cc_test(name = 'a', srcs = ['a.cc'])", + "cc_binary(name = 'b', srcs = ['a.cc'], linkstatic = 0)"); scratch.file("x/a.cc", "int main() {}"); - useConfiguration("--experimental_link_dynamic_binaries_separately"); + useConfiguration("--experimental_link_compile_output_separately", "--force_pic"); ConfiguredTarget configuredTarget = getConfiguredTarget("//x:a"); + String cpu = CrosstoolConfigurationHelper.defaultCpu(); + String extension = OsUtils.executableExtension(); CppLinkAction linkAction = - (CppLinkAction) - getGeneratingAction(configuredTarget, "x/a" + OsUtils.executableExtension()); + (CppLinkAction) getGeneratingAction(configuredTarget, "x/a" + extension); assertThat(artifactsToStrings(linkAction.getInputs())) - .contains("bin _solib_" + CrosstoolConfigurationHelper.defaultCpu() + "/libx_Sliba.ifso"); + .contains("bin _solib_" + cpu + "/libx_Sliba.ifso"); assertThat(linkAction.getArguments()) .contains( - getBinArtifactWithNoOwner( - "_solib_" + CrosstoolConfigurationHelper.defaultCpu() + "/libx_Sliba.ifso") - .getExecPathString()); + getBinArtifactWithNoOwner("_solib_" + cpu + "/libx_Sliba.ifso").getExecPathString()); RunfilesProvider runfilesProvider = configuredTarget.getProvider(RunfilesProvider.class); assertThat(artifactsToStrings(runfilesProvider.getDefaultRunfiles().getArtifacts())) - .contains("bin _solib_" + CrosstoolConfigurationHelper.defaultCpu() + "/libx_Sliba.so"); + .contains("bin _solib_" + cpu + "/libx_Sliba.so"); + + configuredTarget = getConfiguredTarget("//x:b"); + linkAction = (CppLinkAction) getGeneratingAction(configuredTarget, "x/b" + extension); + assertThat(artifactsToStrings(linkAction.getInputs())).contains("bin x/_objs/b/x/a.pic.o"); + runfilesProvider = configuredTarget.getProvider(RunfilesProvider.class); + assertThat(artifactsToStrings(runfilesProvider.getDefaultRunfiles().getArtifacts())) + .containsExactly("bin x/b"); } @Test |