Early support for code generation directly from C++ module files. This can
potentially reduce the linker input. The content of a C++ module (specifically
the inline definitions) is currently put into every single .o file that uses
them and thus can be input to the same final link many times. By generating a
separate .o file for the module itself, we can avoid this unnecessary
duplication of linker input.
--
PiperOrigin-RevId: 147579947
MOS_MIGRATED_REVID=147579947
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java
index 76b57ef..b0183e0 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java
@@ -144,8 +144,13 @@
Artifact soImplArtifact = null;
boolean supportsDynamicLinker =
ruleContext.getFragment(CppConfiguration.class).supportsDynamicLinker();
+ // TODO(djasper): This is hacky. We should actually try to figure out whether we generate
+ // ccOutputs.
boolean createDynamicLibrary =
- !linkStatic && appearsToHaveObjectFiles(ruleContext.attributes()) && supportsDynamicLinker;
+ !linkStatic
+ && supportsDynamicLinker
+ && (appearsToHaveObjectFiles(ruleContext.attributes())
+ || featureConfiguration.isEnabled(CppRuleClasses.HEADER_MODULE_CODEGEN));
if (ruleContext.getRule().isAttrDefined("outs", Type.STRING_LIST)) {
List<String> outs = ruleContext.attributes().get("outs", Type.STRING_LIST);
if (outs.size() > 1) {
@@ -258,8 +263,10 @@
LinkerInputs.toNonSolibArtifacts(linkedLibraries.getExecutionDynamicLibraries()));
CcLinkingOutputs linkingOutputs = info.getCcLinkingOutputs();
- warnAboutEmptyLibraries(
- ruleContext, info.getCcCompilationOutputs(), linkStatic);
+ if (!featureConfiguration.isEnabled(CppRuleClasses.HEADER_MODULE_CODEGEN)) {
+ warnAboutEmptyLibraries(
+ ruleContext, info.getCcCompilationOutputs(), linkStatic);
+ }
NestedSet<Artifact> filesToBuild = filesBuilder.build();
Runfiles staticRunfiles = collectRunfiles(ruleContext, linkingOutputs, ccToolchain,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java
index 0724b07..11fcd47 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java
@@ -282,6 +282,7 @@
private final List<LibraryToLink> picStaticLibraries = new ArrayList<>();
private final List<LibraryToLink> dynamicLibraries = new ArrayList<>();
+ private boolean emitLinkActions = true;
private boolean emitLinkActionsIfEmpty;
private boolean emitCcNativeLibrariesProvider;
private boolean emitCcSpecificLinkParamsProvider;
@@ -833,6 +834,11 @@
return this;
}
+ public CcLibraryHelper setEmitLinkActions(boolean emitLinkActions) {
+ this.emitLinkActions = emitLinkActions;
+ return this;
+ }
+
/**
* Enables or disables generation of link actions if there are no object files. Some rules declare
* a <code>.a</code> or <code>.so</code> implicit output, which requires that these files are
@@ -930,7 +936,7 @@
// Create link actions (only if there are object files or if explicitly requested).
CcLinkingOutputs ccLinkingOutputs = CcLinkingOutputs.EMPTY;
- if (emitLinkActionsIfEmpty || !ccOutputs.isEmpty()) {
+ if (emitLinkActions && (emitLinkActionsIfEmpty || !ccOutputs.isEmpty())) {
// On some systems, the linker gives an error message if there are no input files. Even with
// the check above, this can still happen if there is a .nopic.o or .o files in srcs, but no
// other files. To fix that, we'd have to check for each link action individually.
@@ -1012,7 +1018,7 @@
Map<String, NestedSet<Artifact>> outputGroups = new TreeMap<>();
if (shouldAddLinkerOutputArtifacts(ruleContext, ccOutputs)) {
- addLinkerOutputArtifacts(outputGroups);
+ addLinkerOutputArtifacts(outputGroups, ccOutputs);
}
outputGroups.put(OutputGroupProvider.TEMP_FILES, getTemps(ccOutputs));
@@ -1059,7 +1065,7 @@
/**
* Returns true if the appropriate attributes for linker output artifacts are defined, and either
* the compile action produces object files or the build is configured to produce an archive or
- * dynamic library even in the absense of object files.
+ * dynamic library even in the absence of object files.
*/
private boolean shouldAddLinkerOutputArtifacts(
RuleContext ruleContext, CcCompilationOutputs ccOutputs) {
@@ -1072,7 +1078,8 @@
* Adds linker output artifacts to the given map, to be registered on the configured target as
* output groups.
*/
- private void addLinkerOutputArtifacts(Map<String, NestedSet<Artifact>> outputGroups) {
+ private void addLinkerOutputArtifacts(Map<String, NestedSet<Artifact>> outputGroups,
+ CcCompilationOutputs ccOutputs) {
NestedSetBuilder<Artifact> archiveFile = new NestedSetBuilder<>(Order.STABLE_ORDER);
NestedSetBuilder<Artifact> dynamicLibrary = new NestedSetBuilder<>(Order.STABLE_ORDER);
@@ -1085,7 +1092,8 @@
CppHelper.getLinuxLinkedArtifact(ruleContext, Link.LinkTargetType.STATIC_LIBRARY));
}
- if (CppRuleClasses.shouldCreateDynamicLibrary(ruleContext.attributes())) {
+ if (!ruleContext.attributes().get("linkstatic", Type.BOOLEAN)
+ && !ccOutputs.isEmpty()) {
dynamicLibrary.add(
CppHelper.getLinuxLinkedArtifact(ruleContext, Link.LinkTargetType.DYNAMIC_LIBRARY));
}
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 c51edff..3f84370 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
@@ -123,6 +123,9 @@
* A string constant for the c++ compilation action.
*/
public static final String CPP_COMPILE = "c++-compile";
+
+ /** A string constant for the c++ module compile action. */
+ public static final String CPP_MODULE_CODEGEN = "c++-module-codegen";
/**
* A string constant for the objc compilation action.
@@ -167,6 +170,7 @@
private final ImmutableMap<String, String> localShellEnvironment;
private final boolean isCodeCoverageEnabled;
protected final Artifact outputFile;
+ private final Artifact sourceFile;
private final Label sourceLabel;
private final Artifact optionalSourceFile;
private final NestedSet<Artifact> mandatoryInputs;
@@ -317,6 +321,7 @@
this.localShellEnvironment = localShellEnvironment;
this.isCodeCoverageEnabled = isCodeCoverageEnabled;
this.sourceLabel = sourceLabel;
+ this.sourceFile = sourceFile;
this.outputFile = Preconditions.checkNotNull(outputFile);
this.optionalSourceFile = optionalSourceFile;
this.context = context;
@@ -436,7 +441,7 @@
throws ActionExecutionException, InterruptedException {
Executor executor = actionExecutionContext.getExecutor();
Iterable<Artifact> initialResult;
-
+
actionExecutionContext
.getExecutor()
.getEventBus()
@@ -471,13 +476,18 @@
Set<Artifact> initialResultSet = Sets.newLinkedHashSet(initialResult);
if (shouldPruneModules) {
- usedModules = Sets.newLinkedHashSet();
- topLevelModules = null;
- for (CppCompilationContext.TransitiveModuleHeaders usedModule :
- context.getUsedModules(usePic, initialResultSet)) {
- usedModules.add(usedModule.getModule());
+ if (CppFileTypes.CPP_MODULE.matches(sourceFile.getFilename())) {
+ usedModules = ImmutableSet.of(sourceFile);
+ initialResultSet.add(sourceFile);
+ } else {
+ usedModules = Sets.newLinkedHashSet();
+ topLevelModules = null;
+ for (CppCompilationContext.TransitiveModuleHeaders usedModule :
+ context.getUsedModules(usePic, initialResultSet)) {
+ usedModules.add(usedModule.getModule());
+ }
+ initialResultSet.addAll(usedModules);
}
- initialResultSet.addAll(usedModules);
}
initialResult = initialResultSet;
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 235ca5e..6e75edd 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
@@ -285,6 +285,8 @@
return CppCompileAction.PREPROCESS_ASSEMBLE;
} else if (CppFileTypes.CLIF_INPUT_PROTO.matches(sourcePath)) {
return CppCompileAction.CLIF_MATCH;
+ } else if (CppFileTypes.CPP_MODULE.matches(sourcePath)) {
+ return CppCompileAction.CPP_MODULE_CODEGEN;
}
// CcLibraryHelper ensures CppCompileAction only gets instantiated for supported file types.
throw new IllegalStateException();
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 ea1a1e8..9893dfc 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
@@ -791,6 +791,7 @@
+ " name: 'random_seed'"
+ " flag_set {"
+ " action: 'c++-compile'"
+ + " action: 'c++-module-codegen'"
+ " action: 'c++-module-compile'"
+ " flag_group {"
+ " flag: '-frandom-seed=%{output_file}'"
@@ -808,6 +809,7 @@
+ " flag_set {"
+ " action: 'c-compile'"
+ " action: 'c++-compile'"
+ + " action: 'c++-module-codegen'"
+ " action: 'c++-module-compile'"
+ " action: 'preprocess-assemble'"
+ " expand_if_all_available: 'pic'"
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java
index dc17485..b819aae 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java
@@ -192,7 +192,8 @@
String fileName = source.getFilename();
return !ASSEMBLER.matches(fileName)
&& !PIC_ASSEMBLER.matches(fileName)
- && !CLIF_INPUT_PROTO.matches(fileName);
+ && !CLIF_INPUT_PROTO.matches(fileName)
+ && !CPP_MODULE.matches(fileName);
}
}
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 1df1eb6..43eca8a 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
@@ -483,9 +483,17 @@
AnalysisEnvironment env = ruleContext.getAnalysisEnvironment();
if (shouldProvideHeaderModules()) {
- createModuleAction(result, context.getCppModuleMap(), /*addHeaderTokenFiles=*/ false);
+ Collection<Artifact> modules = createModuleAction(result, context.getCppModuleMap());
+ if (featureConfiguration.isEnabled(CppRuleClasses.HEADER_MODULE_CODEGEN)) {
+ for (Artifact module : modules) {
+ createModuleCodegenAction(result, module);
+ }
+ }
} else if (context.getVerificationModuleMap() != null) {
- createModuleAction(result, context.getVerificationModuleMap(), /*addHeaderTokenFiles=*/ true);
+ Collection<Artifact> modules = createModuleAction(result, context.getVerificationModuleMap());
+ for (Artifact module : modules) {
+ result.addHeaderTokenFile(module);
+ }
}
for (CppSource source : sourceFiles) {
@@ -525,8 +533,7 @@
// info). In that case the LTOBackendAction will generate the dwo.
/*generateDwo=*/ cppConfiguration.useFission() && !bitcodeOutput,
CppFileTypes.mustProduceDotdFile(sourceArtifact),
- source.getBuildVariables(), /*addHeaderTokenFile=*/
- false);
+ source.getBuildVariables());
break;
}
} else {
@@ -583,8 +590,70 @@
result.addHeaderTokenFile(tokenFile);
}
- private void createModuleAction(
- CcCompilationOutputs.Builder result, CppModuleMap cppModuleMap, boolean addHeaderTokenFiles) {
+ private void createModuleCodegenAction(CcCompilationOutputs.Builder result, Artifact module) {
+ if (fake) {
+ // We can't currently foresee a situation where we'd want nocompile tests for module codegen.
+ // If we find one, support needs to be added here.
+ return;
+ }
+ String outputName = semantics.getEffectiveSourcePath(module).getPathString();
+
+ // TODO(djasper): Make this less hacky after refactoring how the PIC/noPIC actions are created.
+ boolean pic = module.getFilename().contains(".pic.");
+
+ // TODO(djasper): Investigate whether we need to use a label separate from that of the module
+ // map. It is used for per-file-copts.
+ CppCompileActionBuilder builder =
+ initializeCompileAction(
+ module, Label.parseAbsoluteUnchecked(context.getCppModuleMap().getName()));
+ builder.setSemantics(semantics);
+ builder.setPicMode(pic);
+ builder.setOutputs(
+ ruleContext,
+ ArtifactCategory.OBJECT_FILE,
+ outputName,
+ CppFileTypes.mustProduceDotdFile(module));
+ PathFragment ccRelativeName = semantics.getEffectiveSourcePath(module);
+
+ String gcnoFileName =
+ CppHelper.getArtifactNameForCategory(
+ ruleContext, ccToolchain, ArtifactCategory.COVERAGE_DATA_FILE, outputName);
+ // TODO(djasper): This is now duplicated. Refactor the various create..Action functions.
+ Artifact gcnoFile =
+ isCodeCoverageEnabled() && !cppConfiguration.isLipoOptimization()
+ ? CppHelper.getCompileOutputArtifact(ruleContext, gcnoFileName, configuration)
+ : null;
+
+ boolean generateDwo = cppConfiguration.useFission();
+ Artifact dwoFile = generateDwo ? getDwoFile(builder.getOutputFile()) : null;
+
+ setupCompileBuildVariables(
+ builder,
+ /*usePic=*/ pic,
+ ccRelativeName,
+ module.getExecPath(),
+ gcnoFile,
+ dwoFile,
+ builder.getContext().getCppModuleMap(),
+ ImmutableMap.<String, String>of());
+
+ builder.setGcnoFile(gcnoFile);
+ builder.setDwoFile(dwoFile);
+
+ semantics.finalizeCompileActionBuilder(ruleContext, builder);
+ CppCompileAction compileAction = builder.build();
+ AnalysisEnvironment env = ruleContext.getAnalysisEnvironment();
+ env.registerAction(compileAction);
+ Artifact objectFile = compileAction.getOutputFile();
+ if (pic) {
+ result.addPicObjectFile(objectFile);
+ } else {
+ result.addObjectFile(objectFile);
+ }
+ }
+
+ private Collection<Artifact> createModuleAction(
+ CcCompilationOutputs.Builder result, CppModuleMap cppModuleMap) {
AnalysisEnvironment env = ruleContext.getAnalysisEnvironment();
Label moduleMapLabel = Label.parseAbsoluteUnchecked(context.getCppModuleMap().getName());
Artifact moduleMapArtifact = cppModuleMap.getArtifact();
@@ -595,7 +664,7 @@
// A header module compile action is just like a normal compile action, but:
// - the compiled source file is the module map
// - it creates a header module (.pcm file).
- createSourceAction(
+ return createSourceAction(
FileSystemUtils.removeExtension(semantics.getEffectiveSourcePath(moduleMapArtifact))
.getPathString(),
result,
@@ -608,8 +677,7 @@
/*enableCoverage=*/ false,
/*generateDwo=*/ false,
CppFileTypes.mustProduceDotdFile(moduleMapArtifact),
- ImmutableMap.<String, String>of(), /*addHeaderTokenFile=*/
- addHeaderTokenFiles);
+ ImmutableMap.<String, String>of());
}
private void createClifMatchAction(
@@ -637,7 +705,7 @@
result.addHeaderTokenFile(tokenFile);
}
- private void createSourceAction(
+ private Collection<Artifact> createSourceAction(
String outputName,
CcCompilationOutputs.Builder result,
AnalysisEnvironment env,
@@ -649,8 +717,8 @@
boolean enableCoverage,
boolean generateDwo,
boolean generateDotd,
- Map<String, String> sourceSpecificBuildVariables,
- boolean addHeaderTokenFiles) {
+ Map<String, String> sourceSpecificBuildVariables) {
+ ImmutableList.Builder<Artifact> directOutputs = new ImmutableList.Builder<>();
PathFragment ccRelativeName = semantics.getEffectiveSourcePath(sourceArtifact);
if (cppConfiguration.isLipoOptimization()) {
// TODO(bazel-team): we shouldn't be needing this, merging context with the binary
@@ -704,9 +772,7 @@
semantics.finalizeCompileActionBuilder(ruleContext, picBuilder);
CppCompileAction picAction = picBuilder.buildAndValidate(ruleContext);
env.registerAction(picAction);
- if (addHeaderTokenFiles) {
- result.addHeaderTokenFile(picAction.getOutputFile());
- }
+ directOutputs.add(picAction.getOutputFile());
if (addObject) {
result.addPicObjectFile(picAction.getOutputFile());
@@ -770,9 +836,7 @@
CppCompileAction compileAction = builder.buildAndValidate(ruleContext);
env.registerAction(compileAction);
Artifact objectFile = compileAction.getOutputFile();
- if (addHeaderTokenFiles) {
- result.addHeaderTokenFile(objectFile);
- }
+ directOutputs.add(objectFile);
if (addObject) {
result.addObjectFile(objectFile);
if (featureConfiguration.isEnabled(CppRuleClasses.THIN_LTO)
@@ -789,6 +853,7 @@
}
}
}
+ return directOutputs.build();
}
private Artifact createCompileActionTemplate(AnalysisEnvironment env,
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 024f26d..bb22ec2 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
@@ -39,7 +39,6 @@
import com.google.devtools.build.lib.packages.ImplicitOutputsFunction.SafeImplicitOutputsFunction;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.rules.test.InstrumentedFilesCollector.InstrumentationSpec;
-import com.google.devtools.build.lib.syntax.Type;
import com.google.devtools.build.lib.util.FileTypeSet;
import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.LipoMode;
@@ -47,12 +46,6 @@
* Rule class definitions for C++ rules.
*/
public class CppRuleClasses {
-
- /** Returns true if this rule should create a dynamic library. */
- public static boolean shouldCreateDynamicLibrary(AttributeMap rule) {
- return !rule.get("linkstatic", Type.BOOLEAN) && CcLibrary.appearsToHaveObjectFiles(rule);
- }
-
/**
* Implementation for the :lipo_context_collector attribute.
*/
@@ -197,6 +190,9 @@
/** A string constant for the header_modules_compile feature. */
public static final String HEADER_MODULE_COMPILE = "header_module_compile";
+
+ /** A string constant for the header_module_codegen feature. */
+ public static final String HEADER_MODULE_CODEGEN = "header_module_codegen";
/**
* A string constant for the compile_all_modules feature.
diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java b/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java
index 6f8f136..1441289 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java
@@ -110,6 +110,16 @@
+ " flag: '--woohoo_modules'"
+ " }"
+ " }"
+ + " flag_set {"
+ + " action: 'c++-module-codegen'"
+ + " flag_group {"
+ + " flag: '--this_is_modules_codegen'"
+ + " }"
+ + " }"
+ + "}"
+ + "feature {"
+ + " name: 'header_module_codegen'"
+ + " implies: 'header_modules'"
+ "}"
+ "feature {"
+ " name: 'module_maps'"
