Skip to content

Commit

Permalink
Shorten object file path
Browse files Browse the repository at this point in the history 
    Closes #4781.
    Fix bazelbuild/bazel#4149

    RELNOTES:
    Users can now pass --experimental_shortened_obj_file_path=true to have a shorter object file path, the object file paths (and all other related paths) will be constructed as following:
    If there's no two or more source files with the same base name:
      <bazel-bin>/<target_package_path>/_objs/<target_name>/<source_base_name>.<extension>
    otherwise:
      <bazel-bin>/<target_package_path>/_objs/<target_name>/N/<source_base_name>.<extension>
      N = the file?s order among the source files with the same basename, starts from 0.

    Examples:
    1. Output names for ["lib1/foo.cc", "lib2/bar.cc"] are ["foo", "bar"]
    2. Output names for ["foo.cc", "bar.cc", "foo.cpp", "lib/foo.cc"] are
       ["0/foo", "bar", "1/foo", "2/foo"]

    The default value of --experimental_shortened_obj_file_path option is false, but we plan to flip it to true and eventually remove this option.
    You shouldn't depend on the format of generated object file path, but if you do and this change breaks you, please use --experimental_shortened_obj_file_path=false to work around it.

    PiperOrigin-RevId: 190214375
  • Loading branch information
Luca Di Grazia committed Sep 4, 2022
1 parent c6161d3 commit faf045a
Show file tree
Hide file tree
Showing 10 changed files with 127 additions and 154 deletions.
36 changes: 6 additions & 30 deletions ...uild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1129,30 +1129,6 @@ public CcCompilationHelper setPurpose(@Nullable String purpose) {
return this;
}

/**
* Return flags that were specified on the Blaze command line. Take the filetype of sourceFilename
* into account.
*/
public static ImmutableList<String> getCoptsFromOptions(
CppConfiguration config, String sourceFilename) {
ImmutableList.Builder<String> flagsBuilder = ImmutableList.builder();

flagsBuilder.addAll(config.getCopts());

if (CppFileTypes.C_SOURCE.matches(sourceFilename)) {
flagsBuilder.addAll(config.getCOptions());
}

if (CppFileTypes.CPP_SOURCE.matches(sourceFilename)
|| CppFileTypes.CPP_HEADER.matches(sourceFilename)
|| CppFileTypes.CPP_MODULE_MAP.matches(sourceFilename)
|| CppFileTypes.CLIF_INPUT_PROTO.matches(sourceFilename)) {
flagsBuilder.addAll(config.getCxxopts());
}

return flagsBuilder.build();
}

