diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java index 6c0952f4eaebae..cbe009783a973d 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java @@ -66,6 +66,8 @@ public enum CompileBuildVariables { CPP_MODULE_MODMAP_FILE("cpp_module_modmap_file"), /** Variable for the c++20 module output file name. */ CPP_MODULE_OUTPUT_FILE("cpp_module_output_file"), + /** Variable for the c++20 module with 2-phase compilation. */ + CPP_MODULES_WITH_TWO_PHASE_COMPILATION("cpp_modules_with_two_phase_compilation"), /** Variable for the module map file name. */ MODULE_MAP_FILE("module_map_file"), /** Variable for the dependent module map file name. */ 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 c0cfc3fc456d27..ecef9f850feac8 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 @@ -664,11 +664,23 @@ public NestedSet discoverInputs(ActionExecutionContext actionExecution // This is necessary because the inputs that can be pruned by .d file parsing must be // returned from discoverInputs() and they cannot be in mandatoryInputs. Thus, even with // include scanning turned off, we pretend that we "discover" these headers. - additionalInputs = - NestedSetBuilder.fromNestedSet(ccCompilationContext.getDeclaredIncludeSrcs()) - .addTransitive(additionalPrunableHeaders) - .addAll(usedCpp20Modules) - .build(); + + // Support C++20 Modules with two-phase compilation + // For compiling C++20 Modules files to object files (.pcm -> .o) + // headers are not needed as the module interface is already compiled into the pcm file. + if (actionName.equals(CppActionNames.CPP20_MODULE_CODEGEN)) { + additionalInputs = + NestedSetBuilder.stableOrder() + .addAll(usedCpp20Modules) + .build(); + } + else { + additionalInputs = + NestedSetBuilder.fromNestedSet(ccCompilationContext.getDeclaredIncludeSrcs()) + .addTransitive(additionalPrunableHeaders) + .addAll(usedCpp20Modules) + .build(); + } if (needsIncludeValidation) { verifyActionIncludePaths(systemIncludeDirs, siblingLayout); } @@ -1557,6 +1569,12 @@ public ActionResult execute(ActionExecutionContext actionExecutionContext) } if (getDotdFile() == null) { + if (discoversInputs() && isCpp20ModuleCompilationAction(actionName) && usedCpp20Modules != null) { + NestedSet discoveredInputs = NestedSetBuilder.stableOrder() + .addAll(usedCpp20Modules) + .build(); + updateActionInputs(discoveredInputs); + } return ActionResult.create(spawnResults); } 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 34d480b211088c..35853f43de111d 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 @@ -367,7 +367,11 @@ NestedSet buildMandatoryInputs() throws EvalException { if (ccToolchain.getGrepIncludes() != null) { realMandatoryInputsBuilder.add(ccToolchain.getGrepIncludes()); } - if (!getShouldScanIncludes() && dotdFile == null && !shouldParseShowIncludes()) { + // Support C++20 Modules with two-phase compilation + // For compiling C++20 Modules files to object files (.pcm -> .o) + // headers are not needed as the module interface is already compiled into the pcm file. + if (!getShouldScanIncludes() && dotdFile == null && !shouldParseShowIncludes() + && !actionName.equals(CppActionNames.CPP20_MODULE_CODEGEN)) { realMandatoryInputsBuilder.addTransitive(ccCompilationContext.getDeclaredIncludeSrcs()); realMandatoryInputsBuilder.addTransitive(additionalPrunableHeaders); } diff --git a/src/main/starlark/builtins_bzl/common/cc/compile/compile.bzl b/src/main/starlark/builtins_bzl/common/cc/compile/compile.bzl index c989ddd589ab17..a1930a1ec0fa32 100644 --- a/src/main/starlark/builtins_bzl/common/cc/compile/compile.bzl +++ b/src/main/starlark/builtins_bzl/common/cc/compile/compile.bzl @@ -637,6 +637,233 @@ def _create_gen_modmap_action( toolchain = None, ) +def _create_compile_module_interface_to_module_file_only_action( + *, + action_construction_context, + cc_compilation_context, + label, + source_label, + source_artifact, + output_name, + cc_toolchain, + feature_configuration, + configuration, + cpp_configuration, + language, + conlyopts, + copts, + cxxopts, + copts_filter, + common_compile_variables, + fdo_build_variables, + cpp_module_map, + enable_coverage, + additional_include_scanning_roots, + use_pic, + output_file, + additional_build_variables = {}, + action_name = None, + additional_outputs = [], + module_files = depset(), + modmap_file = None, + modmap_input_file = None): + output_pic_nopic_name = output_name + if use_pic: + output_pic_nopic_name = _cc_internal.get_artifact_name_for_category( + cc_toolchain = cc_toolchain, + category = artifact_category.PIC_FILE, + output_name = output_name, + ) + dotd_file = _maybe_declare_dotd_file( + ctx = action_construction_context, + label = label, + source_artifact = source_artifact, + category = artifact_category.CPP_MODULE, + output_name = output_pic_nopic_name, + cc_toolchain = cc_toolchain, + language = language, + configuration = configuration, + feature_configuration = feature_configuration, + ) + complete_copts = get_copts( + language = language, + cpp_configuration = cpp_configuration, + source_file = source_artifact, + copts = copts, + conlyopts = conlyopts, + cxxopts = cxxopts, + label = source_label, + # Treat C++20 module interfaces as C++ source files regardless of their extension. + override_extension = extensions.CC_SOURCE[0], + ) + compile_variables = get_specific_compile_build_variables( + source_file = source_artifact, + output_file = output_file, + code_coverage_enabled = enable_coverage, + gcno_file = None, + dwo_file = None, + using_fission = False, + lto_indexing_file = None, + user_compile_flags = complete_copts, + dotd_file = dotd_file, + diagnostics_file = None, + use_pic = use_pic, + cpp_module_map = cpp_module_map, + feature_configuration = feature_configuration, + direct_module_maps = cc_compilation_context._direct_module_maps, + fdo_build_variables = fdo_build_variables, + additional_build_variables = additional_build_variables, + ) + module_args = { + "additional_outputs": additional_outputs, + "module_files": module_files, + "modmap_file": modmap_file, + "modmap_input_file": modmap_input_file, + } + _cc_internal.create_cc_compile_action( + action_construction_context = action_construction_context, + cc_compilation_context = cc_compilation_context, + cc_toolchain = cc_toolchain, + configuration = configuration, + copts_filter = copts_filter, + feature_configuration = feature_configuration, + additional_include_scanning_roots = additional_include_scanning_roots, + source = source_artifact, + output_file = output_file, + diagnostics_file = None, + dotd_file = dotd_file, + gcno_file = None, + dwo_file = None, + use_pic = use_pic, + lto_indexing_file = None, + action_name = action_name, + compile_build_variables = _cc_internal.combine_cc_toolchain_variables( + common_compile_variables, + compile_variables, + ), + needs_include_validation = _starlark_cc_semantics.needs_include_validation(language), + toolchain_type = _starlark_cc_semantics.toolchain, + **module_args + ) + +def _create_compile_module_interface_action( + *, + action_construction_context, + cc_compilation_context, + label, + source_label, + source_artifact, + output_name, + outputs, + cc_toolchain, + feature_configuration, + configuration, + cpp_configuration, + language, + conlyopts, + copts, + cxxopts, + copts_filter, + common_compile_variables, + fdo_build_variables, + output_category, + cpp_module_map, + add_object, + enable_coverage, + generate_dwo, + bitcode_output, + fdo_context, + auxiliary_fdo_inputs, + additional_compilation_inputs, + additional_include_scanning_roots, + use_pic, + module_files, + modmap_file, + modmap_input_file, + module_file): + additional_build_variables = { + "cpp_module_modmap_file": modmap_file, + } + additional_outputs = [] + current_action_name = ACTION_NAMES.cpp20_module_compile + action_source_artifact = source_artifact + if feature_configuration.is_enabled("cpp_modules_with_two_phase_compilation"): + additional_build_variables["cpp_modules_with_two_phase_compilation"] = "1" + additional_outputs = [module_file] + _create_compile_module_interface_to_module_file_only_action( + action_construction_context = action_construction_context, + cc_compilation_context = cc_compilation_context, + label = label, + source_label = source_label, + source_artifact = action_source_artifact, + output_name = output_name, + cc_toolchain = cc_toolchain, + feature_configuration = feature_configuration, + configuration = configuration, + cpp_configuration = cpp_configuration, + language = language, + conlyopts = conlyopts, + copts = copts, + cxxopts = cxxopts, + copts_filter = copts_filter, + common_compile_variables = common_compile_variables, + fdo_build_variables = fdo_build_variables, + enable_coverage = enable_coverage, + cpp_module_map = cpp_module_map, + additional_include_scanning_roots = additional_include_scanning_roots, + use_pic = use_pic, + additional_build_variables = additional_build_variables, + action_name = current_action_name, + additional_outputs = additional_outputs, + module_files = module_files, + modmap_file = modmap_file, + modmap_input_file = modmap_input_file, + output_file = module_file, + ) + current_action_name = ACTION_NAMES.cpp20_module_codegen + action_source_artifact = module_file + additional_outputs = [] + else: + additional_build_variables["cpp_module_output_file"] = module_file + additional_outputs = [module_file] + _create_compile_source_action( + action_construction_context = action_construction_context, + cc_compilation_context = cc_compilation_context, + label = label, + source_label = source_label, + source_artifact = action_source_artifact, + output_name = output_name, + outputs = outputs, + cc_toolchain = cc_toolchain, + feature_configuration = feature_configuration, + configuration = configuration, + cpp_configuration = cpp_configuration, + language = language, + conlyopts = conlyopts, + copts = copts, + cxxopts = cxxopts, + copts_filter = copts_filter, + common_compile_variables = common_compile_variables, + fdo_build_variables = fdo_build_variables, + output_category = output_category, + cpp_module_map = cpp_module_map, + add_object = add_object, + enable_coverage = enable_coverage, + generate_dwo = generate_dwo, + bitcode_output = bitcode_output, + fdo_context = fdo_context, + auxiliary_fdo_inputs = auxiliary_fdo_inputs, + additional_compilation_inputs = additional_compilation_inputs, + additional_include_scanning_roots = additional_include_scanning_roots, + use_pic = use_pic, + additional_build_variables = additional_build_variables, + action_name = current_action_name, + additional_outputs = additional_outputs, + module_files = module_files, + modmap_file = modmap_file, + modmap_input_file = modmap_input_file, + ) + def _create_cc_compile_actions_with_cpp20_module_helper( *, actions, @@ -813,7 +1040,7 @@ def _create_cc_compile_actions_with_cpp20_module_helper( ) compiled_basenames.add(_basename_without_extension(source_artifact)) - _create_compile_source_action( + _create_compile_module_interface_action( action_construction_context = action_construction_context, cc_compilation_context = cc_compilation_context, label = label, @@ -843,15 +1070,10 @@ def _create_cc_compile_actions_with_cpp20_module_helper( additional_compilation_inputs = additional_compilation_inputs, additional_include_scanning_roots = additional_include_scanning_roots, use_pic = use_pic, - additional_build_variables = { - "cpp_module_output_file": module_file, - "cpp_module_modmap_file": modmap_file, - }, - action_name = ACTION_NAMES.cpp20_module_compile, - additional_outputs = [module_file], module_files = all_other_module_files, modmap_file = modmap_file, modmap_input_file = modmap_input_file, + module_file = module_file, ) all_module_files = depset(direct_module_files, transitive = [transitive_module_files]) diff --git a/src/test/shell/bazel/cc_integration_test.sh b/src/test/shell/bazel/cc_integration_test.sh index 00a722afdc3b51..e92d6948c0ea7e 100755 --- a/src/test/shell/bazel/cc_integration_test.sh +++ b/src/test/shell/bazel/cc_integration_test.sh @@ -2291,6 +2291,15 @@ EOF bazel clean || fail "Expected clean success" bazel build //:main --experimental_cpp_modules --repo_env=CC=clang --copt=-std=c++20 --disk_cache=disk &> $TEST_log || fail "Expected build C++20 Modules success with compiler 'clang'" expect_log "17 disk cache hit" + + # Test with two-phase compilation. enable the following tests after rules_cc updated. + bazel build //:main --experimental_cpp_modules --repo_env=CC=clang --copt=-std=c++20 --features=cpp_modules_with_two_phase_compilation --disk_cache=disk &> $TEST_log || fail "Expected build C++20 Modules success with compiler 'clang'" + + # Verify that the build can hit the cache without action cycles. + bazel clean || fail "Expected clean success" + bazel build //:main --experimental_cpp_modules --repo_env=CC=clang --copt=-std=c++20 --features=cpp_modules_with_two_phase_compilation --disk_cache=disk &> $TEST_log || fail "Expected build C++20 Modules success with compiler 'clang'" + # 3 module interfaces. 17 + 3 = 20 + expect_log "20 disk cache hit" } function test_external_repo_lto() {