[clang] [C++20] [Modules] Introduce -fexperimental-modules-reduced-bmi (PR #85050)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/85050 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Introduce -fexperimental-modules-reduced-bmi (PR #85050)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/85050 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Introduce -fexperimental-modules-reduced-bmi (PR #85050)
ChuanqiXu9 wrote: Got it. I've renamed the flag as `-fexperimental-modules-reduced-bmi`. I feel the suggestion like let users to use `-Xclang` options look odd.. --- > What do you mean by "symmetric"? I mean, we always want the users to do something explicitly to enable the feature. But this might not be important now. https://github.com/llvm/llvm-project/pull/85050 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Introduce -fexperimental-modules-reduced-bmi (PR #85050)
https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/85050 >From b46c8fa030deb980c1550edc0ff8510d4e3c4e17 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Tue, 12 Mar 2024 17:26:49 +0800 Subject: [PATCH 1/3] [C++20] [Modules] Introduce -fgen-reduced-bmi --- clang/include/clang/CodeGen/CodeGenAction.h | 2 + clang/include/clang/Driver/Options.td | 6 +++ .../include/clang/Frontend/FrontendOptions.h | 9 +++- clang/include/clang/Serialization/ASTWriter.h | 7 +-- clang/lib/CodeGen/CodeGenAction.cpp | 19 +++ clang/lib/Driver/Driver.cpp | 27 +- clang/lib/Driver/ToolChains/Clang.cpp | 41 -- clang/lib/Driver/ToolChains/Clang.h | 14 + clang/lib/Frontend/FrontendActions.cpp| 7 +++ clang/lib/Frontend/PrecompiledPreamble.cpp| 3 +- clang/lib/Serialization/GeneratePCH.cpp | 23 ++-- .../test/Driver/module-fgen-reduced-bmi.cppm | 53 +++ clang/test/Modules/gen-reduced-bmi.cppm | 36 + 13 files changed, 220 insertions(+), 27 deletions(-) create mode 100644 clang/test/Driver/module-fgen-reduced-bmi.cppm create mode 100644 clang/test/Modules/gen-reduced-bmi.cppm diff --git a/clang/include/clang/CodeGen/CodeGenAction.h b/clang/include/clang/CodeGen/CodeGenAction.h index 7ad2988e589eb2..186dbb43f01ef7 100644 --- a/clang/include/clang/CodeGen/CodeGenAction.h +++ b/clang/include/clang/CodeGen/CodeGenAction.h @@ -57,6 +57,8 @@ class CodeGenAction : public ASTFrontendAction { bool loadLinkModules(CompilerInstance &CI); protected: + bool BeginSourceFileAction(CompilerInstance &CI) override; + /// Create a new code generation action. If the optional \p _VMContext /// parameter is supplied, the action uses it without taking ownership, /// otherwise it creates a fresh LLVM context and takes ownership. diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 29066ea14280c2..7288bb3db493ba 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -3017,6 +3017,7 @@ defm prebuilt_implicit_modules : BoolFOption<"prebuilt-implicit-modules", def fmodule_output_EQ : Joined<["-"], "fmodule-output=">, Flags<[NoXarchOption]>, Visibility<[ClangOption, CC1Option]>, + MarshallingInfoString>, HelpText<"Save intermediate module file results when compiling a standard C++ module unit.">; def fmodule_output : Flag<["-"], "fmodule-output">, Flags<[NoXarchOption]>, Visibility<[ClangOption, CC1Option]>, @@ -3030,6 +3031,11 @@ defm skip_odr_check_in_gmf : BoolOption<"f", "skip-odr-check-in-gmf", "Perform ODR checks for decls in the global module fragment.">>, Group; +def gen_reduced_bmi : Flag<["-"], "fgen-reduced-bmi">, + Group, Visibility<[ClangOption, CC1Option]>, + HelpText<"Generate the reduced BMI">, + MarshallingInfoFlag>; + def fmodules_prune_interval : Joined<["-"], "fmodules-prune-interval=">, Group, Visibility<[ClangOption, CC1Option]>, MetaVarName<"">, HelpText<"Specify the interval (in seconds) between attempts to prune the module cache">, diff --git a/clang/include/clang/Frontend/FrontendOptions.h b/clang/include/clang/Frontend/FrontendOptions.h index 8085dbcbf671a6..ddfd4f30d1b773 100644 --- a/clang/include/clang/Frontend/FrontendOptions.h +++ b/clang/include/clang/Frontend/FrontendOptions.h @@ -387,6 +387,10 @@ class FrontendOptions { LLVM_PREFERRED_TYPE(bool) unsigned ModulesShareFileManager : 1; + /// Whether to generate reduced BMI for C++20 named modules. + LLVM_PREFERRED_TYPE(bool) + unsigned GenReducedBMI : 1; + CodeCompleteOptions CodeCompleteOpts; /// Specifies the output format of the AST. @@ -553,6 +557,9 @@ class FrontendOptions { /// Path which stores the output files for -ftime-trace std::string TimeTracePath; + /// Output Path for module output file. + std::string ModuleOutputPath; + public: FrontendOptions() : DisableFree(false), RelocatablePCH(false), ShowHelp(false), @@ -565,7 +572,7 @@ class FrontendOptions { BuildingImplicitModuleUsesLock(true), ModulesEmbedAllFiles(false), IncludeTimestamps(true), UseTemporary(true), AllowPCMWithCompilerErrors(false), ModulesShareFileManager(true), -TimeTraceGranularity(500) {} +GenReducedBMI(false), TimeTraceGranularity(500) {} /// getInputKindForExtension - Return the appropriate input kind for a file /// extension. For example, "c" would return Language::C. diff --git a/clang/include/clang/Serialization/ASTWriter.h b/clang/include/clang/Serialization/ASTWriter.h index 3ed9803fa3745b..bd310b6c7a5cdd 100644 --- a/clang/include/clang/Serialization/ASTWriter.h +++ b/clang/include/clang/Serialization/ASTWriter.h @@ -846,7 +846,7 @@ class ASTWriter : public ASTDeserializationListener, /// AST and semantic-analysis consumer that generates a /// precompiled header from
[clang] [C++20] [Modules] Introduce -fexperimental-modules-reduced-bmi (PR #85050)
iains wrote: > Got it. I've renamed the flag as `-fexperimental-modules-reduced-bmi`. Thanks. > I feel the suggestion like let users to use `-Xclang` options look odd.. I think the point here is that we want **expert** users to try this out (with understanding that it might not behave exactly as they expect). Because it is experimental, it is not yet ready for "end users" - I would expect expert users to be able to deal with -Xclang x (but I will not insist, it's just a suggestion) https://github.com/llvm/llvm-project/pull/85050 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Introduce -fexperimental-modules-reduced-bmi (PR #85050)
ChuanqiXu9 wrote: > > Got it. I've renamed the flag as `-fexperimental-modules-reduced-bmi`. > > Thanks. > > > I feel the suggestion like let users to use `-Xclang` options look odd.. > > I think the point here is that we want **expert** users to try this out (with > understanding that it might not behave exactly as they expect). Because it is > experimental, it is not yet ready for "end users" - I would expect expert > users to be able to deal with -Xclang x (but I will not insist, it's just > a suggestion) Agreed. https://github.com/llvm/llvm-project/pull/85050 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Introduce -fexperimental-modules-reduced-bmi (PR #85050)
ChuanqiXu9 wrote: I'd like to land this patch in next week if no objection comes in. https://github.com/llvm/llvm-project/pull/85050 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Introduce -fexperimental-modules-reduced-bmi (PR #85050)
@@ -193,6 +193,20 @@ DwarfFissionKind getDebugFissionKind(const Driver &D, const llvm::opt::ArgList &Args, llvm::opt::Arg *&Arg); +// Calculate the output path of the module file when compiling a module unit +// with the `-fmodule-output` option or `-fmodule-output=` option specified. +// The behavior is: +// - If `-fmodule-output=` is specfied, then the module file is +// writing to the value. +// - Otherwise if the output object file of the module unit is specified, the +// output path +// of the module file should be the same with the output object file except +// the corresponding suffix. This requires both `-o` and `-c` are specified. +// - Otherwise, the output path of the module file will be the same with the +// input with the corresponding suffix. +llvm::SmallString<256> +getCXX20NamedModuleOutputPath(const llvm::opt::ArgList &Args, dwblaikie wrote: Pulling this work out into a named function could probably be a preliminary NFC commit? https://github.com/llvm/llvm-project/pull/85050 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Introduce -fexperimental-modules-reduced-bmi (PR #85050)
@@ -3017,6 +3017,7 @@ defm prebuilt_implicit_modules : BoolFOption<"prebuilt-implicit-modules", def fmodule_output_EQ : Joined<["-"], "fmodule-output=">, Flags<[NoXarchOption]>, Visibility<[ClangOption, CC1Option]>, + MarshallingInfoString>, dwblaikie wrote: What's the motivation to include this change in this patch? Could it be separated? https://github.com/llvm/llvm-project/pull/85050 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Introduce -fexperimental-modules-reduced-bmi (PR #85050)
https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/85050 >From 79706501a7a3f0f2e0e9c9411bdd5e00e34ae175 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Tue, 12 Mar 2024 17:26:49 +0800 Subject: [PATCH 1/3] [C++20] [Modules] Introduce -fgen-reduced-bmi --- clang/include/clang/CodeGen/CodeGenAction.h | 2 + clang/include/clang/Driver/Options.td | 6 +++ .../include/clang/Frontend/FrontendOptions.h | 9 +++- clang/include/clang/Serialization/ASTWriter.h | 7 +-- clang/lib/CodeGen/CodeGenAction.cpp | 19 +++ clang/lib/Driver/Driver.cpp | 12 - clang/lib/Driver/ToolChains/Clang.cpp | 23 ++-- clang/lib/Frontend/FrontendActions.cpp| 7 +++ clang/lib/Frontend/PrecompiledPreamble.cpp| 3 +- clang/lib/Serialization/GeneratePCH.cpp | 23 ++-- .../test/Driver/module-fgen-reduced-bmi.cppm | 53 +++ clang/test/Modules/gen-reduced-bmi.cppm | 36 + 12 files changed, 186 insertions(+), 14 deletions(-) create mode 100644 clang/test/Driver/module-fgen-reduced-bmi.cppm create mode 100644 clang/test/Modules/gen-reduced-bmi.cppm diff --git a/clang/include/clang/CodeGen/CodeGenAction.h b/clang/include/clang/CodeGen/CodeGenAction.h index 7ad2988e589eb2..186dbb43f01ef7 100644 --- a/clang/include/clang/CodeGen/CodeGenAction.h +++ b/clang/include/clang/CodeGen/CodeGenAction.h @@ -57,6 +57,8 @@ class CodeGenAction : public ASTFrontendAction { bool loadLinkModules(CompilerInstance &CI); protected: + bool BeginSourceFileAction(CompilerInstance &CI) override; + /// Create a new code generation action. If the optional \p _VMContext /// parameter is supplied, the action uses it without taking ownership, /// otherwise it creates a fresh LLVM context and takes ownership. diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index f5289fb00c895e..3b920db56586f2 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -3017,6 +3017,7 @@ defm prebuilt_implicit_modules : BoolFOption<"prebuilt-implicit-modules", def fmodule_output_EQ : Joined<["-"], "fmodule-output=">, Flags<[NoXarchOption]>, Visibility<[ClangOption, CC1Option]>, + MarshallingInfoString>, HelpText<"Save intermediate module file results when compiling a standard C++ module unit.">; def fmodule_output : Flag<["-"], "fmodule-output">, Flags<[NoXarchOption]>, Visibility<[ClangOption, CC1Option]>, @@ -3030,6 +3031,11 @@ defm skip_odr_check_in_gmf : BoolOption<"f", "skip-odr-check-in-gmf", "Perform ODR checks for decls in the global module fragment.">>, Group; +def gen_reduced_bmi : Flag<["-"], "fgen-reduced-bmi">, + Group, Visibility<[ClangOption, CC1Option]>, + HelpText<"Generate the reduced BMI">, + MarshallingInfoFlag>; + def fmodules_prune_interval : Joined<["-"], "fmodules-prune-interval=">, Group, Visibility<[ClangOption, CC1Option]>, MetaVarName<"">, HelpText<"Specify the interval (in seconds) between attempts to prune the module cache">, diff --git a/clang/include/clang/Frontend/FrontendOptions.h b/clang/include/clang/Frontend/FrontendOptions.h index 8085dbcbf671a6..ddfd4f30d1b773 100644 --- a/clang/include/clang/Frontend/FrontendOptions.h +++ b/clang/include/clang/Frontend/FrontendOptions.h @@ -387,6 +387,10 @@ class FrontendOptions { LLVM_PREFERRED_TYPE(bool) unsigned ModulesShareFileManager : 1; + /// Whether to generate reduced BMI for C++20 named modules. + LLVM_PREFERRED_TYPE(bool) + unsigned GenReducedBMI : 1; + CodeCompleteOptions CodeCompleteOpts; /// Specifies the output format of the AST. @@ -553,6 +557,9 @@ class FrontendOptions { /// Path which stores the output files for -ftime-trace std::string TimeTracePath; + /// Output Path for module output file. + std::string ModuleOutputPath; + public: FrontendOptions() : DisableFree(false), RelocatablePCH(false), ShowHelp(false), @@ -565,7 +572,7 @@ class FrontendOptions { BuildingImplicitModuleUsesLock(true), ModulesEmbedAllFiles(false), IncludeTimestamps(true), UseTemporary(true), AllowPCMWithCompilerErrors(false), ModulesShareFileManager(true), -TimeTraceGranularity(500) {} +GenReducedBMI(false), TimeTraceGranularity(500) {} /// getInputKindForExtension - Return the appropriate input kind for a file /// extension. For example, "c" would return Language::C. diff --git a/clang/include/clang/Serialization/ASTWriter.h b/clang/include/clang/Serialization/ASTWriter.h index 3ed9803fa3745b..bd310b6c7a5cdd 100644 --- a/clang/include/clang/Serialization/ASTWriter.h +++ b/clang/include/clang/Serialization/ASTWriter.h @@ -846,7 +846,7 @@ class ASTWriter : public ASTDeserializationListener, /// AST and semantic-analysis consumer that generates a /// precompiled header from the parsed source code. class PCHGenerator : public SemaConsumer { -
[clang] [C++20] [Modules] Introduce -fexperimental-modules-reduced-bmi (PR #85050)
@@ -193,6 +193,20 @@ DwarfFissionKind getDebugFissionKind(const Driver &D, const llvm::opt::ArgList &Args, llvm::opt::Arg *&Arg); +// Calculate the output path of the module file when compiling a module unit +// with the `-fmodule-output` option or `-fmodule-output=` option specified. +// The behavior is: +// - If `-fmodule-output=` is specfied, then the module file is +// writing to the value. +// - Otherwise if the output object file of the module unit is specified, the +// output path +// of the module file should be the same with the output object file except +// the corresponding suffix. This requires both `-o` and `-c` are specified. +// - Otherwise, the output path of the module file will be the same with the +// input with the corresponding suffix. +llvm::SmallString<256> +getCXX20NamedModuleOutputPath(const llvm::opt::ArgList &Args, ChuanqiXu9 wrote: Done in https://github.com/llvm/llvm-project/commit/21f85e230056172cffcaec76352e5a2019b54b86 https://github.com/llvm/llvm-project/pull/85050 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Introduce -fexperimental-modules-reduced-bmi (PR #85050)
@@ -3017,6 +3017,7 @@ defm prebuilt_implicit_modules : BoolFOption<"prebuilt-implicit-modules", def fmodule_output_EQ : Joined<["-"], "fmodule-output=">, Flags<[NoXarchOption]>, Visibility<[ClangOption, CC1Option]>, + MarshallingInfoString>, ChuanqiXu9 wrote: Since the flag was only used in drivers. So we didn't need a variable. But after this patch, we need a variable. If we separate this out, we may get a unused variable in that patch. I feel it is not so good. https://github.com/llvm/llvm-project/pull/85050 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Introduce -fexperimental-modules-reduced-bmi (PR #85050)
https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/85050 >From 8c51aa534a20655a6a86a84f1050efd850130afd Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Tue, 12 Mar 2024 17:26:49 +0800 Subject: [PATCH] [C++20] [Modules] Introduce -fgen-reduced-bmi --- clang/include/clang/CodeGen/CodeGenAction.h | 2 + clang/include/clang/Driver/Options.td | 6 +++ .../include/clang/Frontend/FrontendOptions.h | 11 clang/lib/CodeGen/CodeGenAction.cpp | 19 +++ clang/lib/Driver/Driver.cpp | 12 - clang/lib/Driver/ToolChains/Clang.cpp | 18 +++ clang/lib/Frontend/FrontendActions.cpp| 7 +++ .../test/Driver/module-fgen-reduced-bmi.cppm | 51 +++ clang/test/Modules/modules-reduced-bmi.cppm | 36 + 9 files changed, 161 insertions(+), 1 deletion(-) create mode 100644 clang/test/Driver/module-fgen-reduced-bmi.cppm create mode 100644 clang/test/Modules/modules-reduced-bmi.cppm diff --git a/clang/include/clang/CodeGen/CodeGenAction.h b/clang/include/clang/CodeGen/CodeGenAction.h index 7ad2988e589eb2..186dbb43f01ef7 100644 --- a/clang/include/clang/CodeGen/CodeGenAction.h +++ b/clang/include/clang/CodeGen/CodeGenAction.h @@ -57,6 +57,8 @@ class CodeGenAction : public ASTFrontendAction { bool loadLinkModules(CompilerInstance &CI); protected: + bool BeginSourceFileAction(CompilerInstance &CI) override; + /// Create a new code generation action. If the optional \p _VMContext /// parameter is supplied, the action uses it without taking ownership, /// otherwise it creates a fresh LLVM context and takes ownership. diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 827d9d7c0c18e4..09feb1c775e2e8 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -3034,6 +3034,7 @@ defm prebuilt_implicit_modules : BoolFOption<"prebuilt-implicit-modules", def fmodule_output_EQ : Joined<["-"], "fmodule-output=">, Flags<[NoXarchOption]>, Visibility<[ClangOption, CC1Option]>, + MarshallingInfoString>, HelpText<"Save intermediate module file results when compiling a standard C++ module unit.">; def fmodule_output : Flag<["-"], "fmodule-output">, Flags<[NoXarchOption]>, Visibility<[ClangOption, CC1Option]>, @@ -3047,6 +3048,11 @@ defm skip_odr_check_in_gmf : BoolOption<"f", "skip-odr-check-in-gmf", "Perform ODR checks for decls in the global module fragment.">>, Group; +def modules_reduced_bmi : Flag<["-"], "fexperimental-modules-reduced-bmi">, + Group, Visibility<[ClangOption, CC1Option]>, + HelpText<"Generate the reduced BMI">, + MarshallingInfoFlag>; + def fmodules_prune_interval : Joined<["-"], "fmodules-prune-interval=">, Group, Visibility<[ClangOption, CC1Option]>, MetaVarName<"">, HelpText<"Specify the interval (in seconds) between attempts to prune the module cache">, diff --git a/clang/include/clang/Frontend/FrontendOptions.h b/clang/include/clang/Frontend/FrontendOptions.h index 5ee4d471670f48..5479d13aab 100644 --- a/clang/include/clang/Frontend/FrontendOptions.h +++ b/clang/include/clang/Frontend/FrontendOptions.h @@ -404,6 +404,10 @@ class FrontendOptions { LLVM_PREFERRED_TYPE(bool) unsigned EmitPrettySymbolGraphs : 1; + /// Whether to generate reduced BMI for C++20 named modules. + LLVM_PREFERRED_TYPE(bool) + unsigned GenReducedBMI : 1; + CodeCompleteOptions CodeCompleteOpts; /// Specifies the output format of the AST. @@ -568,6 +572,9 @@ class FrontendOptions { /// Path which stores the output files for -ftime-trace std::string TimeTracePath; + /// Output Path for module output file. + std::string ModuleOutputPath; + public: FrontendOptions() : DisableFree(false), RelocatablePCH(false), ShowHelp(false), @@ -580,9 +587,13 @@ class FrontendOptions { BuildingImplicitModuleUsesLock(true), ModulesEmbedAllFiles(false), IncludeTimestamps(true), UseTemporary(true), AllowPCMWithCompilerErrors(false), ModulesShareFileManager(true), +<<< HEAD EmitSymbolGraph(false), EmitExtensionSymbolGraphs(false), EmitSymbolGraphSymbolLabelsForTesting(false), EmitPrettySymbolGraphs(false), TimeTraceGranularity(500) {} +=== +GenReducedBMI(false), TimeTraceGranularity(500) {} +>>> 8baafcdac143 ([C++20] [Modules] Introduce -fgen-reduced-bmi) /// getInputKindForExtension - Return the appropriate input kind for a file /// extension. For example, "c" would return Language::C. diff --git a/clang/lib/CodeGen/CodeGenAction.cpp b/clang/lib/CodeGen/CodeGenAction.cpp index bb9aaba025fa59..cd0d011e8c72b0 100644 --- a/clang/lib/CodeGen/CodeGenAction.cpp +++ b/clang/lib/CodeGen/CodeGenAction.cpp @@ -25,8 +25,11 @@ #include "clang/CodeGen/ModuleBuilder.h" #include "clang/Driver/DriverDiagnostic.h" #include "clang/Frontend/CompilerInstance.h" +#include "clang/Frontend/
[clang] [C++20] [Modules] Introduce -fexperimental-modules-reduced-bmi (PR #85050)
ChuanqiXu9 wrote: Rebased with main. @dwblaikie may you have more comments? https://github.com/llvm/llvm-project/pull/85050 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Introduce -fexperimental-modules-reduced-bmi (PR #85050)
https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/85050 >From 7b0b9b52ac06a78a172e3628ab7fb3cdb3749d13 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Tue, 12 Mar 2024 17:26:49 +0800 Subject: [PATCH] [C++20] [Modules] Introduce -fgen-reduced-bmi --- clang/include/clang/CodeGen/CodeGenAction.h | 2 + clang/include/clang/Driver/Options.td | 6 +++ .../include/clang/Frontend/FrontendOptions.h | 10 +++- clang/lib/CodeGen/CodeGenAction.cpp | 19 +++ clang/lib/Driver/Driver.cpp | 12 - clang/lib/Driver/ToolChains/Clang.cpp | 18 +++ clang/lib/Frontend/FrontendActions.cpp| 7 +++ .../test/Driver/module-fgen-reduced-bmi.cppm | 51 +++ clang/test/Modules/modules-reduced-bmi.cppm | 36 + 9 files changed, 159 insertions(+), 2 deletions(-) create mode 100644 clang/test/Driver/module-fgen-reduced-bmi.cppm create mode 100644 clang/test/Modules/modules-reduced-bmi.cppm diff --git a/clang/include/clang/CodeGen/CodeGenAction.h b/clang/include/clang/CodeGen/CodeGenAction.h index 7ad2988e589eb2..186dbb43f01ef7 100644 --- a/clang/include/clang/CodeGen/CodeGenAction.h +++ b/clang/include/clang/CodeGen/CodeGenAction.h @@ -57,6 +57,8 @@ class CodeGenAction : public ASTFrontendAction { bool loadLinkModules(CompilerInstance &CI); protected: + bool BeginSourceFileAction(CompilerInstance &CI) override; + /// Create a new code generation action. If the optional \p _VMContext /// parameter is supplied, the action uses it without taking ownership, /// otherwise it creates a fresh LLVM context and takes ownership. diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 827d9d7c0c18e4..09feb1c775e2e8 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -3034,6 +3034,7 @@ defm prebuilt_implicit_modules : BoolFOption<"prebuilt-implicit-modules", def fmodule_output_EQ : Joined<["-"], "fmodule-output=">, Flags<[NoXarchOption]>, Visibility<[ClangOption, CC1Option]>, + MarshallingInfoString>, HelpText<"Save intermediate module file results when compiling a standard C++ module unit.">; def fmodule_output : Flag<["-"], "fmodule-output">, Flags<[NoXarchOption]>, Visibility<[ClangOption, CC1Option]>, @@ -3047,6 +3048,11 @@ defm skip_odr_check_in_gmf : BoolOption<"f", "skip-odr-check-in-gmf", "Perform ODR checks for decls in the global module fragment.">>, Group; +def modules_reduced_bmi : Flag<["-"], "fexperimental-modules-reduced-bmi">, + Group, Visibility<[ClangOption, CC1Option]>, + HelpText<"Generate the reduced BMI">, + MarshallingInfoFlag>; + def fmodules_prune_interval : Joined<["-"], "fmodules-prune-interval=">, Group, Visibility<[ClangOption, CC1Option]>, MetaVarName<"">, HelpText<"Specify the interval (in seconds) between attempts to prune the module cache">, diff --git a/clang/include/clang/Frontend/FrontendOptions.h b/clang/include/clang/Frontend/FrontendOptions.h index 5ee4d471670f48..a738c1f3757682 100644 --- a/clang/include/clang/Frontend/FrontendOptions.h +++ b/clang/include/clang/Frontend/FrontendOptions.h @@ -404,6 +404,10 @@ class FrontendOptions { LLVM_PREFERRED_TYPE(bool) unsigned EmitPrettySymbolGraphs : 1; + /// Whether to generate reduced BMI for C++20 named modules. + LLVM_PREFERRED_TYPE(bool) + unsigned GenReducedBMI : 1; + CodeCompleteOptions CodeCompleteOpts; /// Specifies the output format of the AST. @@ -568,6 +572,9 @@ class FrontendOptions { /// Path which stores the output files for -ftime-trace std::string TimeTracePath; + /// Output Path for module output file. + std::string ModuleOutputPath; + public: FrontendOptions() : DisableFree(false), RelocatablePCH(false), ShowHelp(false), @@ -582,7 +589,8 @@ class FrontendOptions { AllowPCMWithCompilerErrors(false), ModulesShareFileManager(true), EmitSymbolGraph(false), EmitExtensionSymbolGraphs(false), EmitSymbolGraphSymbolLabelsForTesting(false), -EmitPrettySymbolGraphs(false), TimeTraceGranularity(500) {} +EmitPrettySymbolGraphs(false), GenReducedBMI(false), +TimeTraceGranularity(500) {} /// getInputKindForExtension - Return the appropriate input kind for a file /// extension. For example, "c" would return Language::C. diff --git a/clang/lib/CodeGen/CodeGenAction.cpp b/clang/lib/CodeGen/CodeGenAction.cpp index bb9aaba025fa59..cd0d011e8c72b0 100644 --- a/clang/lib/CodeGen/CodeGenAction.cpp +++ b/clang/lib/CodeGen/CodeGenAction.cpp @@ -25,8 +25,11 @@ #include "clang/CodeGen/ModuleBuilder.h" #include "clang/Driver/DriverDiagnostic.h" #include "clang/Frontend/CompilerInstance.h" +#include "clang/Frontend/FrontendActions.h" #include "clang/Frontend/FrontendDiagnostic.h" +#include "clang/Frontend/MultiplexConsumer.h" #include "clang/Lex/Preprocessor.h" +#include "clang/Serialization/A
[clang] [C++20] [Modules] Introduce -fexperimental-modules-reduced-bmi (PR #85050)
@@ -1061,6 +1070,16 @@ CodeGenAction::CreateASTConsumer(CompilerInstance &CI, StringRef InFile) { CI.getPreprocessor().addPPCallbacks(std::move(Callbacks)); } + if (CI.getFrontendOpts().GenReducedBMI && + !CI.getFrontendOpts().ModuleOutputPath.empty()) { +std::vector> Consumers{2}; +Consumers[0] = std::make_unique( +CI.getPreprocessor(), CI.getModuleCache(), +CI.getFrontendOpts().ModuleOutputPath); +Consumers[1] = std::move(Result); +return std::make_unique(std::move(Consumers)); + } dwblaikie wrote: Why is this new code, rather than an alternative codepath of existing code? (like some code that was producing the BMI would go from producing the unreduced BMI to producing the reduced BMI) Figuring this out would help me unedrstand why `ModuleOutputPath` is new (this patch doesn't add the functionality to output a module to a path - we had that already, this just changes the content that goes there) I guess because maybe this patch is changing the way we output the BMI - or that the reduced BMI is being output in a different way than the non-reduced BMI? Should we address that? Make the current full BMI emission work the same way as the reduced BMI? (or, more precisely, in a pre-patch, change the full BMI emission to use a mechanism that can then be reused for the reduced BMI?) https://github.com/llvm/llvm-project/pull/85050 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Introduce -fexperimental-modules-reduced-bmi (PR #85050)
https://github.com/dwblaikie approved this pull request. Give it a go https://github.com/llvm/llvm-project/pull/85050 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Introduce -fexperimental-modules-reduced-bmi (PR #85050)
https://github.com/dwblaikie edited https://github.com/llvm/llvm-project/pull/85050 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Introduce -fexperimental-modules-reduced-bmi (PR #85050)
@@ -1061,6 +1070,16 @@ CodeGenAction::CreateASTConsumer(CompilerInstance &CI, StringRef InFile) { CI.getPreprocessor().addPPCallbacks(std::move(Callbacks)); } + if (CI.getFrontendOpts().GenReducedBMI && + !CI.getFrontendOpts().ModuleOutputPath.empty()) { +std::vector> Consumers{2}; mizvekov wrote: Minor nit: https://llvm.org/docs/CodingStandards.html#id28 ```suggestion std::vector> Consumers(2); ``` https://github.com/llvm/llvm-project/pull/85050 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Introduce -fexperimental-modules-reduced-bmi (PR #85050)
https://github.com/mizvekov edited https://github.com/llvm/llvm-project/pull/85050 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Introduce -fexperimental-modules-reduced-bmi (PR #85050)
@@ -4045,6 +4045,24 @@ static bool RenderModulesOptions(Compilation &C, const Driver &D, // module fragment. CmdArgs.push_back("-fskip-odr-check-in-gmf"); + if (Args.hasArg(options::OPT_modules_reduced_bmi) && + (Input.getType() == driver::types::TY_CXXModule || + Input.getType() == driver::types::TY_PP_CXXModule)) { +CmdArgs.push_back("-fexperimental-modules-reduced-bmi"); + +if (Args.hasArg(options::OPT_fmodule_output_EQ)) + Args.AddLastArg(CmdArgs, options::OPT_fmodule_output_EQ); +else + CmdArgs.push_back(Args.MakeArgString( + "-fmodule-output=" + + getCXX20NamedModuleOutputPath(Args, Input.getBaseInput(; mizvekov wrote: Would it be reasonable to push two separate arguments, instead of concatenating them? https://github.com/llvm/llvm-project/pull/85050 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Introduce -fexperimental-modules-reduced-bmi (PR #85050)
https://github.com/mizvekov approved this pull request. I share the objections that it may be too soon to introduce a driver flag for this. Only a frontend flag is ok for now, but it's non blocking on my side because it doesn't look like it will be particularly hard to deprecate it later. https://github.com/llvm/llvm-project/pull/85050 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Introduce -fexperimental-modules-reduced-bmi (PR #85050)
https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/85050 >From 59f281786a8c84ecfd7f5ddcfce6251c43ef2c72 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Tue, 12 Mar 2024 17:26:49 +0800 Subject: [PATCH] [C++20] [Modules] Introduce -fgen-reduced-bmi --- clang/include/clang/CodeGen/CodeGenAction.h | 2 + clang/include/clang/Driver/Options.td | 6 +++ .../include/clang/Frontend/FrontendOptions.h | 10 +++- clang/lib/CodeGen/CodeGenAction.cpp | 19 +++ clang/lib/Driver/Driver.cpp | 12 - clang/lib/Driver/ToolChains/Clang.cpp | 18 +++ clang/lib/Frontend/FrontendActions.cpp| 7 +++ .../test/Driver/module-fgen-reduced-bmi.cppm | 51 +++ clang/test/Modules/modules-reduced-bmi.cppm | 36 + 9 files changed, 159 insertions(+), 2 deletions(-) create mode 100644 clang/test/Driver/module-fgen-reduced-bmi.cppm create mode 100644 clang/test/Modules/modules-reduced-bmi.cppm diff --git a/clang/include/clang/CodeGen/CodeGenAction.h b/clang/include/clang/CodeGen/CodeGenAction.h index 7ad2988e589eb2..186dbb43f01ef7 100644 --- a/clang/include/clang/CodeGen/CodeGenAction.h +++ b/clang/include/clang/CodeGen/CodeGenAction.h @@ -57,6 +57,8 @@ class CodeGenAction : public ASTFrontendAction { bool loadLinkModules(CompilerInstance &CI); protected: + bool BeginSourceFileAction(CompilerInstance &CI) override; + /// Create a new code generation action. If the optional \p _VMContext /// parameter is supplied, the action uses it without taking ownership, /// otherwise it creates a fresh LLVM context and takes ownership. diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 9a0b5d3304ca6e..e24626913add76 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -3036,6 +3036,7 @@ defm prebuilt_implicit_modules : BoolFOption<"prebuilt-implicit-modules", def fmodule_output_EQ : Joined<["-"], "fmodule-output=">, Flags<[NoXarchOption]>, Visibility<[ClangOption, CC1Option]>, + MarshallingInfoString>, HelpText<"Save intermediate module file results when compiling a standard C++ module unit.">; def fmodule_output : Flag<["-"], "fmodule-output">, Flags<[NoXarchOption]>, Visibility<[ClangOption, CC1Option]>, @@ -3049,6 +3050,11 @@ defm skip_odr_check_in_gmf : BoolOption<"f", "skip-odr-check-in-gmf", "Perform ODR checks for decls in the global module fragment.">>, Group; +def modules_reduced_bmi : Flag<["-"], "fexperimental-modules-reduced-bmi">, + Group, Visibility<[ClangOption, CC1Option]>, + HelpText<"Generate the reduced BMI">, + MarshallingInfoFlag>; + def fmodules_prune_interval : Joined<["-"], "fmodules-prune-interval=">, Group, Visibility<[ClangOption, CC1Option]>, MetaVarName<"">, HelpText<"Specify the interval (in seconds) between attempts to prune the module cache">, diff --git a/clang/include/clang/Frontend/FrontendOptions.h b/clang/include/clang/Frontend/FrontendOptions.h index 5ee4d471670f48..a738c1f3757682 100644 --- a/clang/include/clang/Frontend/FrontendOptions.h +++ b/clang/include/clang/Frontend/FrontendOptions.h @@ -404,6 +404,10 @@ class FrontendOptions { LLVM_PREFERRED_TYPE(bool) unsigned EmitPrettySymbolGraphs : 1; + /// Whether to generate reduced BMI for C++20 named modules. + LLVM_PREFERRED_TYPE(bool) + unsigned GenReducedBMI : 1; + CodeCompleteOptions CodeCompleteOpts; /// Specifies the output format of the AST. @@ -568,6 +572,9 @@ class FrontendOptions { /// Path which stores the output files for -ftime-trace std::string TimeTracePath; + /// Output Path for module output file. + std::string ModuleOutputPath; + public: FrontendOptions() : DisableFree(false), RelocatablePCH(false), ShowHelp(false), @@ -582,7 +589,8 @@ class FrontendOptions { AllowPCMWithCompilerErrors(false), ModulesShareFileManager(true), EmitSymbolGraph(false), EmitExtensionSymbolGraphs(false), EmitSymbolGraphSymbolLabelsForTesting(false), -EmitPrettySymbolGraphs(false), TimeTraceGranularity(500) {} +EmitPrettySymbolGraphs(false), GenReducedBMI(false), +TimeTraceGranularity(500) {} /// getInputKindForExtension - Return the appropriate input kind for a file /// extension. For example, "c" would return Language::C. diff --git a/clang/lib/CodeGen/CodeGenAction.cpp b/clang/lib/CodeGen/CodeGenAction.cpp index bb9aaba025fa59..1a6b628016f746 100644 --- a/clang/lib/CodeGen/CodeGenAction.cpp +++ b/clang/lib/CodeGen/CodeGenAction.cpp @@ -25,8 +25,11 @@ #include "clang/CodeGen/ModuleBuilder.h" #include "clang/Driver/DriverDiagnostic.h" #include "clang/Frontend/CompilerInstance.h" +#include "clang/Frontend/FrontendActions.h" #include "clang/Frontend/FrontendDiagnostic.h" +#include "clang/Frontend/MultiplexConsumer.h" #include "clang/Lex/Preprocessor.h" +#include "clang/Serialization/A
[clang] [C++20] [Modules] Introduce -fexperimental-modules-reduced-bmi (PR #85050)
@@ -4045,6 +4045,24 @@ static bool RenderModulesOptions(Compilation &C, const Driver &D, // module fragment. CmdArgs.push_back("-fskip-odr-check-in-gmf"); + if (Args.hasArg(options::OPT_modules_reduced_bmi) && + (Input.getType() == driver::types::TY_CXXModule || + Input.getType() == driver::types::TY_PP_CXXModule)) { +CmdArgs.push_back("-fexperimental-modules-reduced-bmi"); + +if (Args.hasArg(options::OPT_fmodule_output_EQ)) + Args.AddLastArg(CmdArgs, options::OPT_fmodule_output_EQ); +else + CmdArgs.push_back(Args.MakeArgString( + "-fmodule-output=" + + getCXX20NamedModuleOutputPath(Args, Input.getBaseInput(; ChuanqiXu9 wrote: I am not sure. I tried to not concat it but the test gets failing. I didn't see why but I see almost we always concat it in such cases (appending a variable to some flag ends with `=`). So I feel it may not be problematic or we can fix such things together if we want. https://github.com/llvm/llvm-project/pull/85050 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Introduce -fexperimental-modules-reduced-bmi (PR #85050)
@@ -1061,6 +1070,16 @@ CodeGenAction::CreateASTConsumer(CompilerInstance &CI, StringRef InFile) { CI.getPreprocessor().addPPCallbacks(std::move(Callbacks)); } + if (CI.getFrontendOpts().GenReducedBMI && + !CI.getFrontendOpts().ModuleOutputPath.empty()) { +std::vector> Consumers{2}; +Consumers[0] = std::make_unique( +CI.getPreprocessor(), CI.getModuleCache(), +CI.getFrontendOpts().ModuleOutputPath); +Consumers[1] = std::move(Result); +return std::make_unique(std::move(Consumers)); + } ChuanqiXu9 wrote: For full BMI, the workflow is: ``` .cpp -> .pcm -> .o ``` no matter if `-fmodule-output` is specified or not. If `-fmodule-output` is not specified, the `.pcm` file will be generated in `/tmp` directory. And for reduced BMI, the workflow is: ``` .cpp -> .o -> .pcm ``` that said, we generate the .o directly from the .cpp then we don't need the .pcm to contain the full information to generate the .o . Then for the original question, can we make the full BMI in the same way with the reduced BMI? Maybe we can, but it looks like an overkill. Since if the .o don't need the .pcm, we don't need the .pcm to be full. https://github.com/llvm/llvm-project/pull/85050 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Introduce -fexperimental-modules-reduced-bmi (PR #85050)
ChuanqiXu9 wrote: Thanks for reviewing. https://github.com/llvm/llvm-project/pull/85050 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Introduce -fexperimental-modules-reduced-bmi (PR #85050)
https://github.com/ChuanqiXu9 closed https://github.com/llvm/llvm-project/pull/85050 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits