Setup FDO command-line options via feature configurations.

--
MOS_MIGRATED_REVID=96835732

10 files changed, 317 insertions, 133 deletions

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 a26585e614..c4c49af695 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
@@ -682,6 +682,25 @@ public final class CcCommon {
       }
     }
     requestedFeatures.addAll(ruleSpecificRequestedFeatures);
+
+    // Enable FDO related features requested by options.
+    CppConfiguration cppConfiguration =
+        ruleContext.getConfiguration().getFragment(CppConfiguration.class);
+    FdoSupport fdoSupport = cppConfiguration.getFdoSupport();
+    if (fdoSupport.getFdoInstrument() != null) {
+      requestedFeatures.add(CppRuleClasses.FDO_INSTRUMENT);
+    }
+    if (fdoSupport.getFdoOptimizeProfile() != null
+        && !fdoSupport.isAutoFdoEnabled()) {
+      requestedFeatures.add(CppRuleClasses.FDO_OPTIMIZE);
+    }
+    if (fdoSupport.isAutoFdoEnabled()) {
+      requestedFeatures.add(CppRuleClasses.AUTOFDO);
+    }
+    if (cppConfiguration.isLipoOptimizationOrInstrumentation()) {
+      requestedFeatures.add(CppRuleClasses.LIPO);
+    }
+
     FeatureConfiguration configuration =
         toolchain.getFeatures().getFeatureConfiguration(requestedFeatures.build());
     for (String feature : unsupportedFeatures) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
index b9db265ba3..4a0fc051e1 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
@@ -70,7 +70,6 @@ import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
-import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -203,6 +202,7 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable
       // will be provided by 'featureConfiguration'.
       ImmutableList<String> features,
       FeatureConfiguration featureConfiguration,
+      CcToolchainFeatures.Variables variables,
       Artifact sourceFile,
       Label sourceLabel,
       NestedSet<Artifact> mandatoryInputs,
@@ -244,8 +244,8 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable
       inputsKnown = true;
     }
     this.cppCompileCommandLine = new CppCompileCommandLine(sourceFile, dotdFile,
-        context.getCppModuleMap(), copts, coptsFilter, pluginOpts,
-        (gcnoFile != null), features, featureConfiguration, fdoBuildStamp);
+        copts, coptsFilter, pluginOpts, (gcnoFile != null), features,
+        featureConfiguration, variables, fdoBuildStamp);
     this.actionContext = actionContext;
     this.lipoScannables = lipoScannables;
     this.actionClassId = actionClassId;
@@ -1166,31 +1166,32 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable
   public final class CppCompileCommandLine {
     private final Artifact sourceFile;
     private final DotdFile dotdFile;
-    private final CppModuleMap cppModuleMap;
     private final List<String> copts;
     private final Predicate<String> coptsFilter;
     private final List<String> pluginOpts;
     private final boolean isInstrumented;
     private final Collection<String> features;
     private final FeatureConfiguration featureConfiguration;
+    private final CcToolchainFeatures.Variables variables;
 
     // The value of the BUILD_FDO_TYPE macro to be defined on command line
     @Nullable private final String fdoBuildStamp;
 
-    public CppCompileCommandLine(Artifact sourceFile, DotdFile dotdFile, CppModuleMap cppModuleMap,
+    public CppCompileCommandLine(Artifact sourceFile, DotdFile dotdFile,
         ImmutableList<String> copts, Predicate<String> coptsFilter,
         ImmutableList<String> pluginOpts, boolean isInstrumented,
         Collection<String> features, FeatureConfiguration featureConfiguration,
+        CcToolchainFeatures.Variables variables,
         @Nullable String fdoBuildStamp) {
       this.sourceFile = Preconditions.checkNotNull(sourceFile);
       this.dotdFile = Preconditions.checkNotNull(dotdFile);
-      this.cppModuleMap = cppModuleMap;
       this.copts = Preconditions.checkNotNull(copts);
       this.coptsFilter = coptsFilter;
       this.pluginOpts = Preconditions.checkNotNull(pluginOpts);
       this.isInstrumented = isInstrumented;
       this.features = Preconditions.checkNotNull(features);
       this.featureConfiguration = featureConfiguration;
+      this.variables = variables;
       this.fdoBuildStamp = fdoBuildStamp;
     }
 
@@ -1264,9 +1265,6 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable
         addFilteredOptions(options, toolchain.getCxxOptions(features));
       }
 
-      // Users don't expect the explicit copts to be filtered by coptsFilter, add them verbatim.
-      options.addAll(copts);
-
       for (String warn : cppConfiguration.getCWarns()) {
         options.add("-W" + warn);
       }
@@ -1279,35 +1277,21 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable
         options.add("-D" + CppConfiguration.FDO_STAMP_MACRO + "=\"" + fdoBuildStamp + "\"");
       }
 
