diff options
author | Googler <noreply@google.com> | 2018-02-13 07:33:14 -0800 |
---|---|---|
committer | Copybara-Service <copybara-piper@google.com> | 2018-02-13 07:35:20 -0800 |
commit | 10118ab46e2f3a8e32384bbf4bb30091e9b629d4 (patch) | |
tree | 2277b220f7679bbf9eb64d1784b2a8da361b6394 /src/main/java/com/google/devtools/build/lib | |
parent | 3bd6663e3644bf22606860bf2e594a07425bcbc1 (diff) |
Route --fdo_optimize information through an implicit dependency instead of CppConfiguration
RELNOTES: None.
PiperOrigin-RevId: 185527875
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib')
4 files changed, 93 insertions, 55 deletions
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 5f848b4a29..f52a13c4a6 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 @@ -55,6 +55,7 @@ import com.google.devtools.build.lib.rules.cpp.CppConfiguration.Tool; import com.google.devtools.build.lib.rules.cpp.FdoSupport.FdoException; import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.util.FileType; +import com.google.devtools.build.lib.util.FileTypeSet; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; @@ -334,9 +335,42 @@ public class CcToolchain implements RuleConfiguredTargetFactory { toolchainInfo = cppConfiguration.getCppToolchainInfo(); } - Path fdoZip = ruleContext.getConfiguration().getCompilationMode() == CompilationMode.OPT - ? cppConfiguration.getFdoZip() - : null; + Path fdoZip = null; + if (ruleContext.getConfiguration().getCompilationMode() == CompilationMode.OPT) { + if (cppConfiguration.getFdoProfileAbsolutePath() != null) { + fdoZip = cppConfiguration.getFdoProfileAbsolutePath(); + } else if (cppConfiguration.getFdoProfileLabel() != null) { + Artifact fdoArtifact = ruleContext.getPrerequisiteArtifact(":fdo_optimize", Mode.TARGET); + if (fdoArtifact != null) { + if (!fdoArtifact.isSourceArtifact()) { + ruleContext.ruleError("--fdo_optimize points to a target that is not an input file"); + return null; + } + Label fdoLabel = ruleContext.getPrerequisite(":fdo_optimize", Mode.TARGET).getLabel(); + if (!fdoLabel + .getPackageIdentifier() + .getPathUnderExecRoot() + .getRelative(fdoLabel.getName()) + .equals(fdoArtifact.getExecPath())) { + ruleContext.ruleError("--fdo_optimize points to a target that is not an input file"); + return null; + } + fdoZip = fdoArtifact.getPath(); + } + } + } + + FileTypeSet validExtensions = + FileTypeSet.of( + CppFileTypes.GCC_AUTO_PROFILE, + CppFileTypes.LLVM_PROFILE, + CppFileTypes.LLVM_PROFILE_RAW, + FileType.of(".zip")); + if (fdoZip != null && !validExtensions.matches(fdoZip.getPathString())) { + ruleContext.ruleError("invalid extension for FDO profile file."); + return null; + } + SkyKey fdoKey = FdoSupportValue.key( cppConfiguration.getLipoMode(), 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 6a66fb6f2c..e7bb964bbe 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 @@ -54,6 +54,12 @@ public final class CcToolchainRule implements RuleDefinition { null, (rule, attributes, cppConfig) -> cppConfig.getSysrootLabel()); + private static final LabelLateBoundDefault<?> FDO_LABEL = + LabelLateBoundDefault.fromTargetConfiguration( + CppConfiguration.class, + null, + (rule, attributes, cppConfig) -> cppConfig.getFdoProfileLabel()); + @Override public RuleClass build(Builder builder, RuleDefinitionEnvironment env) { final Label zipper = env.getToolsLabel("//tools/zip:zipper"); @@ -126,6 +132,7 @@ public final class CcToolchainRule implements RuleDefinition { (rule, attributes, cppConfig) -> cppConfig.isLLVMOptimizedFdo() ? zipper : null))) .add(attr(":libc_top", LABEL).value(LIBC_TOP)) + .add(attr(":fdo_optimize", LABEL).singleArtifact().value(FDO_LABEL)) .add( attr(TransitiveLipoInfoProvider.LIPO_CONTEXT_COLLECTOR, LABEL) .cfg(LipoContextCollectorTransition.INSTANCE) 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 107a4cce77..36624d85a7 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 @@ -168,7 +168,8 @@ public final class CppConfiguration extends BuildConfiguration.Fragment { private final boolean convertLipoToThinLto; private final PathFragment crosstoolTopPathFragment; - private final Path fdoZip; + private final Path fdoProfileAbsolutePath; + private final Label fdoProfileLabel; // TODO(bazel-team): All these labels (except for ccCompilerRuleLabel) can be removed once the // transition to the cc_compiler rule is complete. @@ -272,7 +273,8 @@ public final class CppConfiguration extends BuildConfiguration.Fragment { Preconditions.checkNotNull(params.commonOptions.cpu), cppOptions.convertLipoToThinLto, crosstoolTopPathFragment, - params.fdoZip, + params.fdoProfileAbsolutePath, + params.fdoProfileLabel, params.ccToolchainLabel, params.stlLabel, params.sysrootLabel == null @@ -294,30 +296,22 @@ public final class CppConfiguration extends BuildConfiguration.Fragment { ImmutableList.copyOf(cppOptions.conlyoptList), new FlagList( cppToolchainInfo.configureLinkerOptions( - compilationMode, - cppOptions.getLipoMode(), - LinkingMode.FULLY_STATIC), + compilationMode, cppOptions.getLipoMode(), LinkingMode.FULLY_STATIC), FlagList.convertOptionalOptions(toolchain.getOptionalLinkerFlagList()), ImmutableList.<String>of()), new FlagList( cppToolchainInfo.configureLinkerOptions( - compilationMode, - cppOptions.getLipoMode(), - LinkingMode.MOSTLY_STATIC), + compilationMode, cppOptions.getLipoMode(), LinkingMode.MOSTLY_STATIC), FlagList.convertOptionalOptions(toolchain.getOptionalLinkerFlagList()), ImmutableList.<String>of()), new FlagList( cppToolchainInfo.configureLinkerOptions( - compilationMode, - cppOptions.getLipoMode(), - LinkingMode.MOSTLY_STATIC_LIBRARIES), + compilationMode, cppOptions.getLipoMode(), LinkingMode.MOSTLY_STATIC_LIBRARIES), FlagList.convertOptionalOptions(toolchain.getOptionalLinkerFlagList()), ImmutableList.<String>of()), new FlagList( cppToolchainInfo.configureLinkerOptions( - compilationMode, - cppOptions.getLipoMode(), - LinkingMode.DYNAMIC), + compilationMode, cppOptions.getLipoMode(), LinkingMode.DYNAMIC), FlagList.convertOptionalOptions(toolchain.getOptionalLinkerFlagList()), ImmutableList.<String>of()), ImmutableList.copyOf(cppOptions.coptList), @@ -343,7 +337,8 @@ public final class CppConfiguration extends BuildConfiguration.Fragment { String desiredCpu, boolean convertLipoToThinLto, PathFragment crosstoolTopPathFragment, - Path fdoZip, + Path fdoProfileAbsolutePath, + Label fdoProfileLabel, Label ccToolchainLabel, Label stlLabel, PathFragment nonConfiguredSysroot, @@ -373,7 +368,8 @@ public final class CppConfiguration extends BuildConfiguration.Fragment { this.desiredCpu = desiredCpu; this.convertLipoToThinLto = convertLipoToThinLto; this.crosstoolTopPathFragment = crosstoolTopPathFragment; - this.fdoZip = fdoZip; + this.fdoProfileAbsolutePath = fdoProfileAbsolutePath; + this.fdoProfileLabel = fdoProfileLabel; this.ccToolchainLabel = ccToolchainLabel; this.stlLabel = stlLabel; this.nonConfiguredSysroot = nonConfiguredSysroot; @@ -1333,8 +1329,12 @@ public final class CppConfiguration extends BuildConfiguration.Fragment { return cppOptions.getFdoInstrument(); } - public Path getFdoZip() { - return fdoZip; + public Path getFdoProfileAbsolutePath() { + return fdoProfileAbsolutePath; + } + + public Label getFdoProfileLabel() { + return fdoProfileLabel; } /** @@ -1349,6 +1349,12 @@ public final class CppConfiguration extends BuildConfiguration.Fragment { requestedFeatures.add(CppRuleClasses.FDO_INSTRUMENT); } + String fdoZip = null; + if (fdoProfileAbsolutePath != null) { + fdoZip = fdoProfileAbsolutePath.getPathString(); + } else if (fdoProfileLabel != null) { + fdoZip = fdoProfileLabel.getName(); + } boolean isFdo = fdoZip != null && compilationMode == CompilationMode.OPT; if (isFdo && !CppFileTypes.GCC_AUTO_PROFILE.matches(fdoZip)) { requestedFeatures.add(CppRuleClasses.FDO_OPTIMIZE); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfigurationLoader.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfigurationLoader.java index 57a5a0487f..92754dbce8 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfigurationLoader.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfigurationLoader.java @@ -28,9 +28,6 @@ import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.packages.BuildType; -import com.google.devtools.build.lib.packages.InputFile; -import com.google.devtools.build.lib.packages.NoSuchPackageException; -import com.google.devtools.build.lib.packages.NoSuchTargetException; import com.google.devtools.build.lib.packages.NoSuchThingException; import com.google.devtools.build.lib.packages.NonconfigurableAttributeMapper; import com.google.devtools.build.lib.packages.Rule; @@ -88,7 +85,8 @@ public class CppConfigurationLoader implements ConfigurationFragmentFactory { protected final Label crosstoolTop; protected final Label ccToolchainLabel; protected final Label stlLabel; - protected final Path fdoZip; + protected final Path fdoProfileAbsolutePath; + protected final Label fdoProfileLabel; protected final Label sysrootLabel; protected final CpuTransformer cpuTransformer; @@ -97,7 +95,8 @@ public class CppConfigurationLoader implements ConfigurationFragmentFactory { CrosstoolConfigurationLoader.CrosstoolFile crosstoolFile, String cacheKeySuffix, BuildOptions buildOptions, - Path fdoZip, + Path fdoProfileAbsolutePath, + Label fdoProfileLabel, Label crosstoolTop, Label ccToolchainLabel, Label stlLabel, @@ -108,7 +107,8 @@ public class CppConfigurationLoader implements ConfigurationFragmentFactory { this.cacheKeySuffix = cacheKeySuffix; this.commonOptions = buildOptions.get(BuildConfiguration.Options.class); this.cppOptions = buildOptions.get(CppOptions.class); - this.fdoZip = fdoZip; + this.fdoProfileAbsolutePath = fdoProfileAbsolutePath; + this.fdoProfileLabel = fdoProfileLabel; this.crosstoolTop = crosstoolTop; this.ccToolchainLabel = ccToolchainLabel; this.stlLabel = stlLabel; @@ -151,35 +151,25 @@ public class CppConfigurationLoader implements ConfigurationFragmentFactory { // FDO // TODO(bazel-team): move this to CppConfiguration.prepareHook - Path fdoZip; - if (cppOptions.getFdoOptimize() == null) { - fdoZip = null; - } else if (cppOptions.getFdoOptimize().startsWith("//")) { - try { - Target target = env.getTarget(Label.parseAbsolute(cppOptions.getFdoOptimize())); - if (target == null) { - return null; - } - if (!(target instanceof InputFile)) { - throw new InvalidConfigurationException( - "--fdo_optimize cannot accept targets that do not refer to input files"); + Path fdoProfileAbsolutePath = null; + Label fdoProfileLabel = null; + if (cppOptions.getFdoOptimize() != null) { + if (cppOptions.getFdoOptimize().startsWith("//")) { + try { + fdoProfileLabel = Label.parseAbsolute(cppOptions.getFdoOptimize()); + } catch (LabelSyntaxException e) { + env.getEventHandler().handle(Event.error(e.getMessage())); + throw new InvalidConfigurationException(e); } - fdoZip = env.getPath(target.getPackage(), target.getName()); - if (fdoZip == null) { - throw new InvalidConfigurationException( - "The --fdo_optimize parameter you specified resolves to a file that does not exist"); + } else { + fdoProfileAbsolutePath = + directories.getWorkspace().getRelative(cppOptions.getFdoOptimize()); + try { + // We don't check for file existence, but at least the filename should be well-formed. + FileSystemUtils.checkBaseName(fdoProfileAbsolutePath.getBaseName()); + } catch (IllegalArgumentException e) { + throw new InvalidConfigurationException(e); } - } catch (NoSuchPackageException | NoSuchTargetException | LabelSyntaxException e) { - env.getEventHandler().handle(Event.error(e.getMessage())); - throw new InvalidConfigurationException(e); - } - } else { - fdoZip = directories.getWorkspace().getRelative(cppOptions.getFdoOptimize()); - try { - // We don't check for file existence, but at least the filename should be well-formed. - FileSystemUtils.checkBaseName(fdoZip.getBaseName()); - } catch (IllegalArgumentException e) { - throw new InvalidConfigurationException(e); } } @@ -232,7 +222,8 @@ public class CppConfigurationLoader implements ConfigurationFragmentFactory { file, file.getMd5(), options, - fdoZip, + fdoProfileAbsolutePath, + fdoProfileLabel, crosstoolTopLabel, ccToolchainLabel, stlLabel, |