From 96e244dc211ef3277f154f1f03bd0c87347d522a Mon Sep 17 00:00:00 2001 From: cpeyser Date: Wed, 22 Nov 2017 08:59:00 -0800 Subject: Move CppConfiguration#isLLVMcompiler, #isLLVMOptimizedFdo, #isLipoOptimization, and #getDynamicMode to CcToolchainProvider and CppHelper. Moving toolchain info out of CppConfiguration is required to use platform-based toolchain resolution in the c++ rules. PiperOrigin-RevId: 176662686 --- .../devtools/build/lib/rules/cpp/CcBinary.java | 14 +++-- .../devtools/build/lib/rules/cpp/CcCommon.java | 5 +- .../devtools/build/lib/rules/cpp/CcToolchain.java | 4 +- .../build/lib/rules/cpp/CcToolchainProvider.java | 4 ++ .../build/lib/rules/cpp/CcToolchainRule.java | 1 + .../lib/rules/cpp/CppCompileActionBuilder.java | 8 +-- .../build/lib/rules/cpp/CppConfiguration.java | 59 ++++++++++++---------- .../devtools/build/lib/rules/cpp/CppHelper.java | 23 +++++++++ .../devtools/build/lib/rules/cpp/CppModel.java | 6 +-- .../build/lib/rules/cpp/CppRuleClasses.java | 1 + .../devtools/build/lib/rules/cpp/FdoSupport.java | 5 +- 11 files changed, 84 insertions(+), 46 deletions(-) (limited to 'src/main/java/com/google/devtools/build/lib') 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 6a3aca0a68..642c05c752 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 @@ -186,7 +186,8 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { } List linkopts = common.getLinkopts(); - LinkStaticness linkStaticness = getLinkStaticness(ruleContext, linkopts, cppConfiguration); + LinkStaticness linkStaticness = + getLinkStaticness(ruleContext, linkopts, cppConfiguration, ccToolchain); // We currently only want link the dynamic library generated for test code separately. boolean linkCompileOutputSeparately = @@ -600,13 +601,16 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { return linkopts.contains("-static") || cppConfiguration.hasStaticLinkOption(); } - private static final LinkStaticness getLinkStaticness(RuleContext context, - List linkopts, CppConfiguration cppConfiguration) { - if (cppConfiguration.getDynamicMode() == DynamicMode.FULLY) { + private static final LinkStaticness getLinkStaticness( + RuleContext context, + List linkopts, + CppConfiguration cppConfiguration, + CcToolchainProvider toolchain) { + if (CppHelper.getDynamicMode(cppConfiguration, toolchain) == DynamicMode.FULLY) { return LinkStaticness.DYNAMIC; } else if (dashStaticInLinkopts(linkopts, cppConfiguration)) { return LinkStaticness.FULLY_STATIC; - } else if (cppConfiguration.getDynamicMode() == DynamicMode.OFF + } else if (CppHelper.getDynamicMode(cppConfiguration, toolchain) == DynamicMode.OFF || context.attributes().get("linkstatic", Type.BOOLEAN)) { return LinkStaticness.MOSTLY_STATIC; } else { diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java index 53d5fda3c6..efd1adefe2 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java @@ -130,8 +130,9 @@ public final class CcCommon { Iterable ourLinkopts = ruleContext.attributes().get("linkopts", Type.STRING_LIST); List result; if (ourLinkopts != null) { - boolean allowDashStatic = !cppConfiguration.forceIgnoreDashStatic() - && (cppConfiguration.getDynamicMode() != DynamicMode.FULLY); + boolean allowDashStatic = + !cppConfiguration.forceIgnoreDashStatic() + && (CppHelper.getDynamicMode(cppConfiguration, ccToolchain) != DynamicMode.FULLY); if (!allowDashStatic) { ourLinkopts = Iterables.filter(ourLinkopts, (v) -> !"-static".equals(v)); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java index c66dd9378b..521fcf634f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java @@ -342,7 +342,7 @@ public class CcToolchain implements RuleConfiguredTargetFactory { cppConfiguration.getLipoMode(), fdoZip, cppConfiguration.getFdoInstrument(), - cppConfiguration.isLLVMOptimizedFdo()); + cppConfiguration.isLLVMOptimizedFdo(toolchainInfo.isLLVMCompiler())); SkyFunction.Environment skyframeEnv = ruleContext.getAnalysisEnvironment().getSkyframeEnv(); FdoSupportValue fdoSupport; @@ -486,7 +486,7 @@ public class CcToolchain implements RuleConfiguredTargetFactory { // This tries to convert LLVM profiles to the indexed format if necessary. Artifact profileArtifact = null; - if (cppConfiguration.isLLVMOptimizedFdo()) { + if (cppConfiguration.isLLVMOptimizedFdo(toolchainInfo.isLLVMCompiler())) { profileArtifact = convertLLVMRawProfileToIndexed(fdoZip, toolchainInfo, cppConfiguration, ruleContext); if (ruleContext.hasErrors()) { diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java index 622416056f..adec4f5ee2 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java @@ -659,6 +659,10 @@ public final class CcToolchainProvider extends ToolchainInfo { return toolchainInfo.getGnuSystemArch(); } + public final boolean isLLVMCompiler() { + return toolchainInfo.isLLVMCompiler(); + } + // Not all of CcToolchainProvider is exposed to Skylark, which makes implementing deep equality // impossible: if Java-only parts are considered, the behavior is surprising in Skylark, if they // are not, the behavior is surprising in Java. Thus, object identity it is. diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java index fa38a0bd21..e4847052ad 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java @@ -122,6 +122,7 @@ public final class CcToolchainRule implements RuleDefinition { LateBoundDefault.fromTargetConfiguration( CppConfiguration.class, null, + // TODO(b/69547565): Remove call to isLLVMOptimizedFdo (rule, attributes, cppConfig) -> cppConfig.isLLVMOptimizedFdo() ? zipper : null))) .add(attr(":libc_top", LABEL).value(LIBC_TOP)) diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java index 3d938e015e..9d89feea2f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java @@ -93,7 +93,7 @@ public class CppCompileActionBuilder { this( ruleContext.getActionOwner(), ruleContext.getConfiguration(), - getLipoScannableMap(ruleContext), + getLipoScannableMap(ruleContext, ccToolchain), ccToolchain); } @@ -105,7 +105,7 @@ public class CppCompileActionBuilder { this( ruleContext.getActionOwner(), configuration, - getLipoScannableMap(ruleContext), + getLipoScannableMap(ruleContext, ccToolchain), ccToolchain); } @@ -128,8 +128,8 @@ public class CppCompileActionBuilder { } private static ImmutableMap getLipoScannableMap( - RuleContext ruleContext) { - if (!ruleContext.getFragment(CppConfiguration.class).isLipoOptimization() + RuleContext ruleContext, CcToolchainProvider toolchain) { + if (!CppHelper.isLipoOptimization(ruleContext.getFragment(CppConfiguration.class), toolchain) // Rules that do not contain sources that are compiled into object files, but may // contain headers, will still create CppCompileActions without providing a // lipo_context_collector. 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 b2faac2146..30dbdd976f 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 @@ -252,7 +252,6 @@ public class CppConfiguration extends BuildConfiguration.Fragment { private final Function cpuTransformer; // The dynamic mode for linking. - private final DynamicMode dynamicMode; private final boolean stripBinaries; private final CompilationMode compilationMode; private final boolean useLLVMCoverageMap; @@ -287,15 +286,6 @@ public class CppConfiguration extends BuildConfiguration.Fragment { this.shouldProvideMakeVariables = params.commonOptions.makeVariableSource == MakeVariableSource.CONFIGURATION; - // With LLVM, ThinLTO is automatically used in place of LIPO. ThinLTO works fine with dynamic - // linking (and in fact creates a lot more work when dynamic linking is off). - if (cppOptions.getLipoMode() == LipoMode.BINARY && !isLLVMCompiler()) { - // TODO(bazel-team): implement dynamic linking with LIPO - this.dynamicMode = DynamicMode.OFF; - } else { - this.dynamicMode = cppOptions.dynamicMode; - } - this.fdoZip = params.fdoZip; this.stripBinaries = (cppOptions.stripBinaries == StripMode.ALWAYS @@ -878,11 +868,9 @@ public class CppConfiguration extends BuildConfiguration.Fragment { return ldExecutable != null ? ldExecutable.getPathString() : ""; } - /** - * Returns the dynamic linking mode (full, off, or default). - */ - public DynamicMode getDynamicMode() { - return dynamicMode; + /** Returns the value of the --dynamic_mode flag. */ + public DynamicMode getDynamicModeFlag() { + return cppOptions.dynamicMode; } public boolean getLinkCompileOutputSeparately() { @@ -921,11 +909,20 @@ public class CppConfiguration extends BuildConfiguration.Fragment { return cppOptions.isFdo(); } - public final boolean isLLVMCompiler() { + /** Deprecated: Use {@link CcToolchainProvider#isLLVMCompiler()} */ + // TODO(b/64384912): Remove in favor of CcToolchainProvider#isLLVMCompiler + @Deprecated + private final boolean isLLVMCompiler() { return cppToolchainInfo.isLLVMCompiler(); } - /** Returns true if LLVM FDO Optimization should be applied for this configuration. */ + /** + * Returns true if LLVM FDO Optimization should be applied for this configuration. + * + *