/**
* Supplier that computes legacy_compile_flags lazily at the execution phase.
*
Expand All @@ -1167,13 +1143,15 @@ public static Supplier<ImmutableList<String>> getLegacyCompileFlagsSupplier(
return () -> {
ImmutableList.Builder<String> legacyCompileFlags = ImmutableList.builder();
legacyCompileFlags.addAll(
CppHelper.getCrosstoolCompilerOptions(cppConfiguration, toolchain, features));
CppHelper.getCompilerOptions(cppConfiguration, toolchain, features));
if (CppFileTypes.C_SOURCE.matches(sourceFilename)) {
legacyCompileFlags.addAll(cppConfiguration.getCOptions());
}
if (CppFileTypes.CPP_SOURCE.matches(sourceFilename)
|| CppFileTypes.CPP_HEADER.matches(sourceFilename)
|| CppFileTypes.CPP_MODULE_MAP.matches(sourceFilename)
|| CppFileTypes.CLIF_INPUT_PROTO.matches(sourceFilename)) {
legacyCompileFlags.addAll(
CppHelper.getCrosstoolCxxOptions(cppConfiguration, toolchain, features));
legacyCompileFlags.addAll(CppHelper.getCxxOptions(cppConfiguration, toolchain, features));
}
return legacyCompileFlags.build();
};
Expand Down Expand Up @@ -1544,18 +1522,16 @@ private void setupCompileBuildVariables(

CcCompilationInfo builderCcCompilationInfo = builder.getCcCompilationInfo();
Artifact sourceFile = builder.getSourceFile();
String sourceFilename = sourceFile.getExecPathString();

buildVariables.addStringVariable(SOURCE_FILE_VARIABLE_NAME, sourceFile.getExecPathString());
buildVariables.addStringSequenceVariable(
USER_COMPILE_FLAGS_VARIABLE_NAME,
ImmutableList.<String>builder()
// Add after the copts that come from the target.
.addAll(getCoptsFromOptions(cppConfiguration, sourceFilename))
.addAll(copts)
.addAll(collectPerFileCopts(sourceFile, sourceLabel))
.build());

String sourceFilename = sourceFile.getExecPathString();
buildVariables.addLazyStringSequenceVariable(
LEGACY_COMPILE_FLAGS_VARIABLE_NAME,
getLegacyCompileFlagsSupplier(cppConfiguration, ccToolchain, sourceFilename, features));
Expand Down
8 changes: 8 additions & 0 deletions ....bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ public class CppCompileActionBuilder {
private final ImmutableList.Builder<Artifact> additionalIncludeScanningRoots;
private Boolean shouldScanIncludes;
private Map<String, String> executionInfo = new LinkedHashMap<>();
private Map<String, String> environment = new LinkedHashMap<>();
private CppSemantics cppSemantics;
private CcToolchainProvider ccToolchain;
@Nullable private final Artifact grepIncludes;
Expand Down Expand Up @@ -159,6 +160,7 @@ public CppCompileActionBuilder(CppCompileActionBuilder other) {
this.lipoScannableMap = other.lipoScannableMap;
this.shouldScanIncludes = other.shouldScanIncludes;
this.executionInfo = new LinkedHashMap<>(other.executionInfo);
this.environment = new LinkedHashMap<>(other.environment);
this.localShellEnvironment = other.localShellEnvironment;
this.codeCoverageEnabled = other.codeCoverageEnabled;
this.cppSemantics = other.cppSemantics;
Expand Down Expand Up @@ -418,6 +420,7 @@ public CppCompileAction buildAndVerify(Consumer<String> errorCollector) {
additionalIncludeScanningRoots.build(),
actionClassId,
ImmutableMap.copyOf(executionInfo),
ImmutableMap.copyOf(environment),
getActionName(),
cppSemantics,
ccToolchain,
Expand Down Expand Up @@ -539,6 +542,11 @@ public CcToolchainFeatures.Variables getVariables() {
return variables;
}

public CppCompileActionBuilder addEnvironment(Map<String, String> environment) {
this.environment.putAll(environment);
return this;
}

public CppCompileActionBuilder addExecutionInfo(Map<String, String> executionInfo) {
this.executionInfo.putAll(executionInfo);
return this;
Expand Down
66 changes: 30 additions & 36 deletions ...elbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ public String toString() {
private final PathFragment crosstoolTopPathFragment;

private final Path fdoProfileAbsolutePath;
private final Label fdoOptimizeLabel;
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.
Expand Down Expand Up @@ -199,9 +199,10 @@ public String toString() {
private final boolean shouldProvideMakeVariables;
private final boolean dropFullyStaticLinkingMode;


/**
* If true, the ConfiguredTarget is only used to get the necessary cross-referenced {@code
* CcCompilationContextInfo}s, but registering build actions is disabled.
* CcCompilationInfo}s, but registering build actions is disabled.
*/
private final boolean lipoContextCollector;

