Add support for crosstool feature to prefer PIC compiles even for optimized binaries. This can have performance penalty, but in configurations where dynamic linking is used for tests can lead to a substantially better sharing of artifacts between tests and binaries. In contrast to the existing --force_pic, this can be enabled per crosstool and respects whether PIC is available for the used crosstool.
PiperOrigin-RevId: 492940462
Change-Id: I178088730d50f057d45f41b46ca94bad5bd231da
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
index 3d533b8..277cb0d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
@@ -603,7 +603,8 @@
FeatureConfiguration featureConfiguration) {
return cppConfiguration.forcePic()
|| (toolchain.usePicForDynamicLibraries(cppConfiguration, featureConfiguration)
- && cppConfiguration.getCompilationMode() != CompilationMode.OPT);
+ && (cppConfiguration.getCompilationMode() != CompilationMode.OPT
+ || featureConfiguration.isEnabled(CppRuleClasses.PREFER_PIC_FOR_OPT_BINARIES)));
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java
index a19535c..967055d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java
@@ -254,6 +254,13 @@
/** A string constant for a feature that indicates that the toolchain can produce PIC objects. */
public static final String SUPPORTS_PIC = "supports_pic";
+ /**
+ * A string constant for a feature that indicates that PIC compiles are preferred for binaries
+ * even in optimized builds. For configurations that use dynamic linking for tests, this provides
+ * increases sharing of artifacts between tests and binaries at the cost of performance overhead.
+ */
+ public static final String PREFER_PIC_FOR_OPT_BINARIES = "prefer_pic_for_opt_binaries";
+
/** A string constant for the feature the represents preprocessor defines. */
public static final String PREPROCESSOR_DEFINES = "preprocessor_defines";
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/mock/cc_toolchain_config.bzl b/src/test/java/com/google/devtools/build/lib/analysis/mock/cc_toolchain_config.bzl
index 39683bf..c7be375 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/mock/cc_toolchain_config.bzl
+++ b/src/test/java/com/google/devtools/build/lib/analysis/mock/cc_toolchain_config.bzl
@@ -77,6 +77,7 @@
implicit_fsafdo = "implicit_fsafdo",
enable_fsafdo = "enable_fsafdo",
supports_pic = "supports_pic",
+ prefer_pic_for_opt_binaries = "prefer_pic_for_opt_binaries",
copy_dynamic_libraries_to_binary = "copy_dynamic_libraries_to_binary",
per_object_debug_info = "per_object_debug_info",
supports_start_end_lib = "supports_start_end_lib",
@@ -816,6 +817,11 @@
_supports_pic_feature = feature(name = _FEATURE_NAMES.supports_pic, enabled = True)
+_prefer_pic_for_opt_binaries_feature = feature(
+ name = _FEATURE_NAMES.prefer_pic_for_opt_binaries,
+ enabled = True,
+)
+
_targets_windows_feature = feature(
name = _FEATURE_NAMES.targets_windows,
enabled = True,
@@ -1342,6 +1348,7 @@
_FEATURE_NAMES.copy_dynamic_libraries_to_binary: _copy_dynamic_libraries_to_binary_feature,
_FEATURE_NAMES.supports_start_end_lib: _supports_start_end_lib_feature,
_FEATURE_NAMES.supports_pic: _supports_pic_feature,
+ _FEATURE_NAMES.prefer_pic_for_opt_binaries: _prefer_pic_for_opt_binaries_feature,
_FEATURE_NAMES.targets_windows: _targets_windows_feature,
_FEATURE_NAMES.archive_param_file: _archive_param_file_feature,
_FEATURE_NAMES.compiler_param_file: _compiler_param_file_feature,
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java
index 4743437..0d9eb09 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java
@@ -1014,6 +1014,66 @@
}
@Test
+ public void testPreferPicForOptBinaryFeature() throws Exception {
+ getAnalysisMock()
+ .ccSupport()
+ .setupCcToolchainConfig(
+ mockToolsConfig,
+ CcToolchainConfig.builder()
+ .withFeatures(
+ CppRuleClasses.NO_LEGACY_FEATURES,
+ CppRuleClasses.SUPPORTS_PIC,
+ CppRuleClasses.PREFER_PIC_FOR_OPT_BINARIES)
+ .withActionConfigs(
+ CppActionNames.CPP_LINK_STATIC_LIBRARY,
+ CppActionNames.CPP_COMPILE,
+ CppActionNames.CPP_LINK_NODEPS_DYNAMIC_LIBRARY));
+ useConfiguration("--cpu=k8", "--compilation_mode=opt");
+
+ scratch.file("x/BUILD", "cc_library(name = 'foo', srcs = ['a.cc'])");
+ scratch.file("x/a.cc");
+
+ RuleConfiguredTarget ccLibrary = (RuleConfiguredTarget) getConfiguredTarget("//x:foo");
+ ImmutableList<ActionAnalysisMetadata> actions = ccLibrary.getActions();
+ ImmutableList<String> outputs =
+ actions.stream()
+ .map(ActionAnalysisMetadata::getPrimaryOutput)
+ .map(Artifact::getFilename)
+ .collect(ImmutableList.toImmutableList());
+ assertThat(outputs).doesNotContain("a.o");
+ assertThat(outputs).contains("a.pic.o");
+ }
+
+ @Test
+ public void testPreferPicForOptBinaryFeatureNeedsPicSupport() throws Exception {
+ getAnalysisMock()
+ .ccSupport()
+ .setupCcToolchainConfig(
+ mockToolsConfig,
+ CcToolchainConfig.builder()
+ .withFeatures(
+ CppRuleClasses.NO_LEGACY_FEATURES, CppRuleClasses.PREFER_PIC_FOR_OPT_BINARIES)
+ .withActionConfigs(
+ CppActionNames.CPP_LINK_STATIC_LIBRARY,
+ CppActionNames.CPP_COMPILE,
+ CppActionNames.CPP_LINK_NODEPS_DYNAMIC_LIBRARY));
+ useConfiguration("--cpu=k8", "--compilation_mode=opt");
+
+ scratch.file("x/BUILD", "cc_library(name = 'foo', srcs = ['a.cc'])");
+ scratch.file("x/a.cc");
+
+ RuleConfiguredTarget ccLibrary = (RuleConfiguredTarget) getConfiguredTarget("//x:foo");
+ ImmutableList<ActionAnalysisMetadata> actions = ccLibrary.getActions();
+ ImmutableList<String> outputs =
+ actions.stream()
+ .map(ActionAnalysisMetadata::getPrimaryOutput)
+ .map(Artifact::getFilename)
+ .collect(ImmutableList.toImmutableList());
+ assertThat(outputs).doesNotContain("a.pic.o");
+ assertThat(outputs).contains("a.o");
+ }
+
+ @Test
public void testWhenSupportsPicNotPresentAndForcePicPassedIsError() throws Exception {
reporter.removeHandler(failFastHandler);
getAnalysisMock()