Deprecated: Use {@link CppConfiguration#isLLVMOptimizedFdo(boolean)} + */ + // TODO(b/64384912): Remove in favor of overload with isLLVMCompiler. + @Deprecated public boolean isLLVMOptimizedFdo() { return cppOptions.getFdoOptimize() != null && (CppFileTypes.LLVM_PROFILE.matches(cppOptions.getFdoOptimize()) @@ -934,26 +931,32 @@ public class CppConfiguration extends BuildConfiguration.Fragment { && cppOptions.getFdoOptimize().endsWith(".zip"))); } + /** Returns true if LLVM FDO Optimization should be applied for this configuration. */ + public boolean isLLVMOptimizedFdo(boolean isLLVMCompiler) { + return cppOptions.getFdoOptimize() != null + && (CppFileTypes.LLVM_PROFILE.matches(cppOptions.getFdoOptimize()) + || CppFileTypes.LLVM_PROFILE_RAW.matches(cppOptions.getFdoOptimize()) + || (isLLVMCompiler && cppOptions.getFdoOptimize().endsWith(".zip"))); + } + + /** Returns true if LIPO optimization is implied by the flags of this build. */ + public boolean lipoOptimizationIsActivated() { + return cppOptions.isLipoOptimization(); + } + /** * Returns true if LIPO optimization should be applied for this configuration. + * + *

Deprecated: Use {@link CppHelper#isLipoOptimization(CppConfiguration, CcToolchainProvider)} */ + // TODO(b/64384912): Remove usage in topLevelConfigurationHook and CppRuleClasses and delete. + @Deprecated public boolean isLipoOptimization() { // The LIPO optimization bits are set in the LIPO context collector configuration, too. // If compiler is LLVM, then LIPO gets auto-converted to ThinLTO. return cppOptions.isLipoOptimization() && !isLLVMCompiler(); } - /** - * Returns true if this is a data configuration for a LIPO-optimizing build. - * - *

This means LIPO is not applied for this configuration, but LIPO might be reenabled further - * down the dependency tree. - */ - public boolean isDataConfigurationForLipoOptimization() { - // If compiler is LLVM, then LIPO gets auto-converted to ThinLTO. - return cppOptions.isDataConfigurationForLipoOptimization() && !isLLVMCompiler(); - } - public boolean isLipoOptimizationOrInstrumentation() { return cppOptions.isLipoOptimizationOrInstrumentation(); } 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 2a65615450..c673b87ea0 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 @@ -54,6 +54,7 @@ import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfig import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Tool; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables; import com.google.devtools.build.lib.rules.cpp.CppCompilationContext.Builder; +import com.google.devtools.build.lib.rules.cpp.CppConfiguration.DynamicMode; import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType; import com.google.devtools.build.lib.shell.ShellUtils; import com.google.devtools.build.lib.syntax.Type; @@ -267,6 +268,28 @@ public class CppHelper { return false; } + /** + * Returns the dynamic linking mode (full, off, or default) implied by the given configuration and + * toolchain. + */ + public static DynamicMode getDynamicMode(CppConfiguration config, CcToolchainProvider toolchain) { + // With LLVM, ThinLTO is automatically used in place of LIPO. ThinLTO works fine with dynamic + // linking (and in fact creates a lot more work when dynamic linking is off). + if (config.getLipoMode() == LipoMode.BINARY && !toolchain.isLLVMCompiler()) { + // TODO(bazel-team): implement dynamic linking with LIPO + return DynamicMode.OFF; + } else { + return config.getDynamicModeFlag(); + } + } + + /** Returns true if LIPO optimization should be applied for this configuration and toolchain. */ + public static boolean isLipoOptimization(CppConfiguration config, CcToolchainProvider toolchain) { + // The LIPO optimization bits are set in the LIPO context collector configuration, too. + // If compiler is LLVM, then LIPO gets auto-converted to ThinLTO. + return config.lipoOptimizationIsActivated() && !toolchain.isLLVMCompiler(); + } + /** * Return {@link FdoSupportProvider} using default cc_toolchain attribute name. * diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java index 5a51cb012f..ade706c70f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java @@ -840,7 +840,7 @@ public final class CppModel { ruleContext, ccToolchain, ArtifactCategory.COVERAGE_DATA_FILE, outputName); // TODO(djasper): This is now duplicated. Refactor the various create..Action functions. Artifact gcnoFile = - isCodeCoverageEnabled() && !cppConfiguration.isLipoOptimization() + isCodeCoverageEnabled() && !CppHelper.isLipoOptimization(cppConfiguration, ccToolchain) ? CppHelper.getCompileOutputArtifact(ruleContext, gcnoFileName, configuration) : null; @@ -967,7 +967,7 @@ public final class CppModel { throws RuleErrorException { ImmutableList.Builder directOutputs = new ImmutableList.Builder<>(); PathFragment ccRelativeName = semantics.getEffectiveSourcePath(sourceArtifact); - if (cppConfiguration.isLipoOptimization()) { + if (CppHelper.isLipoOptimization(cppConfiguration, ccToolchain)) { // TODO(bazel-team): we shouldn't be needing this, merging context with the binary // is a superset of necessary information. LipoContextProvider lipoProvider = @@ -1078,7 +1078,7 @@ public final class CppModel { // Create non-PIC compile actions Artifact gcnoFile = - !cppConfiguration.isLipoOptimization() && enableCoverage + !CppHelper.isLipoOptimization(cppConfiguration, ccToolchain) && enableCoverage ? CppHelper.getCompileOutputArtifact(ruleContext, gcnoFileName, configuration) : null; 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 0b284683b6..453ab635a2 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 @@ -60,6 +60,7 @@ public class CppRuleClasses { LateBoundDefault.fromTargetConfiguration( CppConfiguration.class, null, + // TODO(b/69548520): Remove call to isLipoOptimization (rule, attributes, cppConfig) -> cppConfig.isLipoOptimization() ? cppConfig.getLipoContextLabel() : null); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupport.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupport.java index 7a6c07f102..b58c9512be 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupport.java @@ -626,9 +626,10 @@ public class FdoSupport { private Iterable getAuxiliaryInputs( RuleContext ruleContext, PathFragment sourceName, PathFragment sourceExecPath, boolean usePic, FdoSupportProvider fdoSupportProvider) { - CppConfiguration cppConfig = ruleContext.getFragment(CppConfiguration.class); + CcToolchainProvider toolchain = + CppHelper.getToolchainUsingDefaultCcToolchainAttribute(ruleContext); LipoContextProvider lipoContextProvider = - cppConfig.isLLVMCompiler() ? null : CppHelper.getLipoContextProvider(ruleContext); + toolchain.isLLVMCompiler() ? null : CppHelper.getLipoContextProvider(ruleContext); // If --fdo_optimize was not specified, we don't have any additional inputs. if (fdoProfile == null) { -- cgit v1.2.3