Expand Down Expand Up @@ -268,7 +269,7 @@ static CppConfiguration create(CppConfigurationParameters params)
cppOptions.convertLipoToThinLto,
crosstoolTopPathFragment,
params.fdoProfileAbsolutePath,
params.fdoOptimizeLabel,
params.fdoProfileLabel,
params.ccToolchainLabel,
params.stlLabel,
params.sysrootLabel == null
Expand All @@ -281,33 +282,33 @@ static CppConfiguration create(CppConfigurationParameters params)
ImmutableList.copyOf(cppOptions.coptList)),
new FlagList(
cxxOptsBuilder.build(),
ImmutableList.of(),
FlagList.convertOptionalOptions(toolchain.getOptionalCxxFlagList()),
ImmutableList.copyOf(cppOptions.cxxoptList)),
new FlagList(
ImmutableList.copyOf(toolchain.getUnfilteredCxxFlagList()),
ImmutableList.of(),
ImmutableList.of()),
FlagList.convertOptionalOptions(toolchain.getOptionalUnfilteredCxxFlagList()),
ImmutableList.<String>of()),
ImmutableList.copyOf(cppOptions.conlyoptList),
new FlagList(
cppToolchainInfo.configureLinkerOptions(
compilationMode, cppOptions.getLipoMode(), LinkingMode.FULLY_STATIC),
ImmutableList.of(),
ImmutableList.of()),
FlagList.convertOptionalOptions(toolchain.getOptionalLinkerFlagList()),
ImmutableList.<String>of()),
new FlagList(
cppToolchainInfo.configureLinkerOptions(
compilationMode, cppOptions.getLipoMode(), LinkingMode.MOSTLY_STATIC),
ImmutableList.of(),
ImmutableList.of()),
FlagList.convertOptionalOptions(toolchain.getOptionalLinkerFlagList()),
ImmutableList.<String>of()),
new FlagList(
cppToolchainInfo.configureLinkerOptions(
compilationMode, cppOptions.getLipoMode(), LinkingMode.MOSTLY_STATIC_LIBRARIES),
ImmutableList.of(),
ImmutableList.of()),
FlagList.convertOptionalOptions(toolchain.getOptionalLinkerFlagList()),
ImmutableList.<String>of()),
new FlagList(
cppToolchainInfo.configureLinkerOptions(
compilationMode, cppOptions.getLipoMode(), LinkingMode.DYNAMIC),
ImmutableList.of(),
ImmutableList.of()),
FlagList.convertOptionalOptions(toolchain.getOptionalLinkerFlagList()),
ImmutableList.<String>of()),
ImmutableList.copyOf(cppOptions.coptList),
ImmutableList.copyOf(cppOptions.cxxoptList),
linkoptsBuilder.build(),
Expand All @@ -333,7 +334,7 @@ static CppConfiguration create(CppConfigurationParameters params)
boolean convertLipoToThinLto,
PathFragment crosstoolTopPathFragment,
Path fdoProfileAbsolutePath,
Label fdoOptimizeLabel,
Label fdoProfileLabel,
Label ccToolchainLabel,
Label stlLabel,
PathFragment nonConfiguredSysroot,
Expand Down Expand Up @@ -365,7 +366,7 @@ static CppConfiguration create(CppConfigurationParameters params)
this.convertLipoToThinLto = convertLipoToThinLto;
this.crosstoolTopPathFragment = crosstoolTopPathFragment;
this.fdoProfileAbsolutePath = fdoProfileAbsolutePath;
this.fdoOptimizeLabel = fdoOptimizeLabel;
this.fdoProfileLabel = fdoProfileLabel;
this.ccToolchainLabel = ccToolchainLabel;
this.stlLabel = stlLabel;
this.nonConfiguredSysroot = nonConfiguredSysroot;
Expand Down Expand Up @@ -901,7 +902,7 @@ private final boolean isLLVMCompiler() {
/**
* Returns true if LLVM FDO Optimization should be applied for this configuration.
*
* <p>Deprecated: Use {@link CcToolchain#isLLVMOptimizedFdo(boolean, Path)}
* <p>Deprecated: Use {@link CppConfiguration#isLLVMOptimizedFdo(boolean)}
*/
// TODO(b/64384912): Remove in favor of overload with isLLVMCompiler.
@Deprecated
Expand All @@ -913,6 +914,14 @@ public boolean isLLVMOptimizedFdo() {
&& 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();
Expand Down Expand Up @@ -1212,17 +1221,12 @@ public void reportInvalidOptions(EventHandler reporter, BuildOptions buildOption
}
}

// FDO
if (cppOptions.getFdoOptimize() != null && cppOptions.getFdoProfileLabel() != null) {
reporter.handle(Event.error("Both --fdo_optimize and --fdo_profile specified"));
}

if (cppOptions.getFdoInstrument() != null) {
if (cppOptions.getFdoOptimize() != null || cppOptions.getFdoProfileLabel() != null) {
if (cppOptions.getFdoOptimize() != null) {
reporter.handle(
Event.error(
"Cannot instrument and optimize for FDO at the same time. Remove one of the "
+ "'--fdo_instrument' and '--fdo_optimize/--fdo_profile' options"));
"Cannot instrument and optimize for FDO at the same time. "
+ "Remove one of the '--fdo_instrument' and '--fdo_optimize' options"));
}
if (!cppOptions.coptList.contains("-Wno-error")) {
// This is effectively impossible. --fdo_instrument adds this value, and only invocation
Expand All @@ -1231,12 +1235,6 @@ public void reportInvalidOptions(EventHandler reporter, BuildOptions buildOption
}
}

if (cppOptions.getLipoMode() != LipoMode.OFF && cppOptions.getFdoProfileLabel() != null) {
reporter.handle(
Event.error(
"LIPO options can not be used with --fdo_profile. Use --fdo_optimize instead"));
}

if (cppOptions.getLipoMode() != LipoMode.OFF
&& isLLVMCompiler()
&& !cppOptions.convertLipoToThinLto) {
Expand Down Expand Up @@ -1352,12 +1350,8 @@ public Path getFdoProfileAbsolutePath() {
return fdoProfileAbsolutePath;
}

public Label getFdoOptimizeLabel() {
return fdoOptimizeLabel;
}

public Label getFdoProfileLabel() {
return cppOptions.getFdoProfileLabel();
return fdoProfileLabel;
}

public boolean useLLVMCoverageMapFormat() {
Expand Down
47 changes: 10 additions & 37 deletions ...ava/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,13 +241,12 @@ public static List<String> expandLinkopts(
}

/**
* Returns the default options to use for compiling C, C++, and assembler, excluding those
* specified on the command line. This is just the options that should be used for all three
* languages. There may be additional C-specific or C++-specific options that should be used, in
* addition to the ones returned by this method.
* Returns the default options to use for compiling C, C++, and assembler. This is just the
* options that should be used for all three languages. There may be additional C-specific or
* C++-specific options that should be used, in addition to the ones returned by this method.
*/
// TODO(b/70784100): Figure out if these methods can be moved to CcToolchainProvider.
public static ImmutableList<String> getCrosstoolCompilerOptions(
//TODO(b/70784100): Figure out if these methods can be moved to CcToolchainProvider.
public static ImmutableList<String> getCompilerOptions(
CppConfiguration config, CcToolchainProvider toolchain, Iterable<String> features) {
ImmutableList.Builder<String> coptsBuilder =
ImmutableList.<String>builder()
Expand All @@ -265,30 +264,16 @@ public static ImmutableList<String> getCrosstoolCompilerOptions(
new FlagList(
coptsBuilder.build(),
FlagList.convertOptionalOptions(toolchain.getOptionalCompilerFlags()),
ImmutableList.of());
ImmutableList.copyOf(config.getCopts()));

return compilerFlags.evaluate(features);
}

/**
* Returns the default options to use for compiling C, C++, and assembler. This is just the
* options that should be used for all three languages. There may be additional C-specific or
* C++-specific options that should be used, in addition to the ones returned by this method.
*/
public static ImmutableList<String> getCompilerOptions(
CppConfiguration config, CcToolchainProvider toolchain, Iterable<String> features) {
return ImmutableList.<String>builder()
.addAll(getCrosstoolCompilerOptions(config, toolchain, features))
.addAll(config.getCopts())
.build();
}

/**
* Returns the list of additional C++-specific options to use for compiling C++, excluding those
* specified on the command line. These should be go on the command line after the common options
* returned by {@link #getCompilerOptions}.
* Returns the list of additional C++-specific options to use for compiling C++. These should be
* go on the command line after the common options returned by {@link #getCompilerOptions}.
*/
public static ImmutableList<String> getCrosstoolCxxOptions(
public static ImmutableList<String> getCxxOptions(
CppConfiguration config, CcToolchainProvider toolchain, Iterable<String> features) {
ImmutableList.Builder<String> cxxOptsBuilder =
ImmutableList.<String>builder()
Expand All @@ -300,23 +285,11 @@ public static ImmutableList<String> getCrosstoolCxxOptions(
new FlagList(
cxxOptsBuilder.build(),
FlagList.convertOptionalOptions(toolchain.getOptionalCxxFlags()),
ImmutableList.of());
ImmutableList.copyOf(config.getCxxopts()));

return cxxFlags.evaluate(features);
}

/**
* Returns the list of additional C++-specific options to use for compiling C++. These should be
* go on the command line after the common options returned by {@link #getCompilerOptions}.
*/
public static ImmutableList<String> getCxxOptions(
CppConfiguration config, CcToolchainProvider toolchain, Iterable<String> features) {
return ImmutableList.<String>builder()
.addAll(getCrosstoolCxxOptions(config, toolchain, features))
.addAll(config.getCxxopts())
.build();
}

/**
* Returns the immutable list of linker options for fully statically linked outputs. Does not
* include command-line options passed via --linkopt or --linkopts.
Expand Down
2 changes: 1 addition & 1 deletion ...va/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -989,7 +989,7 @@ public Map<String, Set<Label>> getDefaultsLabels() {
}
}

return ImmutableMap.of("CROSSTOOL", crosstoolLabels);
return ImmutableMap.of("CROSSTOOL", crosstoolLabels, "COVERAGE", ImmutableSet.<Label>of());
}

/**
Expand Down
Loading

0 comments on commit faf045a

Please sign in to comment.