From a381b754c15f4752243b25892363eb522f65a508 Mon Sep 17 00:00:00 2001 From: Manuel Klimek Date: Tue, 11 Aug 2015 18:50:39 +0000 Subject: Resolve TODO now that the feature configuration supports conditional expansion of flag sets. -- MOS_MIGRATED_REVID=100400672 --- .../build/lib/rules/cpp/CppConfiguration.java | 121 ++++++++++++--------- .../devtools/build/lib/rules/cpp/FdoSupport.java | 24 ---- 2 files changed, 67 insertions(+), 78 deletions(-) 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 8d66f01753..c999e7fad7 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 @@ -719,69 +719,81 @@ 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: '-Xgcc-only=-fprofile-generate=%{fdo_instrument_path}'" - + " flag: '-Xclang-only=-fprofile-instr-generate=%{fdo_instrument_path}'" - + " }" - + " flag_group {" - + " flag: '-fno-data-sections'" - + " }" - + " }" - + "}", + TextFormat.merge( + "" + + "feature {" + + " name: 'fdo_instrument'" + + " provides: 'profile'" + + " flag_set {" + + " action: 'c-compile'" + + " action: 'c++-compile'" + + " action: 'c++-link'" + + " flag_group {" + + " flag: '-Xgcc-only=-fprofile-generate=%{fdo_instrument_path}'" + + " flag: '-Xclang-only=-fprofile-instr-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: '-Xgcc-only=-fprofile-use=%{fdo_profile_path}'" - + " flag: '-Xclang-only=-fprofile-instr-use=%{fdo_profile_path}'" - + " flag: '-Xclang-only=-Wno-profile-instr-unprofiled'" - + " flag: '-Xclang-only=-Wno-profile-instr-out-of-date'" - + " flag: '-fprofile-correction'" - + " }" - + " }" - + "}", + TextFormat.merge( + "" + + "feature {" + + " name: 'fdo_optimize'" + + " provides: 'profile'" + + " flag_set {" + + " action: 'c-compile'" + + " action: 'c++-compile'" + + " expand_if_all_available: 'fdo_profile_path'" + + " flag_group {" + + " flag: '-Xgcc-only=-fprofile-use=%{fdo_profile_path}'" + + " flag: '-Xclang-only=-fprofile-instr-use=%{fdo_profile_path}'" + + " flag: '-Xclang-only=-Wno-profile-instr-unprofiled'" + + " flag: '-Xclang-only=-Wno-profile-instr-out-of-date'" + + " 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'" - + " }" - + " }" - + "}", + TextFormat.merge( + "" + + "feature {" + + " name: 'autofdo'" + + " provides: 'profile'" + + " flag_set {" + + " action: 'c-compile'" + + " action: 'c++-compile'" + + " expand_if_all_available: 'fdo_profile_path'" + + " 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'" - + " }" - + " }" - + "}", + TextFormat.merge( + "" + + "feature {" + + " name: 'lipo'" + + " requires { feature: 'autofdo' }" + + " requires { feature: 'fdo_optimize' }" + + " requires { feature: 'fdo_instrument' }" + + " flag_set {" + + " action: 'c-compile'" + + " action: 'c++-compile'" + + " flag_group {" + + " flag: '-fripa'" + + " }" + + " }" + + "}", toolchainBuilder); } if (!features.contains("coverage")) { @@ -789,6 +801,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment { "" + "feature {" + " name: 'coverage'" + + " provides: 'profile'" + " flag_set {" + " action: 'preprocess-assemble'" + " action: 'c-compile'" 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 a2339c2b2a..1ab6a8b5f4 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 @@ -44,7 +44,6 @@ import com.google.devtools.build.skyframe.SkyFunction; import java.io.IOException; import java.util.Collection; -import java.util.HashSet; import java.util.zip.ZipException; /** @@ -502,29 +501,6 @@ public class FdoSupport { fdoRootExecPath.getPathString()); } } - } 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 featureNames = cppConfiguration.getFeatures().getFeatureNames(); - Collection 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); } } } -- cgit v1.2.3