-      CcToolchainFeatures.Variables.Builder buildVariables =
-          new CcToolchainFeatures.Variables.Builder();
-      if (featureConfiguration.isEnabled(CppRuleClasses.MODULE_MAPS) && cppModuleMap != null) {
-        // If the feature is enabled and cppModuleMap is null, we are about to fail during analysis
-        // in any case, but don't crash.
-        buildVariables.addVariable("module_name", cppModuleMap.getName());
-        buildVariables.addVariable("module_map_file",
-            cppModuleMap.getArtifact().getExecPathString());
-      }
-      if (featureConfiguration.isEnabled(CppRuleClasses.USE_HEADER_MODULES)) {
-        buildVariables.addSequenceVariable("module_files", getHeaderModulePaths());
-      }
-      if (featureConfiguration.isEnabled(CppRuleClasses.INCLUDE_PATHS)) {
-        buildVariables.addSequenceVariable("include_paths",
-            getSafePathStrings(context.getIncludeDirs()));
-        buildVariables.addSequenceVariable("quote_include_paths",
-            getSafePathStrings(context.getQuoteIncludeDirs()));
-        buildVariables.addSequenceVariable("system_include_paths",
-            getSafePathStrings(context.getSystemIncludeDirs()));
-      }
       // TODO(bazel-team): This needs to be before adding getUnfilteredCompilerOptions() and after
       // adding the warning flags until all toolchains are migrated; currently toolchains use the
       // unfiltered compiler options to inject include paths, which is superseded by the feature
       // configuration; on the other hand toolchains switch off warnings for the layering check
       // that will be re-added by the feature flags.
-      options.addAll(featureConfiguration.getCommandLine(getActionName(), buildVariables.build()));
+      addFilteredOptions(options,
+          featureConfiguration.getCommandLine(getActionName(), variables));
 
       options.addAll(toolchain.getUnfilteredCompilerOptions(features));
 
+      // Users don't expect the explicit copts to be filtered by coptsFilter, add them verbatim.
+      // Make sure these are added after the options from the feature configuration, so that
+      // those options can be overriden.
+      options.addAll(copts);
+
       // GCC gives randomized names to symbols which are defined in
       // an anonymous namespace but have external linkage.  To make
       // computation of these deterministic, we want to override the
@@ -1359,42 +1343,6 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable
       return options;
     }
 
