-
Notifications
You must be signed in to change notification settings - Fork 13.7k
[C++20] [Modules] Introduce -fexperimental-modules-reduced-bmi #85050
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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()))); | ||
Comment on lines
+4057
to
+4058
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it be reasonable to push two separate arguments, instead of concatenating them? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
} | ||
|
||
// Noop if we see '-fexperimental-modules-reduced-bmi' with other translation | ||
// units than module units. This is more user friendly to allow end uers to | ||
// enable this feature without asking for help from build systems. | ||
Args.ClaimAllArgs(options::OPT_modules_reduced_bmi); | ||
|
||
// We need to include the case the input file is a module file here. | ||
// Since the default compilation model for C++ module interface unit will | ||
// create temporary module file and compile the temporary module file | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
// It is annoying to handle different slash direction | ||
// in Windows and Linux. So we disable the test on Windows | ||
// here. | ||
// REQUIRES: !system-windows | ||
ChuanqiXu9 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// On AIX, the default output for `-c` may be `.s` instead of `.o`, | ||
// which makes the test fail. So disable the test on AIX. | ||
// UNSUPPORTED: system-aix | ||
// | ||
// RUN: rm -rf %t && split-file %s %t && cd %t | ||
// | ||
// RUN: %clang -std=c++20 Hello.cppm -fmodule-output=Hello.pcm \ | ||
// RUN: -fexperimental-modules-reduced-bmi -c -o Hello.o -### 2>&1 | FileCheck Hello.cppm | ||
// | ||
// RUN: %clang -std=c++20 Hello.cppm \ | ||
// RUN: -fexperimental-modules-reduced-bmi -c -o Hello.o -### 2>&1 | \ | ||
// RUN: FileCheck Hello.cppm --check-prefix=CHECK-UNSPECIFIED | ||
// | ||
// RUN: %clang -std=c++20 Hello.cppm \ | ||
// RUN: -fexperimental-modules-reduced-bmi -c -### 2>&1 | \ | ||
// RUN: FileCheck Hello.cppm --check-prefix=CHECK-NO-O | ||
// | ||
// RUN: %clang -std=c++20 Hello.cppm \ | ||
// RUN: -fexperimental-modules-reduced-bmi -c -o AnotherName.o -### 2>&1 | \ | ||
// RUN: FileCheck Hello.cppm --check-prefix=CHECK-ANOTHER-NAME | ||
// | ||
// RUN: %clang -std=c++20 Hello.cppm --precompile -fexperimental-modules-reduced-bmi \ | ||
// RUN: -o Hello.full.pcm -### 2>&1 | FileCheck Hello.cppm \ | ||
// RUN: --check-prefix=CHECK-EMIT-MODULE-INTERFACE | ||
// | ||
// RUN: %clang -std=c++20 Hello.cc -fexperimental-modules-reduced-bmi -Wall -Werror \ | ||
// RUN: -c -o Hello.o -### 2>&1 | FileCheck Hello.cc | ||
|
||
//--- Hello.cppm | ||
export module Hello; | ||
|
||
// Test that we won't generate the emit-module-interface as 2 phase compilation model. | ||
// CHECK-NOT: -emit-module-interface | ||
// CHECK: "-fexperimental-modules-reduced-bmi" | ||
|
||
// CHECK-UNSPECIFIED: -fmodule-output=Hello.pcm | ||
|
||
// CHECK-NO-O: -fmodule-output=Hello.pcm | ||
// CHECK-ANOTHER-NAME: -fmodule-output=AnotherName.pcm | ||
|
||
// With `-emit-module-interface` specified, we should still see the `-emit-module-interface` | ||
// flag. | ||
// CHECK-EMIT-MODULE-INTERFACE: -emit-module-interface | ||
|
||
//--- Hello.cc | ||
|
||
// CHECK-NOT: "-fexperimental-modules-reduced-bmi" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
// RUN: rm -rf %t | ||
// RUN: mkdir -p %t | ||
// RUN: split-file %s %t | ||
// | ||
// RUN: %clang_cc1 -std=c++20 %t/a.cppm -emit-reduced-module-interface -o %t/a.reduced.pcm | ||
// RUN: %clang_cc1 -std=c++20 %t/a.cppm -fexperimental-modules-reduced-bmi -fmodule-output=%t/a.pcm \ | ||
// RUN: -S -emit-llvm -o %t/a.ll | ||
// | ||
// Test that the generated BMI from `-fexperimental-modules-reduced-bmi -fmodule-output=` is same with | ||
// `-emit-reduced-module-interface`. | ||
// RUN: diff %t/a.reduced.pcm %t/a.pcm | ||
// | ||
// Test that we can consume the produced BMI correctly. | ||
// RUN: %clang_cc1 -std=c++20 %t/b.cppm -fmodule-file=a=%t/a.pcm -fsyntax-only -verify | ||
// | ||
// RUN: rm -f %t/a.pcm | ||
// RUN: %clang_cc1 -std=c++20 %t/a.cppm -fexperimental-modules-reduced-bmi -fmodule-output=%t/a.pcm \ | ||
// RUN: -emit-module-interface -o %t/a.full.pcm | ||
// RUN: diff %t/a.reduced.pcm %t/a.pcm | ||
// RUN: not diff %t/a.pcm %t/a.full.pcm | ||
// | ||
// RUN: %clang_cc1 -std=c++20 %t/b.cppm -fmodule-file=a=%t/a.pcm -fsyntax-only -verify | ||
// RUN: %clang_cc1 -std=c++20 %t/b.cppm -fmodule-file=a=%t/a.full.pcm -fsyntax-only -verify | ||
|
||
//--- a.cppm | ||
export module a; | ||
export int a() { | ||
return 43; | ||
} | ||
|
||
//--- b.cppm | ||
// Test that we can consume the produced BMI correctly as a smocking test. | ||
// expected-no-diagnostics | ||
export module b; | ||
import a; | ||
export int b() { return a(); } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the motivation to include this change in this patch? Could it be separated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.