Skip to content

Commit

Permalink
[7.2.0] Stop listing .gcno files as outputs when using LLVM coverag…
Browse files Browse the repository at this point in the history
…e map format (#22275)

When using `--experimental_use_llvm_covmap`, no `.gcno` files are
output. By listing them as outputs there is a mismatch between expected
and actual outputs. Bazel will create these empty files after the action
is run, but they non-deterministically get counted in the `Action`
outputs, which can lead to cache misses on subsequent runs.

Closes #22132.

PiperOrigin-RevId: 630996820
Change-Id: Ia7de7a05305095c8befee4f86181cf84d9737814

Co-authored-by: Brentley Jones <github@brentleyjones.com>
  • Loading branch information
c-mita and brentleyjones authored May 7, 2024
1 parent 2c54455 commit dad7eea
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1153,6 +1153,7 @@ private Artifact createCompileActionTemplate(
usePic,
/* needsFdoBuildVariables= */ false,
ccCompilationContext.getCppModuleMap(),
/* enableCoverage= */ false,
/* gcnoFile= */ null,
/* isUsingFission= */ false,
/* dwoFile= */ null,
Expand Down Expand Up @@ -1262,6 +1263,7 @@ private CcToolchainVariables setupCompileBuildVariables(
boolean usePic,
boolean needsFdoBuildVariables,
CppModuleMap cppModuleMap,
boolean enableCoverage,
Artifact gcnoFile,
boolean isUsingFission,
Artifact dwoFile,
Expand Down Expand Up @@ -1348,6 +1350,7 @@ private CcToolchainVariables setupCompileBuildVariables(
buildVariables,
toPathString(sourceFile),
toPathString(builder.getOutputFile()),
enableCoverage,
toPathString(gcnoFile),
toPathString(dwoFile),
isUsingFission,
Expand Down Expand Up @@ -1409,7 +1412,7 @@ private void createModuleCodegenAction(
ccToolchain, ArtifactCategory.COVERAGE_DATA_FILE, outputName);
// TODO(djasper): This is now duplicated. Refactor the various create..Action functions.
Artifact gcnoFile =
isCodeCoverageEnabled
isCodeCoverageEnabled && !cppConfiguration.useLLVMCoverageMapFormat()
? CppHelper.getCompileOutputArtifact(
actionConstructionContext, label, gcnoFileName, configuration)
: null;
Expand All @@ -1431,6 +1434,7 @@ private void createModuleCodegenAction(
/* usePic= */ pic,
/* needsFdoBuildVariables= */ ccRelativeName != null,
ccCompilationContext.getCppModuleMap(),
isCodeCoverageEnabled,
gcnoFile,
generateDwo,
dwoFile,
Expand Down Expand Up @@ -1480,6 +1484,7 @@ private void createHeaderAction(
generatePicAction,
/* needsFdoBuildVariables= */ false,
ccCompilationContext.getCppModuleMap(),
/* enableCoverage= */ false,
/* gcnoFile= */ null,
/* isUsingFission= */ false,
/* dwoFile= */ null,
Expand Down Expand Up @@ -1547,7 +1552,7 @@ private ImmutableList<Artifact> createSourceAction(
CppHelper.getArtifactNameForCategory(
ccToolchain, ArtifactCategory.COVERAGE_DATA_FILE, picOutputBase);
Artifact gcnoFile =
enableCoverage
enableCoverage && !cppConfiguration.useLLVMCoverageMapFormat()
? CppHelper.getCompileOutputArtifact(
actionConstructionContext, label, gcnoFileName, configuration)
: null;
Expand All @@ -1564,6 +1569,7 @@ private ImmutableList<Artifact> createSourceAction(
/* usePic= */ true,
/* needsFdoBuildVariables= */ ccRelativeName != null && addObject,
cppModuleMap,
enableCoverage,
gcnoFile,
generateDwo,
dwoFile,
Expand Down Expand Up @@ -1624,7 +1630,7 @@ private ImmutableList<Artifact> createSourceAction(

// Create no-PIC compile actions
Artifact gcnoFile =
enableCoverage
isCodeCoverageEnabled && !cppConfiguration.useLLVMCoverageMapFormat()
? CppHelper.getCompileOutputArtifact(
actionConstructionContext, label, gcnoFileName, configuration)
: null;
Expand All @@ -1640,6 +1646,7 @@ private ImmutableList<Artifact> createSourceAction(
/* usePic= */ false,
/* needsFdoBuildVariables= */ ccRelativeName != null,
cppModuleMap,
isCodeCoverageEnabled,
gcnoFile,
generateDwo,
noPicDwoFile,
Expand Down Expand Up @@ -1810,6 +1817,7 @@ private ImmutableList<Artifact> createTempsActions(
usePic,
/* needsFdoBuildVariables= */ ccRelativeName != null,
ccCompilationContext.getCppModuleMap(),
/* enableCoverage= */ false,
/* gcnoFile= */ null,
/* isUsingFission= */ false,
/* dwoFile= */ null,
Expand Down Expand Up @@ -1837,6 +1845,7 @@ private ImmutableList<Artifact> createTempsActions(
usePic,
/* needsFdoBuildVariables= */ ccRelativeName != null,
ccCompilationContext.getCppModuleMap(),
/* enableCoverage= */ false,
/* gcnoFile= */ null,
/* isUsingFission= */ false,
/* dwoFile= */ null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,7 @@ public CcToolchainVariables getCompileBuildVariables(
.getCppConfigurationFromFeatureConfigurationCreatedForStarlark_andIKnowWhatImDoing(),
convertFromNoneable(sourceFile, /* defaultValue= */ null),
convertFromNoneable(outputFile, /* defaultValue= */ null),
/* isCodeCoverageEnabled= */ false,
/* gcnoFile= */ null,
/* isUsingFission= */ false,
/* dwoFile= */ null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ public static CcToolchainVariables setupVariablesOrReportRuleError(
CppConfiguration cppConfiguration,
String sourceFile,
String outputFile,
boolean isCodeCoverageEnabled,
String gcnoFile,
boolean isUsingFission,
String dwoFile,
Expand Down Expand Up @@ -179,6 +180,7 @@ public static CcToolchainVariables setupVariablesOrReportRuleError(
ccToolchainProvider.getBuildVariables(thread, buildOptions, cppConfiguration),
sourceFile,
outputFile,
isCodeCoverageEnabled,
gcnoFile,
isUsingFission,
dwoFile,
Expand Down Expand Up @@ -216,6 +218,7 @@ public static CcToolchainVariables setupVariablesOrThrowEvalException(
CppConfiguration cppConfiguration,
String sourceFile,
String outputFile,
boolean isCodeCoverageEnabled,
String gcnoFile,
boolean isUsingFission,
String dwoFile,
Expand Down Expand Up @@ -250,6 +253,7 @@ public static CcToolchainVariables setupVariablesOrThrowEvalException(
ccToolchainProvider.getBuildVariables(thread, buildOptions, cppConfiguration),
sourceFile,
outputFile,
isCodeCoverageEnabled,
gcnoFile,
isUsingFission,
dwoFile,
Expand Down Expand Up @@ -280,6 +284,7 @@ private static CcToolchainVariables setupVariables(
CcToolchainVariables parent,
String sourceFile,
String outputFile,
boolean isCodeCoverageEnabled,
String gcnoFile,
boolean isUsingFission,
String dwoFile,
Expand Down Expand Up @@ -323,6 +328,7 @@ private static CcToolchainVariables setupVariables(
buildVariables,
sourceFile,
outputFile,
isCodeCoverageEnabled,
gcnoFile,
dwoFile,
isUsingFission,
Expand All @@ -343,6 +349,7 @@ public static void setupSpecificVariables(
CcToolchainVariables.Builder buildVariables,
String sourceFile,
String outputFile,
boolean isCodeCoverageEnabled,
String gcnoFile,
String dwoFile,
boolean isUsingFission,
Expand Down Expand Up @@ -380,6 +387,10 @@ public static void setupSpecificVariables(

if (gcnoFile != null) {
buildVariables.addStringVariable(GCOV_GCNO_FILE.getVariableName(), gcnoFile);
} else if (isCodeCoverageEnabled) {
// TODO: Blaze currently uses `gcov_gcno_file` to detect if code coverage is enabled. It
// should use a different signal.
buildVariables.addStringVariable(GCOV_GCNO_FILE.getVariableName(), "");
}

if (dwoFile != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ private static CcToolchainVariables getVariables(
cppConfiguration,
sourceFile.getExecPathString(),
outputFile.getExecPathString(),
/* isCodeCoverageEnabled= */ false,
/* gcnoFile= */ null,
/* isUsingFission= */ false,
/* dwoFile= */ null,
Expand Down

0 comments on commit dad7eea

Please sign in to comment.