-    /**
-     * Get the safe path strings for a list of paths to use in the build variables.
-     */
-    private Collection<String> getSafePathStrings(Collection<PathFragment> paths) {
-      ImmutableSet.Builder<String> result = ImmutableSet.builder();
-      for (PathFragment path : paths) {
-        result.add(path.getSafePathString());
-      }
-      return result.build();
-    }
-
-    /**
-     * Select .pcm inputs to pass on the command line depending on whether we are in pic or non-pic
-     * mode.
-     */
-    private Collection<String> getHeaderModulePaths() {
-      Collection<String> result = new LinkedHashSet<>();
-      NestedSet<Artifact> artifacts = featureConfiguration.isEnabled(
-          CppRuleClasses.HEADER_MODULE_INCLUDES_DEPENDENCIES)
-          ? context.getTopLevelHeaderModules()
-          : context.getAdditionalInputs();
-      for (Artifact artifact : artifacts) {
-        String filename = artifact.getFilename();
-        if (!filename.endsWith(".pcm")) {
-          continue;
-        }
-        // Depending on whether this specific compile action is pic or non-pic, select the
-        // corresponding header modules. Note that the compilation context might give us both
-        // from targets that are built in both modes.
-        if (usePic == filename.endsWith(".pic.pcm")) {
-          result.add(artifact.getExecPathString());
-        }
-      }
-      return result;
-    }
-
     // For each option in 'in', add it to 'out' unless it is matched by the 'coptsFilter' regexp.
     private void addFilteredOptions(List<String> out, List<String> in) {
       Iterables.addAll(out, Iterables.filter(in, coptsFilter));
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 216acb8ed6..3fdda79b5d 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
@@ -54,6 +54,7 @@ public class CppCompileActionBuilder {
   private final ActionOwner owner;
   private final List<String> features = new ArrayList<>();
   private CcToolchainFeatures.FeatureConfiguration featureConfiguration;
+  private CcToolchainFeatures.Variables variables;
   private final Artifact sourceFile;
   private final Label sourceLabel;
   private final NestedSetBuilder<Artifact> mandatoryInputsBuilder;
@@ -261,15 +262,15 @@ public class CppCompileActionBuilder {
     // Copying the collections is needed to make the builder reusable.
     if (fake) {
       return new FakeCppCompileAction(owner, ImmutableList.copyOf(features), featureConfiguration,
-          sourceFile, sourceLabel, realMandatoryInputsBuilder.build(), outputFile, tempOutputFile,
-          dotdFile, configuration, cppConfiguration, context, actionContext,
+          variables, sourceFile, sourceLabel, realMandatoryInputsBuilder.build(), outputFile,
+          tempOutputFile, dotdFile, configuration, cppConfiguration, context, actionContext,
           ImmutableList.copyOf(copts), ImmutableList.copyOf(pluginOpts),
           getNocoptPredicate(nocopts), extraSystemIncludePrefixes, fdoBuildStamp, ruleContext);
     } else {
       NestedSet<Artifact> realMandatoryInputs = realMandatoryInputsBuilder.build();
 
       return new CppCompileAction(owner, ImmutableList.copyOf(features), featureConfiguration,
-          sourceFile, sourceLabel, realMandatoryInputs, outputFile, dotdFile,
+          variables, sourceFile, sourceLabel, realMandatoryInputs, outputFile, dotdFile,
           gcnoFile, getDwoFile(outputFile, analysisEnvironment, cppConfiguration),
           optionalSourceFile, configuration, cppConfiguration, context,
           actionContext, ImmutableList.copyOf(copts),
@@ -289,6 +290,14 @@ public class CppCompileActionBuilder {
     this.featureConfiguration = featureConfiguration;
     return this;
   }
+  
+  /**
+   * Sets the feature build variables to be used for the action. 
+   */
+  public CppCompileActionBuilder setVariables(CcToolchainFeatures.Variables variables) {
+    this.variables = variables;
+    return this;
+  }
 
   public CppCompileActionBuilder setIncludeResolver(IncludeResolver includeResolver) {
     this.includeResolver = includeResolver;
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 398c276838..af342ea241 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
@@ -680,6 +680,8 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
     return result;
   }
 
+  // TODO(bazel-team): Remove this once bazel supports all crosstool flags through
+  // feature configuration, and all crosstools have been converted.
   private CToolchain addLegacyFeatures(CToolchain toolchain) {
     CToolchain.Builder toolchainBuilder = CToolchain.newBuilder();
     ImmutableSet.Builder<String> featuresBuilder = ImmutableSet.builder();
@@ -833,6 +835,68 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
             + "}",
             toolchainBuilder);
       }
+      if (!features.contains("fdo_instrument")) {
+        TextFormat.merge(""
+            + "feature {"
+            + "  name: 'fdo_instrument'"
+            + "  flag_set {"
+            + "    action: 'c-compile'"
+            + "    action: 'c++-compile'"
+            + "    action: 'c++-link'"
+            + "    flag_group {"
+            + "      flag: '-fprofile-generate=%{fdo_instrument_path}'"
+            + "    }"
+            + "    flag_group {"
+            + "      flag: '-fno-data-sections'"
+            + "    }"
+            + "  }"
+            + "}",
+            toolchainBuilder);
+      }
+      if (!features.contains("fdo_optimize")) {
+        TextFormat.merge(""
+            + "feature {"
+            + "  name: 'fdo_optimize'"
+            + "  flag_set {"
+            + "    action: 'c-compile'"
+            + "    action: 'c++-compile'"
+            + "    flag_group {"
+            + "      flag: '-fprofile-use=%{fdo_profile_path}'"
+            + "      flag: '-fprofile-correction'"
+            + "    }"
+            + "  }"
+            + "}",
+            toolchainBuilder);
+      }
+      if (!features.contains("autofdo")) {
+        TextFormat.merge(""
+            + "feature {"
+            + "  name: 'autofdo'"
+            + "  flag_set {"
+            + "    action: 'c-compile'"
+            + "    action: 'c++-compile'"
+            + "    flag_group {"
+            + "      flag: '-fauto-profile=%{fdo_profile_path}'"
+            + "      flag: '-fprofile-correction'"
+            + "    }"
+            + "  }"
+            + "}",
+            toolchainBuilder);
+      }
+      if (!features.contains("lipo")) {
+        TextFormat.merge(""
+            + "feature {"
+            + "  name: 'lipo'"
+            + "  flag_set {"
+            + "    action: 'c-compile'"
+            + "    action: 'c++-compile'"
+            + "    flag_group {"
+            + "      flag: '-fripa'"
+            + "    }"
+            + "  }"
+            + "}",
+            toolchainBuilder);
+      }
     } catch (ParseException e) {
       // Can only happen if we change the proto definition without changing our configuration above.
       throw new RuntimeException(e);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java
index 46988c91bb..b6e9f24407 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java
@@ -445,7 +445,8 @@ public final class CppLinkAction extends AbstractAction {
    */
   public static class Builder {
     // Builder-only
-    private final RuleContext ruleContext;
+    // Null when invoked from tests (e.g. via createTestBuilder).
+    @Nullable private final RuleContext ruleContext;
     private final AnalysisEnvironment analysisEnvironment;
     private final PathFragment outputPath;
 
@@ -508,7 +509,7 @@ public final class CppLinkAction extends AbstractAction {
      * @param configuration the configuration used to determine the tool chain
      *        and the default link options
      */
-    private Builder(RuleContext ruleContext, PathFragment outputPath,
+    private Builder(@Nullable RuleContext ruleContext, PathFragment outputPath,
         BuildConfiguration configuration, AnalysisEnvironment analysisEnvironment,
         CcToolchainProvider toolchain) {
       this.ruleContext = ruleContext;
@@ -642,7 +643,7 @@ public final class CppLinkAction extends AbstractAction {
               : null;
 
       LinkCommandLine linkCommandLine =
-          new LinkCommandLine.Builder(configuration, getOwner())
+          new LinkCommandLine.Builder(configuration, getOwner(), ruleContext)
               .setOutput(outputLibrary.getArtifact())
               .setInterfaceOutput(interfaceOutput)
               .setSymbolCountsOutput(symbolCountOutput)
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 08683fd4e0..4ea8355e8d 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
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.rules.cpp;
 
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.AnalysisEnvironment;
@@ -23,6 +24,7 @@ import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.packages.Type;
 import com.google.devtools.build.lib.rules.cpp.CcCompilationOutputs.Builder;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
@@ -37,6 +39,7 @@ import com.google.devtools.build.lib.vfs.PathFragment;
 
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.regex.Pattern;
 
@@ -290,6 +293,77 @@ public final class CppModel {
   }
 
   /**
+   * Get the safe path strings for a list of paths to use in the build variables.
+   */
+  private Collection<String> getSafePathStrings(Collection<PathFragment> paths) {
+    ImmutableSet.Builder<String> result = ImmutableSet.builder();
+    for (PathFragment path : paths) {
+      result.add(path.getSafePathString());
+    }
+    return result.build();
+  }
+
+  /**
+   * Select .pcm inputs to pass on the command line depending on whether we are in pic or non-pic
+   * mode.
+   */
+  private Collection<String> getHeaderModulePaths(CppCompileActionBuilder builder,
+      boolean usePic) {
+    Collection<String> result = new LinkedHashSet<>();
+    NestedSet<Artifact> artifacts = featureConfiguration.isEnabled(
+        CppRuleClasses.HEADER_MODULE_INCLUDES_DEPENDENCIES)
+        ? builder.getContext().getTopLevelHeaderModules()
+        : builder.getContext().getAdditionalInputs();
+    for (Artifact artifact : artifacts) {
+      String filename = artifact.getFilename();
+      if (!filename.endsWith(".pcm")) {
+        continue;
+      }
+      // Depending on whether this specific compile action is pic or non-pic, select the
+      // corresponding header modules. Note that the compilation context might give us both
+      // from targets that are built in both modes.
+      if (usePic == filename.endsWith(".pic.pcm")) {
+        result.add(artifact.getExecPathString());
+      }
+    }
+    return result;
+  }
+
+  private void setupBuildVariables(CppCompileActionBuilder builder,
+      boolean usePic, PathFragment ccRelativeName) {
+    CcToolchainFeatures.Variables.Builder buildVariables =
+        new CcToolchainFeatures.Variables.Builder();
+
+    CppModuleMap cppModuleMap = context.getCppModuleMap();
+    if (featureConfiguration.isEnabled(CppRuleClasses.MODULE_MAPS) && cppModuleMap != null) {
+      // If the feature is enabled and cppModuleMap is null, we are about to fail during analysis
+      // in any case, but don't crash.
+      buildVariables.addVariable("module_name", cppModuleMap.getName());
+      buildVariables.addVariable("module_map_file",
+          cppModuleMap.getArtifact().getExecPathString());
+    }
+    if (featureConfiguration.isEnabled(CppRuleClasses.USE_HEADER_MODULES)) {
+      buildVariables.addSequenceVariable("module_files", getHeaderModulePaths(builder, usePic));
+    }
+    if (featureConfiguration.isEnabled(CppRuleClasses.INCLUDE_PATHS)) {
+      buildVariables.addSequenceVariable("include_paths",
+          getSafePathStrings(context.getIncludeDirs()));
+      buildVariables.addSequenceVariable("quote_include_paths",
+          getSafePathStrings(context.getQuoteIncludeDirs()));
+      buildVariables.addSequenceVariable("system_include_paths",
+          getSafePathStrings(context.getSystemIncludeDirs()));
+    }
+
+    if (ccRelativeName != null) {
+      cppConfiguration.getFdoSupport().configureCompilation(builder, buildVariables, ruleContext,
+          ccRelativeName, usePic, featureConfiguration, cppConfiguration);
+    }
+
+    CcToolchainFeatures.Variables variables = buildVariables.build();
+    builder.setVariables(variables);
+  }
+
+  /**
    * Constructs the C++ compiler actions. It generally creates one action for every specified source
    * file. It takes into account LIPO, fake-ness, coverage, and PIC, in addition to using the
    * settings specified on the current object. This method should only be called once.
@@ -338,6 +412,7 @@ public final class CppModel {
         .setDotdFile(outputName, ".h.d")
         // If we generate pic actions, we prefer the header actions to use the pic artifacts.
         .setPicMode(this.getGeneratePicActions());
+    setupBuildVariables(builder, this.getGeneratePicActions(), null);
     semantics.finalizeCompileActionBuilder(ruleContext, builder);
     CppCompileAction compileAction = builder.build();
     env.registerAction(compileAction);
@@ -354,12 +429,11 @@ public final class CppModel {
       String dependencyFileExtension,
       boolean addObject) {
     PathFragment ccRelativeName = semantics.getEffectiveSourcePath(sourceArtifact);
-    LipoContextProvider lipoProvider = null;
     if (cppConfiguration.isLipoOptimization()) {
       // TODO(bazel-team): we shouldn't be needing this, merging context with the binary
       // is a superset of necessary information.
-      lipoProvider = Preconditions.checkNotNull(CppHelper.getLipoContextProvider(ruleContext),
-          outputName);
+      LipoContextProvider lipoProvider =
+          Preconditions.checkNotNull(CppHelper.getLipoContextProvider(ruleContext), outputName);
       builder.setContext(CppCompilationContext.mergeForLipo(lipoProvider.getLipoContext(),
           context));
     }
@@ -375,6 +449,7 @@ public final class CppModel {
           .setOutputFile(outputFile)
           .setDotdFile(outputName, dependencyFileExtension)
           .setTempOutputFile(tempOutputName);
+      setupBuildVariables(builder, getGeneratePicActions(), ccRelativeName);
       semantics.finalizeCompileActionBuilder(ruleContext, builder);
       CppCompileAction action = builder.build();
       env.registerAction(action);
@@ -392,13 +467,12 @@ public final class CppModel {
       if (generatePicAction) {
         CppCompileActionBuilder picBuilder =
             copyAsPicBuilder(builder, outputName, outputExtension, dependencyFileExtension);
-        cppConfiguration.getFdoSupport().configureCompilation(picBuilder, ruleContext, env,
-            ruleContext.getLabel(), ccRelativeName, nocopts, /*usePic=*/true,
-            lipoProvider);
+        setupBuildVariables(picBuilder, /*usePic=*/true, ccRelativeName);
 
         if (maySaveTemps) {
           result.addTemps(
-              createTempsActions(sourceArtifact, outputName, picBuilder, /*usePic=*/true));
+              createTempsActions(sourceArtifact, outputName, picBuilder, /*usePic=*/true,
+                ccRelativeName));
         }
 
         if (isCodeCoverageEnabled()) {
@@ -425,13 +499,12 @@ public final class CppModel {
             .setOutputFile(ruleContext.getRelatedArtifact(outputName, outputExtension))
             .setDotdFile(outputName, dependencyFileExtension);
         // Create non-PIC compile actions
-        cppConfiguration.getFdoSupport().configureCompilation(builder, ruleContext, env,
-            ruleContext.getLabel(), ccRelativeName, nocopts, /*usePic=*/false,
-            lipoProvider);
+        setupBuildVariables(builder, /*usePic=*/false, ccRelativeName);
 
         if (maySaveTemps) {
           result.addTemps(
-              createTempsActions(sourceArtifact, outputName, builder, /*usePic=*/false));
+              createTempsActions(sourceArtifact, outputName, builder, /*usePic=*/false,
+                ccRelativeName));
         }
 
         if (!cppConfiguration.isLipoOptimization() && isCodeCoverageEnabled()) {
@@ -630,7 +703,7 @@ public final class CppModel {
    * Create the actions for "--save_temps".
    */
   private ImmutableList<Artifact> createTempsActions(Artifact source, PathFragment outputName,
-      CppCompileActionBuilder builder, boolean usePic) {
+      CppCompileActionBuilder builder, boolean usePic, PathFragment ccRelativeName) {
     if (!cppConfiguration.getSaveTemps()) {
       return ImmutableList.of();
     }
@@ -646,7 +719,9 @@ public final class CppModel {
     String iExt = isCFile ? ".i" : ".ii";
     String picExt = usePic ? ".pic" : "";
     CppCompileActionBuilder dBuilder = new CppCompileActionBuilder(builder);
+    setupBuildVariables(dBuilder, usePic, ccRelativeName);
     CppCompileActionBuilder sdBuilder = new CppCompileActionBuilder(builder);
+    setupBuildVariables(sdBuilder, usePic, ccRelativeName);
 
     dBuilder
         .setOutputFile(ruleContext.getRelatedArtifact(outputName, picExt + iExt))
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 c6b3a216d6..98a6568a9f 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
@@ -145,4 +145,24 @@ public class CppRuleClasses {
    * A string constant for the include_paths feature.
    */
   public static final String INCLUDE_PATHS = "include_paths";
+
+  /**
+   * A string constant for the fdo_instrument feature.
+   */
+  public static final String FDO_INSTRUMENT = "fdo_instrument";
+
+  /**
+   * A string constant for the fdo_optimize feature.
+   */
+  public static final String FDO_OPTIMIZE = "fdo_optimize";
+
+  /**
+   * A string constant for the autofdo feature.
+   */
+  public static final String AUTOFDO = "autofdo";
+
+  /**
+   * A string constant for the lipo feature.
+   */
+  public static final String LIPO = "lipo";
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java
index 994d93ad35..a7ed3da2df 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java
@@ -62,6 +62,7 @@ public class FakeCppCompileAction extends CppCompileAction {
   FakeCppCompileAction(ActionOwner owner,
       ImmutableList<String> features,
       FeatureConfiguration featureConfiguration,
+      CcToolchainFeatures.Variables variables,
       Artifact sourceFile,
       Label sourceLabel,
       NestedSet<Artifact> mandatoryInputs,
@@ -78,8 +79,8 @@ public class FakeCppCompileAction extends CppCompileAction {
       ImmutableList<PathFragment> extraSystemIncludePrefixes,
       @Nullable String fdoBuildStamp,
       RuleContext ruleContext) {
-    super(owner, features, featureConfiguration, sourceFile, sourceLabel, mandatoryInputs,
-        outputFile, dotdFile, null, null, null,
+    super(owner, features, featureConfiguration, variables, sourceFile, sourceLabel,
+        mandatoryInputs, outputFile, dotdFile, null, null, null,
         configuration, cppConfiguration,
         // We only allow inclusion of header files explicitly declared in
         // "srcs", so we only use declaredIncludeSrcs, not declaredIncludeDirs.
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 c10eea9edc..6474d7f01d 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
@@ -16,7 +16,6 @@ package com.google.devtools.build.lib.rules.cpp;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
-import com.google.common.base.Predicate;
 import com.google.common.collect.ImmutableCollection;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMultimap;
@@ -31,6 +30,7 @@ import com.google.devtools.build.lib.analysis.AnalysisEnvironment;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadHostile;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
+import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
 import com.google.devtools.build.lib.skyframe.FileValue;
 import com.google.devtools.build.lib.skyframe.PrecomputedValue;
 import com.google.devtools.build.lib.syntax.Label;
@@ -44,10 +44,8 @@ import com.google.devtools.build.skyframe.SkyFunction;
 
 import java.io.IOException;
 import java.io.Serializable;
-import java.util.ArrayList;
 import java.util.Collection;
-import java.util.List;
-import java.util.regex.Pattern;
+import java.util.HashSet;
 import java.util.zip.ZipException;
 
 /**
@@ -440,16 +438,18 @@ public class FdoSupport implements Serializable {
   }
 
   /**
-   * Configures a compile action builder by adding command line options and
+   * Configures a compile action builder by setting up command line options and
    * auxiliary inputs according to the FDO configuration. This method does
    * nothing If FDO is disabled.
    */
   @ThreadSafe
-  public void configureCompilation(CppCompileActionBuilder builder, RuleContext ruleContext,
-      AnalysisEnvironment env, Label lipoLabel, PathFragment sourceName, final Pattern nocopts,
-      boolean usePic, LipoContextProvider lipoInputProvider) {
+  public void configureCompilation(CppCompileActionBuilder builder,
+      CcToolchainFeatures.Variables.Builder buildVariables, RuleContext ruleContext,
+      PathFragment sourceName, boolean usePic, FeatureConfiguration featureConfiguration,
+      CppConfiguration cppConfiguration) {
     // It is a bug if this method is called with useLipo if lipo is disabled. However, it is legal
     // if is is called with !useLipo, even though lipo is enabled.
+    LipoContextProvider lipoInputProvider = CppHelper.getLipoContextProvider(ruleContext);
     Preconditions.checkArgument(lipoInputProvider == null || isLipoEnabled());
 
     // FDO is disabled -> do nothing.
@@ -457,55 +457,62 @@ public class FdoSupport implements Serializable {
       return;
     }
 
-    List<String> fdoCopts = new ArrayList<>();
-    // Instrumentation phase
-    if (fdoInstrument != null) {
-      fdoCopts.add("-fprofile-generate=" + fdoInstrument.getPathString());
-      fdoCopts.add("-fno-data-sections");
-      if (lipoMode != LipoMode.OFF) {
-        fdoCopts.add("-fripa");
-      }
+    if (featureConfiguration.isEnabled(CppRuleClasses.FDO_INSTRUMENT)) {
+      buildVariables.addVariable("fdo_instrument_path",
+          fdoInstrument.getPathString());
     }
 
     // Optimization phase
     if (fdoRoot != null) {
+      AnalysisEnvironment env = ruleContext.getAnalysisEnvironment();
       // Declare dependency on contents of zip file.
       if (env.getSkyframeEnv().valuesMissing()) {
         return;
       }
       Iterable<Artifact> auxiliaryInputs = getAuxiliaryInputs(
-          ruleContext, env, lipoLabel, sourceName, usePic, lipoInputProvider);
+          ruleContext, env, sourceName, usePic, lipoInputProvider);
       builder.addMandatoryInputs(auxiliaryInputs);
       if (!Iterables.isEmpty(auxiliaryInputs)) {
-        if (useAutoFdo) {
-          fdoCopts.add("-fauto-profile=" + getAutoProfilePath().getPathString());
-        } else {
-          fdoCopts.add("-fprofile-use=" + fdoRootExecPath);
+        if (featureConfiguration.isEnabled(CppRuleClasses.AUTOFDO)) {
+          buildVariables.addVariable("fdo_profile_path",
+              getAutoProfilePath().getPathString());
         }
-        fdoCopts.add("-fprofile-correction");
-        if (lipoInputProvider != null) {
-          fdoCopts.add("-fripa");
+        if (featureConfiguration.isEnabled(CppRuleClasses.FDO_OPTIMIZE)) {
+          buildVariables.addVariable("fdo_profile_path",
+              fdoRootExecPath.getPathString());
         }
-      }
-    }
-    Iterable<String> filteredCopts = fdoCopts;
-    if (nocopts != null) {
-      // Filter fdoCopts with nocopts if they exist.
-      filteredCopts = Iterables.filter(fdoCopts, new Predicate<String>() {
-        @Override
-        public boolean apply(String copt) {
-          return !nocopts.matcher(copt).matches();
+      } else {
+        // TODO(bazel-team): Remove this workaround once the feature configuration
+        // supports per-file feature enabling.
+        // The feature configuration was created based on blaze options, which
+        // enabled the fdo optimize options since the fdoRoot was set.
+        // In this case the source file doesn't have an associated profile,
+        // so we need to disable these features so that we don't add the FDO options.
+        // However, the list of features is immutable and set on the CppModel.
+        // Create a new feature config here, enabling just what we want,
+        // and set it in this builder.
+        Collection<String> featureNames = cppConfiguration.getFeatures().getFeatureNames();
+        Collection<String> newFeatureNames = new HashSet<>();
+        for (String name : featureNames) {
+          if (featureConfiguration.isEnabled(name)) {
+            newFeatureNames.add(name);
+          }
         }
-      });
+        newFeatureNames.remove(CppRuleClasses.FDO_OPTIMIZE);
+        newFeatureNames.remove(CppRuleClasses.AUTOFDO);
+        newFeatureNames.remove(CppRuleClasses.LIPO);
+        FeatureConfiguration newFeatureConfiguration =
+            cppConfiguration.getFeatures().getFeatureConfiguration(newFeatureNames);
+        builder.setFeatureConfiguration(newFeatureConfiguration);
+      }
     }
-    builder.addCopts(0, filteredCopts);
   }
 
   /**
    * Returns the auxiliary files that need to be added to the {@link CppCompileAction}.
    */
   private Iterable<Artifact> getAuxiliaryInputs(
-      RuleContext ruleContext, AnalysisEnvironment env, Label lipoLabel, PathFragment sourceName,
+      RuleContext ruleContext, AnalysisEnvironment env, PathFragment sourceName,
       boolean usePic, LipoContextProvider lipoContextProvider) {
     // If --fdo_optimize was not specified, we don't have any additional inputs.
     if (fdoProfile == null) {
@@ -527,6 +534,7 @@ public class FdoSupport implements Serializable {
       PathFragment objectName =
           FileSystemUtils.replaceExtension(sourceName, usePic ? ".pic.o" : ".o");
 
+      Label lipoLabel = ruleContext.getLabel();
       auxiliaryInputs.addAll(
           getGcdaArtifactsForObjectFileName(ruleContext, env, objectName, lipoLabel));
 
@@ -634,14 +642,17 @@ public class FdoSupport implements Serializable {
   }
 
   /**
-   * Returns an immutable list of command line arguments to add to the linker
-   * command line. If FDO is disabled, and empty list is returned.
+   * Adds the FDO profile output path to the variable builder.
+   * If FDO is disabled, no build variable is added.
    */
   @ThreadSafe
-  public ImmutableList<String> getLinkOptions() {
-    return fdoInstrument != null
-        ? ImmutableList.of("-fprofile-generate=" + fdoInstrument.getPathString())
-        : ImmutableList.<String>of();
+  public void getLinkOptions(FeatureConfiguration featureConfiguration,
+      CcToolchainFeatures.Variables.Builder buildVariables
+      ) {
+    if (featureConfiguration.isEnabled(CppRuleClasses.FDO_INSTRUMENT)) {
+      buildVariables.addVariable("fdo_instrument_path",
+          fdoInstrument.getPathString());
+    }
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java
index e597907015..66b2722e83 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java
@@ -31,6 +31,7 @@ import com.google.devtools.build.lib.analysis.actions.CommandLine;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.collect.CollectionUtils;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
+import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
 import com.google.devtools.build.lib.rules.cpp.Link.LinkStaticness;
 import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType;
 import com.google.devtools.build.lib.syntax.Label;
@@ -58,6 +59,9 @@ public final class LinkCommandLine extends CommandLine {
   private final BuildConfiguration configuration;
   private final CppConfiguration cppConfiguration;
   private final ActionOwner owner;
+  private final CcToolchainFeatures.Variables variables;
+  // The feature config can be null for tests.
+  @Nullable private final FeatureConfiguration featureConfiguration;
   @Nullable private final Artifact output;
   @Nullable private final Artifact interfaceOutput;
   @Nullable private final Artifact symbolCountsOutput;
@@ -77,6 +81,12 @@ public final class LinkCommandLine extends CommandLine {
   @Nullable private final Artifact paramFile;
   @Nullable private final Artifact interfaceSoBuilder;
 
+  /**
+   * A string constant for the c++ link action, used to access the feature
+   * configuration.
+   */
+  public static final String CPP_LINK = "c++-link";
+
   private LinkCommandLine(
       BuildConfiguration configuration,
       ActionOwner owner,
@@ -97,7 +107,9 @@ public final class LinkCommandLine extends CommandLine {
       boolean useTestOnlyFlags,
       boolean needWholeArchive,
       @Nullable Artifact paramFile,
-      Artifact interfaceSoBuilder) {
+      Artifact interfaceSoBuilder,
+      CcToolchainFeatures.Variables variables,
+      @Nullable FeatureConfiguration featureConfiguration) {
     Preconditions.checkArgument(linkTargetType != LinkTargetType.INTERFACE_DYNAMIC_LIBRARY,
         "you can't link an interface dynamic library directly");
     if (linkTargetType != LinkTargetType.DYNAMIC_LIBRARY) {
@@ -123,6 +135,8 @@ public final class LinkCommandLine extends CommandLine {
 
     this.configuration = Preconditions.checkNotNull(configuration);
     this.cppConfiguration = configuration.getFragment(CppConfiguration.class);
+    this.variables = variables;
+    this.featureConfiguration = featureConfiguration;
     this.owner = Preconditions.checkNotNull(owner);
     this.output = output;
     this.interfaceOutput = interfaceOutput;
@@ -658,7 +672,10 @@ public final class LinkCommandLine extends CommandLine {
     }
 
     argv.addAll(cppConfiguration.getLinkOptions());
-    argv.addAll(cppConfiguration.getFdoSupport().getLinkOptions());
+    // The feature config can be null for tests.
+    if (featureConfiguration != null) {
+      argv.addAll(featureConfiguration.getCommandLine(CPP_LINK, variables));
+    }
   }
 
   private static boolean isDynamicLibrary(LinkerInput linkInput) {
@@ -933,6 +950,7 @@ public final class LinkCommandLine extends CommandLine {
 
     private final BuildConfiguration configuration;
     private final ActionOwner owner;
+    @Nullable private final RuleContext ruleContext;
 
     @Nullable private Artifact output;
     @Nullable private Artifact interfaceOutput;
@@ -953,13 +971,18 @@ public final class LinkCommandLine extends CommandLine {
     @Nullable private Artifact paramFile;
     @Nullable private Artifact interfaceSoBuilder;
 
-    public Builder(BuildConfiguration configuration, ActionOwner owner) {
+    // This interface is needed to support tests that don't create a
+    // ruleContext, in which case the configuration and action owner
+    // cannot be accessed off of the give ruleContext.
+    public Builder(BuildConfiguration configuration, ActionOwner owner,
+        @Nullable RuleContext ruleContext) {
       this.configuration = configuration;
       this.owner = owner;
+      this.ruleContext = ruleContext;
     }
 
     public Builder(RuleContext ruleContext) {
-      this(ruleContext.getConfiguration(), ruleContext.getActionOwner());
+      this(ruleContext.getConfiguration(), ruleContext.getActionOwner(), ruleContext);
     }
 
     public LinkCommandLine build() {
@@ -970,6 +993,17 @@ public final class LinkCommandLine extends CommandLine {
         actualLinkstampCompileOptions = ImmutableList.copyOf(
             Iterables.concat(DEFAULT_LINKSTAMP_OPTIONS, linkstampCompileOptions));
       }
+      CcToolchainFeatures.Variables variables = null;
+      FeatureConfiguration featureConfiguration = null;
+      // The ruleContext can be null for some tests.
+      if (ruleContext != null) {
+        featureConfiguration = CcCommon.configureFeatures(ruleContext);
+        CcToolchainFeatures.Variables.Builder buildVariables =
+            new CcToolchainFeatures.Variables.Builder();
+        CppConfiguration cppConfiguration = configuration.getFragment(CppConfiguration.class);
+        cppConfiguration.getFdoSupport().getLinkOptions(featureConfiguration, buildVariables);
+        variables = buildVariables.build();
+      }
       return new LinkCommandLine(
           configuration,
           owner,
@@ -990,7 +1024,9 @@ public final class LinkCommandLine extends CommandLine {
           useTestOnlyFlags,
           needWholeArchive,
           paramFile,
-          interfaceSoBuilder);
+          interfaceSoBuilder,
+          variables,
+          featureConfiguration);
     }
 
     /**