Skip to content

Commit

Permalink
Always use transformed cpu value when selecting C++ toolchain
Browse files Browse the repository at this point in the history
As a result, I could rewire CToolchain selection in
CppConfigurationLoader.selectCcToolchainLabel to
use the same entry point method as we do in the
CppConfigurationLoader.createParameters and in CcToolchain.create, and simplify
the code a bit.

#6072

RELNOTES: None.
PiperOrigin-RevId: 213258694
  • Loading branch information
hlopko authored and Copybara-Service committed Sep 17, 2018
1 parent ce15459 commit bcaae47
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 85 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
// limitations under the License.
package com.google.devtools.build.lib.rules.cpp;

import com.google.common.base.Function;
import com.google.common.base.Preconditions;
import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
import com.google.devtools.build.lib.util.StringUtil;
Expand Down Expand Up @@ -42,18 +41,16 @@ public class CToolchainSelectionUtils {
* @param cpuOption value of the --cpu option
* @param compilerOption value of the --compiler option
* @param proto content of the CROSSTOOL file
* @param cpuTransformer because life is hard
* @return selected CToolchain or throws InvalidConfigurationException when not found. Never
* returns null.
*/
public static CToolchain selectCToolchain(
static CToolchain selectCToolchain(
@Nullable String identifierAttribute,
@Nullable String cpuAttribute,
@Nullable String compilerAttribute,
String cpuOption,
@Nullable String compilerOption,
CrosstoolRelease proto,
Function<String, String> cpuTransformer)
CrosstoolRelease proto)
throws InvalidConfigurationException {
String identifierAttributeOrNull = StringUtil.emptyToNull(identifierAttribute);
String cpuAttributeOrNull = StringUtil.emptyToNull(cpuAttribute);
Expand All @@ -68,8 +65,7 @@ public static CToolchain selectCToolchain(
compilerAttributeOrNull,
cpuOption,
compilerOptionOrNull,
proto,
cpuTransformer);
proto);
}

