[clang] [C++20] [Modules] Introduce -fmodules-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 );
protected:
+ bool BeginSourceFileAction(CompilerInstance ) 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 the
[clang] [C++20] [Modules] Introduce -fmodules-reduced-bmi (PR #85050)
dwblaikie wrote: > > > > > @iains @dwblaikie Understood. And I thought the major problem is that > > > > > there are a lot flags from clang modules. And it is indeed confusing. > > > > > But given we have to introduce new flags in this case, probably we > > > > > can only try to make it more clear by better documents. > > > > > > > > > > > > So you do not think it possible to restrict the new flag to be > > > > "internal" (i.e. cc1-only) and to put some _temporary_ driver > > > > processing to handle that? (I agree that this is an unusual mechanism, > > > > but the idea is to recognise that the driver-side processing is only > > > > expected to me temporary). > > > > > > > > > I have no idea how can we make that. We still need the users to provide > > > something to enable reduced BMI. And I think it is symmetric to a new > > > flag. > > > > > > What I mean is that (a) we need the internal 'cc1' flag permanently; but > > (b) we do not expect to need a user-facing driver flag permanently. and (c) > > We want to allow users to try this out. > > I am suggesting we could say "to try this out use -Xclang > > -fmodules-reduced-bmi" and have _temporary_ code in the driver to deal with > > the changes needed to phasing. > > If this is not possible. then I suppose I am a bit sad that we keep saying > > 'there are too many modules options' - but yet still add more. however - we > > need to make progress, so if the suggestion here is really a non-starter .. > > then such is life. > > Perhaps the second suggestion (-fexperimental-x options) could be > > discussed at the project level. > > Got your point. But I feel the `-Xclang xxx` style or the > `-fexperimental-xxx` style may make people feel, "oh, this is not ready, > let's wait". Then we may lose more testing opportunity. That seems good to me - these are pretty experimental directions we're going in. We haven't figured out how this stuff should work long term - we are experimenting. > Also I feel such options are symmetric to the existing one. What do you mean by "symmetric"? The difference @iains is trying to highlight is that adding driver flags is a long-term commitment burden - adding cc1 flags, clarifying that these are not long term supported/guaranteed parts of the interface, but a way to experiment with some things until we figure out what the long term supported interface is. Especially if we think the long-term future is always (or, at least by default) reduced BMIs, experimenting with it under a flag until we have confidence in that direction - then maybe just changing the Clang behavior (again, perhaps with a cc1 flag to opt-out as an escape hatch that's intended to be short-term while we figure out whatever bugs lead to the need to use that escape hatch - then when we fixt he bugs and remove the flag, the people on the bleeding edge will need to remove the flag, which is fine/good - rather than leaving these weird flags around forever). 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 -fmodules-reduced-bmi (PR #85050)
ChuanqiXu9 wrote: > > > > @iains @dwblaikie Understood. And I thought the major problem is that > > > > there are a lot flags from clang modules. And it is indeed confusing. > > > > But given we have to introduce new flags in this case, probably we can > > > > only try to make it more clear by better documents. > > > > > > > > > So you do not think it possible to restrict the new flag to be "internal" > > > (i.e. cc1-only) and to put some _temporary_ driver processing to handle > > > that? (I agree that this is an unusual mechanism, but the idea is to > > > recognise that the driver-side processing is only expected to me > > > temporary). > > > > > > I have no idea how can we make that. We still need the users to provide > > something to enable reduced BMI. And I think it is symmetric to a new flag. > > What I mean is that (a) we need the internal 'cc1' flag permanently; but (b) > we do not expect to need a user-facing driver flag permanently. and (c) We > want to allow users to try this out. > > I am suggesting we could say "to try this out use -Xclang > -fmodules-reduced-bmi" and have _temporary_ code in the driver to deal with > the changes needed to phasing. > > If this is not possible. then I suppose I am a bit sad that we keep saying > 'there are too many modules options' - but yet still add more. however - we > need to make progress, so if the suggestion here is really a non-starter .. > then such is life. > > Perhaps the second suggestion (-fexperimental-x options) could be > discussed at the project level. Got your point. But I feel the `-Xclang xxx` style or the `-fexperimental-xxx` style may make people feel, "oh, this is not ready, let's wait". Then we may lose more testing opportunity. Also I feel such options are symmetric to the existing one. It still makes the interfaces to be more complicated. I don't feel they are better than the existing one. And yes, it is indeed that we have a lot modules options. But I think this may not be the blocking point for adding new modules options if we think it is really needed. If there are things needed to be added by flags really and we don't do that due to we feel we have too many flags, I feel it is somewhat "not eating for fear of choking". I think what we need to do for this case is to be pretty careful when adding new flags and I think we already make it for 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 -fmodules-reduced-bmi (PR #85050)
iains wrote: > > > @iains @dwblaikie Understood. And I thought the major problem is that > > > there are a lot flags from clang modules. And it is indeed confusing. But > > > given we have to introduce new flags in this case, probably we can only > > > try to make it more clear by better documents. > > > > > > So you do not think it possible to restrict the new flag to be "internal" > > (i.e. cc1-only) and to put some _temporary_ driver processing to handle > > that? (I agree that this is an unusual mechanism, but the idea is to > > recognise that the driver-side processing is only expected to me temporary). > > I have no idea how can we make that. We still need the users to provide > something to enable reduced BMI. And I think it is symmetric to a new flag. What I mean is that (a) we need the internal 'cc1' flag permanently; but (b) we do not expect to need a user-facing driver flag permanently. and (c) We want to allow users to try this out. I am suggesting we could say "to try this out use -Xclang -fmodules-reduced-bmi" and have _temporary_ code in the driver to deal with the changes needed to phasing. If this is not possible. then I suppose I am a bit sad that we keep saying 'there are too many modules options' - but yet still add more. however - we need to make progress, so if the suggestion here is really a non-starter .. then such is life. Perhaps the second suggestion (-fexperimental-x options) could be discussed at the project level. 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 -fmodules-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 -fmodules-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