private static CToolchain selectCToolchainNoEmptyStrings(
Expand All @@ -78,8 +74,7 @@ private static CToolchain selectCToolchainNoEmptyStrings(
String compilerAttribute,
String cpuOption,
String compilerOption,
CrosstoolRelease proto,
Function<String, String> cpuTransformer)
CrosstoolRelease proto)
throws InvalidConfigurationException {
CToolchain cToolchain = null;
// Use the identifier to find the CToolchain from the CROSSTOOL (this is the way how
Expand All @@ -93,9 +88,7 @@ private static CToolchain selectCToolchainNoEmptyStrings(
try {
cToolchain =
selectToolchainUsingCpuAndMaybeCompiler(
proto,
new CrosstoolConfigurationIdentifier(cpuAttribute, compilerAttribute),
cpuTransformer);
proto, new CrosstoolConfigurationIdentifier(cpuAttribute, compilerAttribute));
} catch (InvalidConfigurationException e) {
// We couldn't find the CToolchain using attributes, let's catch the exception and try
// with options. It's safe to ignore the exception here, since if it was caused by
Expand All @@ -107,9 +100,7 @@ private static CToolchain selectCToolchainNoEmptyStrings(
// it using --cpu/--compiler options (the legacy way, doesn't work with platforms).
cToolchain =
selectToolchainUsingCpuAndMaybeCompiler(
proto,
new CrosstoolConfigurationIdentifier(cpuOption, compilerOption),
cpuTransformer);
proto, new CrosstoolConfigurationIdentifier(cpuOption, compilerOption));
}
return cToolchain;
}
Expand Down Expand Up @@ -172,10 +163,8 @@ private static void checkToolchain(String selectedIdentifier)
* @throws InvalidConfigurationException if no matching toolchain can be found, or if the input
* parameters do not obey the constraints described above
*/
public static CToolchain selectToolchainUsingCpuAndMaybeCompiler(
CrosstoolRelease release,
CrosstoolConfigurationIdentifier config,
Function<String, String> cpuTransformer)
private static CToolchain selectToolchainUsingCpuAndMaybeCompiler(
CrosstoolRelease release, CrosstoolConfigurationIdentifier config)
throws InvalidConfigurationException {
if (config.getCompiler() != null) {
ArrayList<CToolchain> candidateToolchains = new ArrayList<>();
Expand Down Expand Up @@ -208,11 +197,8 @@ public static CToolchain selectToolchainUsingCpuAndMaybeCompiler(
}
}
String selectedIdentifier = null;
// We use fake CPU values to allow cross-platform builds for other languages that use the
// C++ toolchain. Translate to the actual target architecture.
String desiredCpu = cpuTransformer.apply(config.getCpu());
for (CrosstoolConfig.DefaultCpuToolchain selector : release.getDefaultToolchainList()) {
if (selector.getCpu().equals(desiredCpu)) {
if (selector.getCpu().equals(config.getCpu())) {
selectedIdentifier = selector.getToolchainIdentifier();
break;
}
Expand All @@ -228,7 +214,7 @@ public static CToolchain selectToolchainUsingCpuAndMaybeCompiler(
}
throw new InvalidConfigurationException(
"No default_toolchain found for cpu '"
+ desiredCpu
+ config.getCpu()
+ "'. Valid cpus are: [\n"
+ cpuBuilder
+ "]");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -781,8 +781,7 @@ private CToolchain getToolchainFromAttributes(
compiler,
cppConfiguration.getTransformedCpuFromOptions(),
cppConfiguration.getCompilerFromOptions(),
cppConfiguration.getCrosstoolFile().getProto(),
cppConfiguration.getCpuTransformer());
cppConfiguration.getCrosstoolFile().getProto());
} catch (InvalidConfigurationException e) {
ruleContext.throwWithRuleError(
String.format("Error while selecting cc_toolchain: %s", e.getMessage()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package com.google.devtools.build.lib.rules.cpp;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Function;
import com.google.common.base.Preconditions;
import com.google.common.base.Verify;
import com.google.common.collect.ImmutableList;
Expand Down Expand Up @@ -214,7 +213,6 @@ public String toString() {
private final ImmutableList<String> ltobackendOptions;

private final CppOptions cppOptions;
private final CpuTransformer cpuTransformerEnum;

// The dynamic mode for linking.
private final boolean stripBinaries;
Expand Down Expand Up @@ -293,7 +291,6 @@ static CppConfiguration create(CppConfigurationParameters params)
ImmutableList.copyOf(cppOptions.ltoindexoptList),
ImmutableList.copyOf(cppOptions.ltobackendoptList),
cppOptions,
params.cpuTransformer,
(cppOptions.stripBinaries == StripMode.ALWAYS
|| (cppOptions.stripBinaries == StripMode.SOMETIMES
&& compilationMode == CompilationMode.FASTBUILD)),
Expand Down Expand Up @@ -328,7 +325,6 @@ private CppConfiguration(
ImmutableList<String> ltoindexOptions,
ImmutableList<String> ltobackendOptions,
CppOptions cppOptions,
CpuTransformer cpuTransformerEnum,
boolean stripBinaries,
CompilationMode compilationMode,
boolean shouldProvideMakeVariables,
Expand Down Expand Up @@ -358,7 +354,6 @@ private CppConfiguration(
this.ltoindexOptions = ltoindexOptions;
this.ltobackendOptions = ltobackendOptions;
this.cppOptions = cppOptions;
this.cpuTransformerEnum = cpuTransformerEnum;
this.stripBinaries = stripBinaries;
this.compilationMode = compilationMode;
this.shouldProvideMakeVariables = shouldProvideMakeVariables;
Expand Down Expand Up @@ -405,11 +400,6 @@ public Label getCrosstoolTop() {
return crosstoolTop;
}

/** Returns the transformer that should be applied to cpu names in toolchain selection. */
public Function<String, String> getCpuTransformer() {
return cpuTransformerEnum.getTransformer();
}

/**
* Returns the path of the crosstool.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ public static class CppConfigurationParameters {
protected final PathFragment fdoPath;
protected final Label fdoOptimizeLabel;
protected final Label sysrootLabel;
protected final CpuTransformer cpuTransformer;
protected final CcToolchainConfigInfo ccToolchainConfigInfo;
protected final String transformedCpu;
protected final String compiler;
Expand All @@ -106,7 +105,6 @@ public static class CppConfigurationParameters {
Label ccToolchainLabel,
Label stlLabel,
Label sysrootLabel,
CpuTransformer cpuTransformer,
CcToolchainConfigInfo ccToolchainConfigInfo) {
this.transformedCpu = transformedCpu;
this.compiler = compiler;
Expand All @@ -120,7 +118,6 @@ public static class CppConfigurationParameters {
this.ccToolchainLabel = ccToolchainLabel;
this.stlLabel = stlLabel;
this.sysrootLabel = sysrootLabel;
this.cpuTransformer = cpuTransformer;
this.ccToolchainConfigInfo = ccToolchainConfigInfo;
}
}
Expand Down Expand Up @@ -164,7 +161,7 @@ protected CppConfigurationParameters createParameters(
transformedCpu + (cppOptions.cppCompiler == null ? "" : ("|" + cppOptions.cppCompiler));
Label ccToolchainLabel =
selectCcToolchainLabel(
options, cppOptions, crosstoolTopLabel, (Rule) crosstoolTop, file, transformedCpu, key);
cppOptions, crosstoolTopLabel, (Rule) crosstoolTop, file, transformedCpu, key);

Target ccToolchain = loadCcToolchainTarget(env, ccToolchainLabel);
if (ccToolchain == null) {
Expand All @@ -190,8 +187,7 @@ protected CppConfigurationParameters createParameters(
compilerAttribute,
transformedCpu,
cppOptions.cppCompiler,
file.getProto(),
cpuTransformer.getTransformer());
file.getProto());

cToolchain =
CppToolchainInfo.addLegacyFeatures(
Expand Down Expand Up @@ -240,7 +236,6 @@ protected CppConfigurationParameters createParameters(
ccToolchainLabel,
stlLabel,
sysrootLabel,
cpuTransformer,
ccToolchainConfigInfo);
}

Expand All @@ -265,7 +260,6 @@ private Target loadCcToolchainTarget(ConfigurationEnvironment env, Label ccToolc
}

private Label selectCcToolchainLabel(
BuildOptions options,
CppOptions cppOptions,
Label crosstoolTopLabel,
Rule crosstoolTop,
Expand All @@ -277,8 +271,9 @@ private Label selectCcToolchainLabel(
String.format(
"cc_toolchain_suite '%s' does not contain a toolchain for CPU '%s'",
crosstoolTopLabel, transformedCpu);
if (cppOptions.cppCompiler != null) {
errorMessage = errorMessage + " and compiler " + cppOptions.cppCompiler;
String compiler = cppOptions.cppCompiler;
if (compiler != null) {
errorMessage = errorMessage + " and compiler " + compiler;
}

Map<String, Label> toolchains =
Expand All @@ -292,8 +287,13 @@ private Label selectCcToolchainLabel(
// present). Then we use the toolchain.target_cpu|toolchain.compiler key to get the
// cc_toolchain label.
CToolchain toolchain =
CrosstoolConfigurationLoader.selectToolchain(
file.getProto(), options, cpuTransformer.getTransformer());
CToolchainSelectionUtils.selectCToolchain(
/* identifierAttribute= */ null,
/* cpuAttribute= */ null,
/* compilerAttribute= */ null,
transformedCpu,
compiler,
file.getProto());
ccToolchainLabel = toolchains.get(toolchain.getTargetCpu() + "|" + toolchain.getCompiler());
if (cppOptions.disableCcToolchainFromCrosstool) {
throw new InvalidConfigurationException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@
package com.google.devtools.build.lib.rules.cpp;

import com.google.common.base.Preconditions;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Options;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CToolchain;
import java.util.Objects;

Expand All @@ -41,16 +38,6 @@ public final class CrosstoolConfigurationIdentifier implements CrosstoolConfigur
this.compiler = compiler;
}

/**
* Creates a new crosstool configuration from the given crosstool release and
* configuration options.
*/
public static CrosstoolConfigurationIdentifier fromOptions(BuildOptions buildOptions) {
Options options = buildOptions.get(BuildConfiguration.Options.class);
CppOptions cppOptions = buildOptions.get(CppOptions.class);
return new CrosstoolConfigurationIdentifier(options.cpu, cppOptions.cppCompiler);
}

public static CrosstoolConfigurationIdentifier fromToolchain(CToolchain toolchain) {
return new CrosstoolConfigurationIdentifier(toolchain.getTargetCpu(), toolchain.getCompiler());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,10 @@
import static java.nio.charset.StandardCharsets.UTF_8;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Function;
import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;
import com.google.common.io.BaseEncoding;
import com.google.devtools.build.lib.analysis.RedirectChaser;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.ConfigurationEnvironment;
import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
import com.google.devtools.build.lib.cmdline.Label;
Expand Down Expand Up @@ -271,26 +269,4 @@ public static CrosstoolConfigurationLoader.CrosstoolFile readCrosstool(
throw new InvalidConfigurationException(e);
}
}

/**
* Selects a crosstool toolchain corresponding to the given crosstool configuration options. If
* all of these options are null, it returns the default toolchain specified in the crosstool
* release. If only cpu is non-null, it returns the default toolchain for that cpu, as specified
* in the crosstool release. Otherwise, all values must be non-null, and this method returns the
* toolchain which matches all of the values.
*
* @throws NullPointerException if {@code release} is null
* @throws InvalidConfigurationException if no matching toolchain can be found, or if the input
* parameters do not obey the constraints described above
*/
public static CrosstoolConfig.CToolchain selectToolchain(
CrosstoolConfig.CrosstoolRelease release,
BuildOptions options,
Function<String, String> cpuTransformer)
throws InvalidConfigurationException {
CrosstoolConfigurationIdentifier config =
CrosstoolConfigurationIdentifier.fromOptions(options);
return CToolchainSelectionUtils.selectToolchainUsingCpuAndMaybeCompiler(
release, config, cpuTransformer);
}
}

0 comments on commit bcaae47

Please sign in to comment.