[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Iain Sandoe via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGafda39a566d9: re-land [C++20][Modules] Build module static 
initializers per P1874R1. (authored by iains).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/Module.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Parse/ParseAST.cpp
  clang/lib/Sema/SemaModule.cpp
  clang/test/CodeGen/module-intializer-pmf.cpp
  clang/test/CodeGen/module-intializer.cpp

Index: clang/test/CodeGen/module-intializer.cpp
===
--- /dev/null
+++ clang/test/CodeGen/module-intializer.cpp
@@ -0,0 +1,186 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 N.cpp \
+// RUN:-emit-module-interface -o N.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 N.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-N
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 O.cpp \
+// RUN:-emit-module-interface -o O.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 O.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-O
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M-part.cpp \
+// RUN:-emit-module-interface -o M-part.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M-part.pcm -S \
+// RUN: -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-P
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M.cpp \
+// RUN: -fmodule-file=N.pcm -fmodule-file=O.pcm -fmodule-file=M-part.pcm \
+// RUN:-emit-module-interface -o M.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-M
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 useM.cpp \
+// RUN: -fmodule-file=M.pcm -S -emit-llvm  -o - \
+// RUN: | FileCheck %s --check-prefix=CHECK-USE
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M-impl.cpp \
+// RUN: -fmodule-file=M.pcm -S -emit-llvm  -o - \
+// RUN: | FileCheck %s --check-prefix=CHECK-IMPL
+
+//--- N-h.h
+
+struct Oink {
+  Oink(){};
+};
+
+Oink Hog;
+
+//--- N.cpp
+
+module;
+#include "N-h.h"
+
+export module N;
+
+export struct Quack {
+  Quack(){};
+};
+
+export Quack Duck;
+
+// CHECK-N: define internal void @__cxx_global_var_init
+// CHECK-N: call {{.*}} @_ZN4OinkC1Ev
+// CHECK-N: define internal void @__cxx_global_var_init
+// CHECK-N: call {{.*}} @_ZNW1N5QuackC1Ev
+// CHECK-N: define void @_ZGIW1N
+// CHECK-N: store i8 1, ptr @_ZGIW1N__in_chrg
+// CHECK-N: call void @__cxx_global_var_init
+// CHECK-N: call void @__cxx_global_var_init
+
+//--- O-h.h
+
+struct Meow {
+  Meow(){};
+};
+
+Meow Cat;
+
+//--- O.cpp
+
+module;
+#include "O-h.h"
+
+export module O;
+
+export struct Bark {
+  Bark(){};
+};
+
+export Bark Dog;
+
+// CHECK-O: define internal void @__cxx_global_var_init
+// CHECK-O: call {{.*}} @_ZN4MeowC2Ev
+// CHECK-O: define internal void @__cxx_global_var_init
+// CHECK-O: call {{.*}} @_ZNW1O4BarkC1Ev
+// CHECK-O: define void @_ZGIW1O
+// CHECK-O: store i8 1, ptr @_ZGIW1O__in_chrg
+// CHECK-O: call void @__cxx_global_var_init
+// CHECK-O: call void @__cxx_global_var_init
+
+//--- P-h.h
+
+struct Croak {
+  Croak(){};
+};
+
+Croak Frog;
+
+//--- M-part.cpp
+
+module;
+#include "P-h.h"
+
+module M:Part;
+
+struct Squawk {
+  Squawk(){};
+};
+
+Squawk parrot;
+
+// CHECK-P: define internal void @__cxx_global_var_init
+// CHECK-P: call {{.*}} @_ZN5CroakC1Ev
+// CHECK-P: define internal void @__cxx_global_var_init
+// CHECK-P: call {{.*}} @_ZNW1M6SquawkC1Ev
+// CHECK-P: define void @_ZGIW1MWP4Part
+// CHECK-P: store i8 1, ptr @_ZGIW1MWP4Part__in_chrg
+// CHECK-P: call void @__cxx_global_var_init
+// CHECK-P: call void @__cxx_global_var_init
+
+//--- M-h.h
+
+struct Moo {
+  Moo(){};
+};
+
+Moo Cow;
+
+//--- M.cpp
+
+module;
+#include "M-h.h"
+
+export module M;
+import N;
+export import O;
+import :Part;
+
+export struct Baa {
+  int x;
+  Baa(){};
+  Baa(int x) : x(x) {}
+  int getX() { return x; }
+};
+
+export Baa Sheep(10);
+
+// CHECK-M: define internal void @__cxx_global_var_init
+// CHECK-M: call {{.*}} @_ZN3MooC1Ev
+// CHECK-M: define internal void @__cxx_global_var_init
+// CHECK-M: call {{.*}} @_ZNW1M3BaaC1Ei
+// CHECK-M: declare void @_ZGIW1O()
+// CHECK-M: declare void @_ZGIW1N()
+// CHECK-M: declare void @_ZGIW1MWP4Part()
+// CHECK-M: define void @_ZGIW1M
+// CHECK-M: store i8 1, ptr @_ZGIW1M__in_chrg
+// CHECK-M: call void @_ZGIW1O()
+// CHECK-M: call void @_ZGIW1N()
+// CHECK-M: call void @_ZGIW1MWP4Part()
+// CHECK-M: call void @__cxx_global_var_init
+// CHECK-M: call void @__cxx_global_var_init
+
+//--- useM.cpp

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

I feel like we could land this sooner to avoid any unimagined failures.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

In D126189#3662123 , @h-vetinari 
wrote:

> Is it realistic for this to land before LLVM 15 branches? Would be great!

that is the intention - I just got side-tracked with trying to replicate the 
test fails that got the commit reverted.  AFAICT it's OK now, and i plan to 
re-land it soon.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[H. Vetinari via Phabricator via cfe-commits]

h-vetinari added a comment.

Is it realistic for this to land before LLVM 15 branches? Would be great!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Iain Sandoe via Phabricator via cfe-commits]

iains updated this revision to Diff 444989.
iains added a comment.

rebased, fixed the interaction with clang module map modules.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/Module.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Parse/ParseAST.cpp
  clang/lib/Sema/SemaModule.cpp
  clang/test/CodeGen/module-intializer-pmf.cpp
  clang/test/CodeGen/module-intializer.cpp

Index: clang/test/CodeGen/module-intializer.cpp
===
--- /dev/null
+++ clang/test/CodeGen/module-intializer.cpp
@@ -0,0 +1,186 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 N.cpp \
+// RUN:-emit-module-interface -o N.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 N.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-N
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 O.cpp \
+// RUN:-emit-module-interface -o O.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 O.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-O
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M-part.cpp \
+// RUN:-emit-module-interface -o M-part.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M-part.pcm -S \
+// RUN: -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-P
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M.cpp \
+// RUN: -fmodule-file=N.pcm -fmodule-file=O.pcm -fmodule-file=M-part.pcm \
+// RUN:-emit-module-interface -o M.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-M
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 useM.cpp \
+// RUN: -fmodule-file=M.pcm -S -emit-llvm  -o - \
+// RUN: | FileCheck %s --check-prefix=CHECK-USE
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M-impl.cpp \
+// RUN: -fmodule-file=M.pcm -S -emit-llvm  -o - \
+// RUN: | FileCheck %s --check-prefix=CHECK-IMPL
+
+//--- N-h.h
+
+struct Oink {
+  Oink(){};
+};
+
+Oink Hog;
+
+//--- N.cpp
+
+module;
+#include "N-h.h"
+
+export module N;
+
+export struct Quack {
+  Quack(){};
+};
+
+export Quack Duck;
+
+// CHECK-N: define internal void @__cxx_global_var_init
+// CHECK-N: call {{.*}} @_ZN4OinkC1Ev
+// CHECK-N: define internal void @__cxx_global_var_init
+// CHECK-N: call {{.*}} @_ZNW1N5QuackC1Ev
+// CHECK-N: define void @_ZGIW1N
+// CHECK-N: store i8 1, ptr @_ZGIW1N__in_chrg
+// CHECK-N: call void @__cxx_global_var_init
+// CHECK-N: call void @__cxx_global_var_init
+
+//--- O-h.h
+
+struct Meow {
+  Meow(){};
+};
+
+Meow Cat;
+
+//--- O.cpp
+
+module;
+#include "O-h.h"
+
+export module O;
+
+export struct Bark {
+  Bark(){};
+};
+
+export Bark Dog;
+
+// CHECK-O: define internal void @__cxx_global_var_init
+// CHECK-O: call {{.*}} @_ZN4MeowC2Ev
+// CHECK-O: define internal void @__cxx_global_var_init
+// CHECK-O: call {{.*}} @_ZNW1O4BarkC1Ev
+// CHECK-O: define void @_ZGIW1O
+// CHECK-O: store i8 1, ptr @_ZGIW1O__in_chrg
+// CHECK-O: call void @__cxx_global_var_init
+// CHECK-O: call void @__cxx_global_var_init
+
+//--- P-h.h
+
+struct Croak {
+  Croak(){};
+};
+
+Croak Frog;
+
+//--- M-part.cpp
+
+module;
+#include "P-h.h"
+
+module M:Part;
+
+struct Squawk {
+  Squawk(){};
+};
+
+Squawk parrot;
+
+// CHECK-P: define internal void @__cxx_global_var_init
+// CHECK-P: call {{.*}} @_ZN5CroakC1Ev
+// CHECK-P: define internal void @__cxx_global_var_init
+// CHECK-P: call {{.*}} @_ZNW1M6SquawkC1Ev
+// CHECK-P: define void @_ZGIW1MWP4Part
+// CHECK-P: store i8 1, ptr @_ZGIW1MWP4Part__in_chrg
+// CHECK-P: call void @__cxx_global_var_init
+// CHECK-P: call void @__cxx_global_var_init
+
+//--- M-h.h
+
+struct Moo {
+  Moo(){};
+};
+
+Moo Cow;
+
+//--- M.cpp
+
+module;
+#include "M-h.h"
+
+export module M;
+import N;
+export import O;
+import :Part;
+
+export struct Baa {
+  int x;
+  Baa(){};
+  Baa(int x) : x(x) {}
+  int getX() { return x; }
+};
+
+export Baa Sheep(10);
+
+// CHECK-M: define internal void @__cxx_global_var_init
+// CHECK-M: call {{.*}} @_ZN3MooC1Ev
+// CHECK-M: define internal void @__cxx_global_var_init
+// CHECK-M: call {{.*}} @_ZNW1M3BaaC1Ei
+// CHECK-M: declare void @_ZGIW1O()
+// CHECK-M: declare void @_ZGIW1N()
+// CHECK-M: declare void @_ZGIW1MWP4Part()
+// CHECK-M: define void @_ZGIW1M
+// CHECK-M: store i8 1, ptr @_ZGIW1M__in_chrg
+// CHECK-M: call void @_ZGIW1O()
+// CHECK-M: call void @_ZGIW1N()
+// CHECK-M: call void @_ZGIW1MWP4Part()
+// CHECK-M: call void @__cxx_global_var_init
+// CHECK-M: call void @__cxx_global_var_init
+
+//--- useM.cpp
+
+import M;
+
+int main() {
+  return Sheep.getX();
+}
+
+// CHECK-U

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Iain Sandoe via Phabricator via cfe-commits]

iains reopened this revision.
iains added a comment.
This revision is now accepted and ready to land.

reopening to post the patch I intend to land.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

> hmm there seems to be a compiler error, which looks somewhat unrelated to the 
> active patch:
>
>   
> /Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/cross-project-tests/debuginfo-tests/clang_llvm_roundtrip/simplified_template_names.cpp:125:27:
>  error: no type named 'size_t' in namespace 'std'
> void* operator new(std::size_t, T) {
>~^
>   
> /Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/cross-project-tests/debuginfo-tests/clang_llvm_roundtrip/simplified_template_names.cpp:132:29:
>  error: no type named 'size_t' in namespace 'std'
> void* operator new[](std::size_t, T) {

Added the missing include here 4ca205855267191ccfd191539cf4b3ed792a4257 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

I need to do some more builds to be able to reproduce this - my guess (at 
present) is that this is a manifestation of '-fcxx-modules -std=c++20' being 
almost, but not exactly, the same as C++20 standardised modules.  It is 
possible that the -gmodules flag interacts with that (although one might have 
expected more than one fail if that were so).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere added a comment.

In D126189#3643045 , @iains wrote:

> In D126189#3643021 , @JDevlieghere 
> wrote:
>
>> In D126189#3643001 , @iains wrote:
>>
>>> In D126189#3642992 , 
>>> @JDevlieghere wrote:
>>>
 In D126189#3642820 , @iains 
 wrote:

> JFTR, I did not get any notification from green dragon (which is odd, 
> AFAIR it's sent email before) or I would have looked right away  - kicked 
> off a build will take a look as soon as that's done.

 Yes, the bot was already red because of a different issue.

 FWIW I tried reverting ac507102d258b6fc0cb57eb60c9dfabd57ff562f 
  and 
 4328b960176f4394416093e640ad4265bde65ad7 
  
 locally and I'm still getting a linker error about missing symbols:
>>>
>>> those refs come up as 'bad object' for me .. can you identify the upstream 
>>> changes?
>>
>> That's odd, both Github and Phab think those are the canonical hashes:
>>
>> https://github.com/llvm/llvm-project/commit/ac507102d258b6fc0cb57eb60c9dfabd57ff562f
>> https://github.com/llvm/llvm-project/commit/4328b960176f4394416093e640ad4265bde65ad7
>
> pilot error (pasto...)
>
   Undefined symbols for architecture arm64:
 "vtable for std::__1::format_error", referenced from:
 std::__1::format_error::format_error(char const*) in main.o
 std::__1::format_error::format_error(std::__1::basic_string>>> std::__1::char_traits, std::__1::allocator > const&) in main.o
>>>
>>> That seems also unrelated to the modules code, but I could always be 
>>> surprised :)
>
> OK - so if I cannot figure out what is happening in the next couple or hours, 
> I can revert those two commits (that's 9PM for me so I probably cannot do 
> much more today).

Thanks, if the bot reproduces the other issue I mentioned I'll bisect it down 
to see if there's another commit that's causing issues. I should've mentioned 
that that failures only happens when building with `-gmodules`, so it does 
sound somewhat related.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

In D126189#3643021 , @JDevlieghere 
wrote:

> In D126189#3643001 , @iains wrote:
>
>> In D126189#3642992 , @JDevlieghere 
>> wrote:
>>
>>> In D126189#3642820 , @iains wrote:
>>>
 JFTR, I did not get any notification from green dragon (which is odd, 
 AFAIR it's sent email before) or I would have looked right away  - kicked 
 off a build will take a look as soon as that's done.
>>>
>>> Yes, the bot was already red because of a different issue.
>>>
>>> FWIW I tried reverting ac507102d258b6fc0cb57eb60c9dfabd57ff562f 
>>>  and 
>>> 4328b960176f4394416093e640ad4265bde65ad7 
>>>  
>>> locally and I'm still getting a linker error about missing symbols:
>>
>> those refs come up as 'bad object' for me .. can you identify the upstream 
>> changes?
>
> That's odd, both Github and Phab think those are the canonical hashes:
>
> https://github.com/llvm/llvm-project/commit/ac507102d258b6fc0cb57eb60c9dfabd57ff562f
> https://github.com/llvm/llvm-project/commit/4328b960176f4394416093e640ad4265bde65ad7

pilot error (pasto...)

>>>   Undefined symbols for architecture arm64:
>>> "vtable for std::__1::format_error", referenced from:
>>> std::__1::format_error::format_error(char const*) in main.o
>>> std::__1::format_error::format_error(std::__1::basic_string>> std::__1::char_traits, std::__1::allocator > const&) in main.o
>>
>> That seems also unrelated to the modules code, but I could always be 
>> surprised :)

OK - so if I cannot figure out what is happening in the next couple or hours, I 
can revert those two commits (that's 9PM for me so I probably cannot do much 
more today).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere added a comment.

In D126189#3643001 , @iains wrote:

> In D126189#3642992 , @JDevlieghere 
> wrote:
>
>> In D126189#3642820 , @iains wrote:
>>
>>> JFTR, I did not get any notification from green dragon (which is odd, AFAIR 
>>> it's sent email before) or I would have looked right away  - kicked off a 
>>> build will take a look as soon as that's done.
>>
>> Yes, the bot was already red because of a different issue.
>>
>> FWIW I tried reverting ac507102d258b6fc0cb57eb60c9dfabd57ff562f 
>>  and 
>> 4328b960176f4394416093e640ad4265bde65ad7 
>>  
>> locally and I'm still getting a linker error about missing symbols:
>
> those refs come up as 'bad object' for me .. can you identify the upstream 
> changes?

That's odd, both Github and Phab think those are the canonical hashes:

https://github.com/llvm/llvm-project/commit/ac507102d258b6fc0cb57eb60c9dfabd57ff562f
https://github.com/llvm/llvm-project/commit/4328b960176f4394416093e640ad4265bde65ad7

>>   Undefined symbols for architecture arm64:
>> "vtable for std::__1::format_error", referenced from:
>> std::__1::format_error::format_error(char const*) in main.o
>> std::__1::format_error::format_error(std::__1::basic_string> std::__1::char_traits, std::__1::allocator > const&) in main.o
>
> That seems also unrelated to the modules code, but I could always be 
> surprised :)




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

In D126189#3642992 , @JDevlieghere 
wrote:

> In D126189#3642820 , @iains wrote:
>
>> JFTR, I did not get any notification from green dragon (which is odd, AFAIR 
>> it's sent email before) or I would have looked right away  - kicked off a 
>> build will take a look as soon as that's done.
>
> Yes, the bot was already red because of a different issue.
>
> FWIW I tried reverting ac507102d258b6fc0cb57eb60c9dfabd57ff562f 
>  and 
> 4328b960176f4394416093e640ad4265bde65ad7 
>  locally 
> and I'm still getting a linker error about missing symbols:

those refs come up as 'bad object' for me .. can you identify the upstream 
changes?

>   Undefined symbols for architecture arm64:
> "vtable for std::__1::format_error", referenced from:
> std::__1::format_error::format_error(char const*) in main.o
> std::__1::format_error::format_error(std::__1::basic_string std::__1::char_traits, std::__1::allocator > const&) in main.o

That seems also unrelated to the modules code, but I could always be surprised 
:)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere added a comment.

In D126189#3642820 , @iains wrote:

> JFTR, I did not get any notification from green dragon (which is odd, AFAIR 
> it's sent email before) or I would have looked right away  - kicked off a 
> build will take a look as soon as that's done.

Yes, the bot was already red because of a different issue.

FWIW I tried reverting ac507102d258b6fc0cb57eb60c9dfabd57ff562f 
 and 
4328b960176f4394416093e640ad4265bde65ad7 
 locally 
and I'm still getting a linker error about missing symbols:

  Undefined symbols for architecture arm64:
"vtable for std::__1::format_error", referenced from:
std::__1::format_error::format_error(char const*) in main.o
std::__1::format_error::format_error(std::__1::basic_string, std::__1::allocator > const&) in main.o


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

In D126189#3642779 , @JDevlieghere 
wrote:

> In D126189#3642777 , @iains wrote:
>
>> In D126189#3642762 , @JDevlieghere 
>> wrote:
>>
>>> This breaks TestDataFormatterLibcxxSpan.py on GreenDragon:



>>> https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/45207/
>>
>> ack, (I develop on macOS so usually catch these things)...
>> do you need me to revert the commit - or can we try to find out what's 
>> broken and fix it?
>
> If you think you'll be able to get to the bottom of this quickly then I don't 
> think we need to revert, but the bot has been red for a while (which means 
> that other issues can get buried) so I'd like to get it green ASAP.

JFTR, I did not get any notification from green dragon (which is odd, AFAIR 
it's sent email before) or I would have looked right away  - kicked off a build 
will take a look as soon as that's done.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

In D126189#3642777 , @iains wrote:

> In D126189#3642762 , @JDevlieghere 
> wrote:
>
>> This breaks TestDataFormatterLibcxxSpan.py on GreenDragon:
>>
>>   Undefined symbols for architecture x86_64:

..

>>   "__ZGIW6vector", referenced from:
>>   __GLOBAL__sub_I_main.cpp in main.o
>>
>>   https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/45207/
>
> ack, (I develop on macOS so usually catch these things)...
> do you need me to revert the commit - or can we try to find out what's broken 
> and fix it?

hmm there seems to be a compiler error, which looks somewhat unrelated to the 
active patch:

  
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/cross-project-tests/debuginfo-tests/clang_llvm_roundtrip/simplified_template_names.cpp:125:27:
 error: no type named 'size_t' in namespace 'std'
void* operator new(std::size_t, T) {
   ~^
  
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/cross-project-tests/debuginfo-tests/clang_llvm_roundtrip/simplified_template_names.cpp:132:29:
 error: no type named 'size_t' in namespace 'std'
void* operator new[](std::size_t, T) {


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere added a comment.

In D126189#3642777 , @iains wrote:

> In D126189#3642762 , @JDevlieghere 
> wrote:
>
>> This breaks TestDataFormatterLibcxxSpan.py on GreenDragon:
>>
>>   Undefined symbols for architecture x86_64:
>> "__ZGIW10std_config", referenced from:
>> __GLOBAL__sub_I_main.cpp in main.o
>> "__ZGIW4span", referenced from:
>> __GLOBAL__sub_I_main.cpp in main.o
>> "__ZGIW5array", referenced from:
>> __GLOBAL__sub_I_main.cpp in main.o
>> "__ZGIW5stdio", referenced from:
>> __GLOBAL__sub_I_main.cpp in main.o
>> "__ZGIW6string", referenced from:
>> __GLOBAL__sub_I_main.cpp in main.o
>> "__ZGIW6vector", referenced from:
>> __GLOBAL__sub_I_main.cpp in main.o
>>
>> https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/45207/
>
> ack, (I develop on macOS so usually catch these things)...
> do you need me to revert the commit - or can we try to find out what's broken 
> and fix it?

If you think you'll be able to get to the bottom of this quickly then I don't 
think we need to revert, but the bot has been red for a while (which means that 
other issues can get buried) so I'd like to get it green ASAP.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

In D126189#3642762 , @JDevlieghere 
wrote:

> This breaks TestDataFormatterLibcxxSpan.py on GreenDragon:
>
>   Undefined symbols for architecture x86_64:
> "__ZGIW10std_config", referenced from:
> __GLOBAL__sub_I_main.cpp in main.o
> "__ZGIW4span", referenced from:
> __GLOBAL__sub_I_main.cpp in main.o
> "__ZGIW5array", referenced from:
> __GLOBAL__sub_I_main.cpp in main.o
> "__ZGIW5stdio", referenced from:
> __GLOBAL__sub_I_main.cpp in main.o
> "__ZGIW6string", referenced from:
> __GLOBAL__sub_I_main.cpp in main.o
> "__ZGIW6vector", referenced from:
> __GLOBAL__sub_I_main.cpp in main.o
>
> https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/45207/

ack, (I develop on macOS so usually catch these things)...
do you need me to revert the commit - or can we try to find out what's broken 
and fix it?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere added a comment.

This breaks TestDataFormatterLibcxxSpan.py on GreenDragon:

  Undefined symbols for architecture x86_64:
"__ZGIW10std_config", referenced from:
__GLOBAL__sub_I_main.cpp in main.o
"__ZGIW4span", referenced from:
__GLOBAL__sub_I_main.cpp in main.o
"__ZGIW5array", referenced from:
__GLOBAL__sub_I_main.cpp in main.o
"__ZGIW5stdio", referenced from:
__GLOBAL__sub_I_main.cpp in main.o
"__ZGIW6string", referenced from:
__GLOBAL__sub_I_main.cpp in main.o
"__ZGIW6vector", referenced from:
__GLOBAL__sub_I_main.cpp in main.o

https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/45207/


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Iain Sandoe via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGac507102d258: [C++20][Modules] Build module static 
initializers per P1874R1. (authored by iains).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/Module.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Parse/ParseAST.cpp
  clang/lib/Sema/SemaModule.cpp
  clang/test/CodeGen/module-intializer-pmf.cpp
  clang/test/CodeGen/module-intializer.cpp

Index: clang/test/CodeGen/module-intializer.cpp
===
--- /dev/null
+++ clang/test/CodeGen/module-intializer.cpp
@@ -0,0 +1,186 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 N.cpp \
+// RUN:-emit-module-interface -o N.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 N.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-N
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 O.cpp \
+// RUN:-emit-module-interface -o O.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 O.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-O
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M-part.cpp \
+// RUN:-emit-module-interface -o M-part.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M-part.pcm -S \
+// RUN: -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-P
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M.cpp \
+// RUN: -fmodule-file=N.pcm -fmodule-file=O.pcm -fmodule-file=M-part.pcm \
+// RUN:-emit-module-interface -o M.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-M
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 useM.cpp \
+// RUN: -fmodule-file=M.pcm -S -emit-llvm  -o - \
+// RUN: | FileCheck %s --check-prefix=CHECK-USE
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M-impl.cpp \
+// RUN: -fmodule-file=M.pcm -S -emit-llvm  -o - \
+// RUN: | FileCheck %s --check-prefix=CHECK-IMPL
+
+//--- N-h.h
+
+struct Oink {
+  Oink(){};
+};
+
+Oink Hog;
+
+//--- N.cpp
+
+module;
+#include "N-h.h"
+
+export module N;
+
+export struct Quack {
+  Quack(){};
+};
+
+export Quack Duck;
+
+// CHECK-N: define internal void @__cxx_global_var_init
+// CHECK-N: call void @_ZN4OinkC1Ev
+// CHECK-N: define internal void @__cxx_global_var_init
+// CHECK-N: call void @_ZNW1N5QuackC1Ev
+// CHECK-N: define void @_ZGIW1N
+// CHECK-N: store i8 1, ptr @_ZGIW1N__in_chrg
+// CHECK-N: call void @__cxx_global_var_init
+// CHECK-N: call void @__cxx_global_var_init
+
+//--- O-h.h
+
+struct Meow {
+  Meow(){};
+};
+
+Meow Cat;
+
+//--- O.cpp
+
+module;
+#include "O-h.h"
+
+export module O;
+
+export struct Bark {
+  Bark(){};
+};
+
+export Bark Dog;
+
+// CHECK-O: define internal void @__cxx_global_var_init
+// CHECK-O: call void @_ZN4MeowC2Ev
+// CHECK-O: define internal void @__cxx_global_var_init
+// CHECK-O: call void @_ZNW1O4BarkC1Ev
+// CHECK-O: define void @_ZGIW1O
+// CHECK-O: store i8 1, ptr @_ZGIW1O__in_chrg
+// CHECK-O: call void @__cxx_global_var_init
+// CHECK-O: call void @__cxx_global_var_init
+
+//--- P-h.h
+
+struct Croak {
+  Croak(){};
+};
+
+Croak Frog;
+
+//--- M-part.cpp
+
+module;
+#include "P-h.h"
+
+module M:Part;
+
+struct Squawk {
+  Squawk(){};
+};
+
+Squawk parrot;
+
+// CHECK-P: define internal void @__cxx_global_var_init
+// CHECK-P: call void @_ZN5CroakC1Ev
+// CHECK-P: define internal void @__cxx_global_var_init
+// CHECK-P: call void @_ZNW1M6SquawkC1Ev
+// CHECK-P: define void @_ZGIW1MWP4Part
+// CHECK-P: store i8 1, ptr @_ZGIW1MWP4Part__in_chrg
+// CHECK-P: call void @__cxx_global_var_init
+// CHECK-P: call void @__cxx_global_var_init
+
+//--- M-h.h
+
+struct Moo {
+  Moo(){};
+};
+
+Moo Cow;
+
+//--- M.cpp
+
+module;
+#include "M-h.h"
+
+export module M;
+import N;
+export import O;
+import :Part;
+
+export struct Baa {
+  int x;
+  Baa(){};
+  Baa(int x) : x(x) {}
+  int getX() { return x; }
+};
+
+export Baa Sheep(10);
+
+// CHECK-M: define internal void @__cxx_global_var_init
+// CHECK-M: call void @_ZN3MooC1Ev
+// CHECK-M: define internal void @__cxx_global_var_init
+// CHECK-M: call void @_ZNW1M3BaaC1Ei
+// CHECK-M: declare void @_ZGIW1O()
+// CHECK-M: declare void @_ZGIW1N()
+// CHECK-M: declare void @_ZGIW1MWP4Part()
+// CHECK-M: define void @_ZGIW1M
+// CHECK-M: store i8 1, ptr @_ZGIW1M__in_chrg
+// CHECK-M: call void @_ZGIW1O()
+// CHECK-M: call void @_ZGIW1N()
+// CHECK-M: call void @_ZGIW1MWP4Part()
+// CHECK-M: call void @__cxx_global_var_init
+// CHECK-M: call void @__cxx_gl

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D126189#3617896 , @iains wrote:

> In D126189#3617850 , @ChuanqiXu 
> wrote:
>
>> @iains may I ask what's the issue to not land this? It looks like you're 
>> waiting for the behavior to be consistency with GCC?
>>
>> Since this patch could fix 
>> https://github.com/llvm/llvm-project/issues/51873, which breaks the users to 
>> compile a hello world example.
>
> I need to make some typo corrections; there is no issue (I'm not waiting for 
> anything) just prioritising posting patches to complete C++20 support .. will 
> land this soon.

Thanks! The reason why I am a little bit hurry is that I'm trying to implement 
std modules: https://github.com/ChuanqiXu9/stdmodules

Now it runs good for libcxx. But it would meet segfault in 
https://github.com/ChuanqiXu9/stdmodules/blob/master/test/HelloWorld/HelloWorld.cpp
 if we don't land this revision.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

In D126189#3598568 , @urnathan wrote:

> please sed /initialiser/initializer/, I noticed a few had crept in.

should be done now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Iain Sandoe via Phabricator via cfe-commits]

iains updated this revision to Diff 440924.
iains added a comment.

rebased, corrected some spellings.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/Module.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Parse/ParseAST.cpp
  clang/lib/Sema/SemaModule.cpp
  clang/test/CodeGen/module-intializer-pmf.cpp
  clang/test/CodeGen/module-intializer.cpp

Index: clang/test/CodeGen/module-intializer.cpp
===
--- /dev/null
+++ clang/test/CodeGen/module-intializer.cpp
@@ -0,0 +1,186 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 N.cpp \
+// RUN:-emit-module-interface -o N.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 N.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-N
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 O.cpp \
+// RUN:-emit-module-interface -o O.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 O.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-O
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M-part.cpp \
+// RUN:-emit-module-interface -o M-part.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M-part.pcm -S \
+// RUN: -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-P
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M.cpp \
+// RUN: -fmodule-file=N.pcm -fmodule-file=O.pcm -fmodule-file=M-part.pcm \
+// RUN:-emit-module-interface -o M.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-M
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 useM.cpp \
+// RUN: -fmodule-file=M.pcm -S -emit-llvm  -o - \
+// RUN: | FileCheck %s --check-prefix=CHECK-USE
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M-impl.cpp \
+// RUN: -fmodule-file=M.pcm -S -emit-llvm  -o - \
+// RUN: | FileCheck %s --check-prefix=CHECK-IMPL
+
+//--- N-h.h
+
+struct Oink {
+  Oink(){};
+};
+
+Oink Hog;
+
+//--- N.cpp
+
+module;
+#include "N-h.h"
+
+export module N;
+
+export struct Quack {
+  Quack(){};
+};
+
+export Quack Duck;
+
+// CHECK-N: define internal void @__cxx_global_var_init
+// CHECK-N: call void @_ZN4OinkC1Ev
+// CHECK-N: define internal void @__cxx_global_var_init
+// CHECK-N: call void @_ZNW1N5QuackC1Ev
+// CHECK-N: define void @_ZGIW1N
+// CHECK-N: store i8 1, ptr @_ZGIW1N__in_chrg
+// CHECK-N: call void @__cxx_global_var_init
+// CHECK-N: call void @__cxx_global_var_init
+
+//--- O-h.h
+
+struct Meow {
+  Meow(){};
+};
+
+Meow Cat;
+
+//--- O.cpp
+
+module;
+#include "O-h.h"
+
+export module O;
+
+export struct Bark {
+  Bark(){};
+};
+
+export Bark Dog;
+
+// CHECK-O: define internal void @__cxx_global_var_init
+// CHECK-O: call void @_ZN4MeowC2Ev
+// CHECK-O: define internal void @__cxx_global_var_init
+// CHECK-O: call void @_ZNW1O4BarkC1Ev
+// CHECK-O: define void @_ZGIW1O
+// CHECK-O: store i8 1, ptr @_ZGIW1O__in_chrg
+// CHECK-O: call void @__cxx_global_var_init
+// CHECK-O: call void @__cxx_global_var_init
+
+//--- P-h.h
+
+struct Croak {
+  Croak(){};
+};
+
+Croak Frog;
+
+//--- M-part.cpp
+
+module;
+#include "P-h.h"
+
+module M:Part;
+
+struct Squawk {
+  Squawk(){};
+};
+
+Squawk parrot;
+
+// CHECK-P: define internal void @__cxx_global_var_init
+// CHECK-P: call void @_ZN5CroakC1Ev
+// CHECK-P: define internal void @__cxx_global_var_init
+// CHECK-P: call void @_ZNW1M6SquawkC1Ev
+// CHECK-P: define void @_ZGIW1MWP4Part
+// CHECK-P: store i8 1, ptr @_ZGIW1MWP4Part__in_chrg
+// CHECK-P: call void @__cxx_global_var_init
+// CHECK-P: call void @__cxx_global_var_init
+
+//--- M-h.h
+
+struct Moo {
+  Moo(){};
+};
+
+Moo Cow;
+
+//--- M.cpp
+
+module;
+#include "M-h.h"
+
+export module M;
+import N;
+export import O;
+import :Part;
+
+export struct Baa {
+  int x;
+  Baa(){};
+  Baa(int x) : x(x) {}
+  int getX() { return x; }
+};
+
+export Baa Sheep(10);
+
+// CHECK-M: define internal void @__cxx_global_var_init
+// CHECK-M: call void @_ZN3MooC1Ev
+// CHECK-M: define internal void @__cxx_global_var_init
+// CHECK-M: call void @_ZNW1M3BaaC1Ei
+// CHECK-M: declare void @_ZGIW1O()
+// CHECK-M: declare void @_ZGIW1N()
+// CHECK-M: declare void @_ZGIW1MWP4Part()
+// CHECK-M: define void @_ZGIW1M
+// CHECK-M: store i8 1, ptr @_ZGIW1M__in_chrg
+// CHECK-M: call void @_ZGIW1O()
+// CHECK-M: call void @_ZGIW1N()
+// CHECK-M: call void @_ZGIW1MWP4Part()
+// CHECK-M: call void @__cxx_global_var_init
+// CHECK-M: call void @__cxx_global_var_init
+
+//--- useM.cpp
+
+import M;
+
+int main() {
+  return Sheep.getX();
+}
+
+// CHECK-USE: declare void @_ZGIW1M
+// CHECK-USE: de

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

In D126189#3617850 , @ChuanqiXu wrote:

> @iains may I ask what's the issue to not land this? It looks like you're 
> waiting for the behavior to be consistency with GCC?
>
> Since this patch could fix https://github.com/llvm/llvm-project/issues/51873, 
> which breaks the users to compile a hello world example.

I need to make some typo corrections; there is no issue (I'm not waiting for 
anything) just prioritising posting patches to complete C++20 support .. will 
land this soon.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

@iains may I ask what's the issue to not land this? It looks like you're 
waiting for the behavior to be consistency with GCC?

Since this patch could fix https://github.com/llvm/llvm-project/issues/51873, 
which breaks the users to compile a hello world example.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Nathan Sidwell via Phabricator via cfe-commits]

urnathan added a comment.

please sed /initialiser/initializer/, I noticed a few had crept in.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

@urnathan - I believe that this now implements the same scheme as you have done 
for GCC (less any of the optimisations).
In particular, we now emit global CTOR entries for module initializers, even 
though these should really be called explicitly, but as was discussed on the 
core/sg15 reflectors it is possible (at least for unix-like systems) to 
construct cases where module objects are included in a final binary without 
their initializers being called.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Iain Sandoe via Phabricator via cfe-commits]

iains updated this revision to Diff 436487.
iains added a comment.

rebased. Amended to include a global CTOR entry for each module kind.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/Module.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Parse/ParseAST.cpp
  clang/lib/Sema/SemaModule.cpp
  clang/test/CodeGen/module-intializer-pmf.cpp
  clang/test/CodeGen/module-intializer.cpp

Index: clang/test/CodeGen/module-intializer.cpp
===
--- /dev/null
+++ clang/test/CodeGen/module-intializer.cpp
@@ -0,0 +1,186 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 N.cpp \
+// RUN:-emit-module-interface -o N.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 N.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-N
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 O.cpp \
+// RUN:-emit-module-interface -o O.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 O.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-O
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M-part.cpp \
+// RUN:-emit-module-interface -o M-part.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M-part.pcm -S \
+// RUN: -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-P
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M.cpp \
+// RUN: -fmodule-file=N.pcm -fmodule-file=O.pcm -fmodule-file=M-part.pcm \
+// RUN:-emit-module-interface -o M.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-M
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 useM.cpp \
+// RUN: -fmodule-file=M.pcm -S -emit-llvm  -o - \
+// RUN: | FileCheck %s --check-prefix=CHECK-USE
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M-impl.cpp \
+// RUN: -fmodule-file=M.pcm -S -emit-llvm  -o - \
+// RUN: | FileCheck %s --check-prefix=CHECK-IMPL
+
+//--- N-h.h
+
+struct Oink {
+  Oink(){};
+};
+
+Oink Hog;
+
+//--- N.cpp
+
+module;
+#include "N-h.h"
+
+export module N;
+
+export struct Quack {
+  Quack(){};
+};
+
+export Quack Duck;
+
+// CHECK-N: define internal void @__cxx_global_var_init
+// CHECK-N: call void @_ZN4OinkC1Ev
+// CHECK-N: define internal void @__cxx_global_var_init
+// CHECK-N: call void @_ZNW1N5QuackC1Ev
+// CHECK-N: define void @_ZGIW1N
+// CHECK-N: store i8 1, ptr @_ZGIW1N__in_chrg
+// CHECK-N: call void @__cxx_global_var_init
+// CHECK-N: call void @__cxx_global_var_init
+
+//--- O-h.h
+
+struct Meow {
+  Meow(){};
+};
+
+Meow Cat;
+
+//--- O.cpp
+
+module;
+#include "O-h.h"
+
+export module O;
+
+export struct Bark {
+  Bark(){};
+};
+
+export Bark Dog;
+
+// CHECK-O: define internal void @__cxx_global_var_init
+// CHECK-O: call void @_ZN4MeowC2Ev
+// CHECK-O: define internal void @__cxx_global_var_init
+// CHECK-O: call void @_ZNW1O4BarkC1Ev
+// CHECK-O: define void @_ZGIW1O
+// CHECK-O: store i8 1, ptr @_ZGIW1O__in_chrg
+// CHECK-O: call void @__cxx_global_var_init
+// CHECK-O: call void @__cxx_global_var_init
+
+//--- P-h.h
+
+struct Croak {
+  Croak(){};
+};
+
+Croak Frog;
+
+//--- M-part.cpp
+
+module;
+#include "P-h.h"
+
+module M:Part;
+
+struct Squawk {
+  Squawk(){};
+};
+
+Squawk parrot;
+
+// CHECK-P: define internal void @__cxx_global_var_init
+// CHECK-P: call void @_ZN5CroakC1Ev
+// CHECK-P: define internal void @__cxx_global_var_init
+// CHECK-P: call void @_ZNW1M6SquawkC1Ev
+// CHECK-P: define void @_ZGIW1MWP4Part
+// CHECK-P: store i8 1, ptr @_ZGIW1MWP4Part__in_chrg
+// CHECK-P: call void @__cxx_global_var_init
+// CHECK-P: call void @__cxx_global_var_init
+
+//--- M-h.h
+
+struct Moo {
+  Moo(){};
+};
+
+Moo Cow;
+
+//--- M.cpp
+
+module;
+#include "M-h.h"
+
+export module M;
+import N;
+export import O;
+import :Part;
+
+export struct Baa {
+  int x;
+  Baa(){};
+  Baa(int x) : x(x) {}
+  int getX() { return x; }
+};
+
+export Baa Sheep(10);
+
+// CHECK-M: define internal void @__cxx_global_var_init
+// CHECK-M: call void @_ZN3MooC1Ev
+// CHECK-M: define internal void @__cxx_global_var_init
+// CHECK-M: call void @_ZNW1M3BaaC1Ei
+// CHECK-M: declare void @_ZGIW1O()
+// CHECK-M: declare void @_ZGIW1N()
+// CHECK-M: declare void @_ZGIW1MWP4Part()
+// CHECK-M: define void @_ZGIW1M
+// CHECK-M: store i8 1, ptr @_ZGIW1M__in_chrg
+// CHECK-M: call void @_ZGIW1O()
+// CHECK-M: call void @_ZGIW1N()
+// CHECK-M: call void @_ZGIW1MWP4Part()
+// CHECK-M: call void @__cxx_global_var_init
+// CHECK-M: call void @__cxx_global_var_init
+
+//--- useM.cpp
+
+import M;
+
+int main() {
+  return Sheep.getX();
+}
+
+// CHECK-USE: decl

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Iain Sandoe via Phabricator via cfe-commits]

iains updated this revision to Diff 434781.
iains added a comment.

rebased


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/Module.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Parse/ParseAST.cpp
  clang/test/CodeGen/module-intializer-pmf.cpp
  clang/test/CodeGen/module-intializer.cpp

Index: clang/test/CodeGen/module-intializer.cpp
===
--- /dev/null
+++ clang/test/CodeGen/module-intializer.cpp
@@ -0,0 +1,186 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 N.cpp \
+// RUN:-emit-module-interface -o N.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 N.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-N
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 O.cpp \
+// RUN:-emit-module-interface -o O.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 O.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-O
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M-part.cpp \
+// RUN:-emit-module-interface -o M-part.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M-part.pcm -S \
+// RUN: -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-P
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M.cpp \
+// RUN: -fmodule-file=N.pcm -fmodule-file=O.pcm -fmodule-file=M-part.pcm \
+// RUN:-emit-module-interface -o M.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-M
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 useM.cpp \
+// RUN: -fmodule-file=M.pcm -S -emit-llvm  -o - \
+// RUN: | FileCheck %s --check-prefix=CHECK-USE
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M-impl.cpp \
+// RUN: -fmodule-file=M.pcm -S -emit-llvm  -o - \
+// RUN: | FileCheck %s --check-prefix=CHECK-IMPL
+
+//--- N-h.h
+
+struct Oink {
+  Oink(){};
+};
+
+Oink Hog;
+
+//--- N.cpp
+
+module;
+#include "N-h.h"
+
+export module N;
+
+export struct Quack {
+  Quack(){};
+};
+
+export Quack Duck;
+
+// CHECK-N: define internal void @__cxx_global_var_init
+// CHECK-N: call void @_ZN4OinkC1Ev
+// CHECK-N: define internal void @__cxx_global_var_init
+// CHECK-N: call void @_ZNW1N5QuackC1Ev
+// CHECK-N: define void @_ZGIW1N
+// CHECK-N: store i8 1, ptr @_ZGIW1N__in_chrg
+// CHECK-N: call void @__cxx_global_var_init
+// CHECK-N: call void @__cxx_global_var_init
+
+//--- O-h.h
+
+struct Meow {
+  Meow(){};
+};
+
+Meow Cat;
+
+//--- O.cpp
+
+module;
+#include "O-h.h"
+
+export module O;
+
+export struct Bark {
+  Bark(){};
+};
+
+export Bark Dog;
+
+// CHECK-O: define internal void @__cxx_global_var_init
+// CHECK-O: call void @_ZN4MeowC2Ev
+// CHECK-O: define internal void @__cxx_global_var_init
+// CHECK-O: call void @_ZNW1O4BarkC1Ev
+// CHECK-O: define void @_ZGIW1O
+// CHECK-O: store i8 1, ptr @_ZGIW1O__in_chrg
+// CHECK-O: call void @__cxx_global_var_init
+// CHECK-O: call void @__cxx_global_var_init
+
+//--- P-h.h
+
+struct Croak {
+  Croak(){};
+};
+
+Croak Frog;
+
+//--- M-part.cpp
+
+module;
+#include "P-h.h"
+
+module M:Part;
+
+struct Squawk {
+  Squawk(){};
+};
+
+Squawk parrot;
+
+// CHECK-P: define internal void @__cxx_global_var_init
+// CHECK-P: call void @_ZN5CroakC1Ev
+// CHECK-P: define internal void @__cxx_global_var_init
+// CHECK-P: call void @_ZNW1M6SquawkC1Ev
+// CHECK-P: define void @_ZGIW1MWP4Part
+// CHECK-P: store i8 1, ptr @_ZGIW1MWP4Part__in_chrg
+// CHECK-P: call void @__cxx_global_var_init
+// CHECK-P: call void @__cxx_global_var_init
+
+//--- M-h.h
+
+struct Moo {
+  Moo(){};
+};
+
+Moo Cow;
+
+//--- M.cpp
+
+module;
+#include "M-h.h"
+
+export module M;
+import N;
+export import O;
+import :Part;
+
+export struct Baa {
+  int x;
+  Baa(){};
+  Baa(int x) : x(x) {}
+  int getX() { return x; }
+};
+
+export Baa Sheep(10);
+
+// CHECK-M: define internal void @__cxx_global_var_init
+// CHECK-M: call void @_ZN3MooC1Ev
+// CHECK-M: define internal void @__cxx_global_var_init
+// CHECK-M: call void @_ZNW1M3BaaC1Ei
+// CHECK-M: declare void @_ZGIW1O()
+// CHECK-M: declare void @_ZGIW1N()
+// CHECK-M: declare void @_ZGIW1MWP4Part()
+// CHECK-M: define void @_ZGIW1M
+// CHECK-M: store i8 1, ptr @_ZGIW1M__in_chrg
+// CHECK-M: call void @_ZGIW1O()
+// CHECK-M: call void @_ZGIW1N()
+// CHECK-M: call void @_ZGIW1MWP4Part()
+// CHECK-M: call void @__cxx_global_var_init
+// CHECK-M: call void @__cxx_global_var_init
+
+//--- useM.cpp
+
+import M;
+
+int main() {
+  return Sheep.getX();
+}
+
+// CHECK-USE: declare void @_ZGIW1M
+// CHECK-USE: define internal void @_GLOBAL__sub_I_useM.cpp
+// CHECK-USE: 

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Iain Sandoe via Phabricator via cfe-commits]

iains updated this revision to Diff 434003.
iains edited the summary of this revision.
iains added a comment.

rebased and updated

previously, this was not doing the right thing for module implementation units
which were being processed as if they were interfaces, so we've introduced a 
module
implemetation type, and that is now checked for in the emitting of the inits.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/Module.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Parse/ParseAST.cpp
  clang/test/CodeGen/module-intializer-pmf.cpp
  clang/test/CodeGen/module-intializer.cpp

Index: clang/test/CodeGen/module-intializer.cpp
===
--- /dev/null
+++ clang/test/CodeGen/module-intializer.cpp
@@ -0,0 +1,186 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 N.cpp \
+// RUN:-emit-module-interface -o N.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 N.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-N
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 O.cpp \
+// RUN:-emit-module-interface -o O.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 O.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-O
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M-part.cpp \
+// RUN:-emit-module-interface -o M-part.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M-part.pcm -S \
+// RUN: -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-P
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M.cpp \
+// RUN: -fmodule-file=N.pcm -fmodule-file=O.pcm -fmodule-file=M-part.pcm \
+// RUN:-emit-module-interface -o M.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-M
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 useM.cpp \
+// RUN: -fmodule-file=M.pcm -S -emit-llvm  -o - \
+// RUN: | FileCheck %s --check-prefix=CHECK-USE
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M-impl.cpp \
+// RUN: -fmodule-file=M.pcm -S -emit-llvm  -o - \
+// RUN: | FileCheck %s --check-prefix=CHECK-IMPL
+
+//--- N-h.h
+
+struct Oink {
+  Oink(){};
+};
+
+Oink Hog;
+
+//--- N.cpp
+
+module;
+#include "N-h.h"
+
+export module N;
+
+export struct Quack {
+  Quack(){};
+};
+
+export Quack Duck;
+
+// CHECK-N: define internal void @__cxx_global_var_init
+// CHECK-N: call void @_ZN4OinkC1Ev
+// CHECK-N: define internal void @__cxx_global_var_init
+// CHECK-N: call void @_ZNW1N5QuackC1Ev
+// CHECK-N: define void @_ZGIW1N
+// CHECK-N: store i8 1, ptr @_ZGIW1N__in_chrg
+// CHECK-N: call void @__cxx_global_var_init
+// CHECK-N: call void @__cxx_global_var_init
+
+//--- O-h.h
+
+struct Meow {
+  Meow(){};
+};
+
+Meow Cat;
+
+//--- O.cpp
+
+module;
+#include "O-h.h"
+
+export module O;
+
+export struct Bark {
+  Bark(){};
+};
+
+export Bark Dog;
+
+// CHECK-O: define internal void @__cxx_global_var_init
+// CHECK-O: call void @_ZN4MeowC2Ev
+// CHECK-O: define internal void @__cxx_global_var_init
+// CHECK-O: call void @_ZNW1O4BarkC1Ev
+// CHECK-O: define void @_ZGIW1O
+// CHECK-O: store i8 1, ptr @_ZGIW1O__in_chrg
+// CHECK-O: call void @__cxx_global_var_init
+// CHECK-O: call void @__cxx_global_var_init
+
+//--- P-h.h
+
+struct Croak {
+  Croak(){};
+};
+
+Croak Frog;
+
+//--- M-part.cpp
+
+module;
+#include "P-h.h"
+
+module M:Part;
+
+struct Squawk {
+  Squawk(){};
+};
+
+Squawk parrot;
+
+// CHECK-P: define internal void @__cxx_global_var_init
+// CHECK-P: call void @_ZN5CroakC1Ev
+// CHECK-P: define internal void @__cxx_global_var_init
+// CHECK-P: call void @_ZNW1M6SquawkC1Ev
+// CHECK-P: define void @_ZGIW1MWP4Part
+// CHECK-P: store i8 1, ptr @_ZGIW1MWP4Part__in_chrg
+// CHECK-P: call void @__cxx_global_var_init
+// CHECK-P: call void @__cxx_global_var_init
+
+//--- M-h.h
+
+struct Moo {
+  Moo(){};
+};
+
+Moo Cow;
+
+//--- M.cpp
+
+module;
+#include "M-h.h"
+
+export module M;
+import N;
+export import O;
+import :Part;
+
+export struct Baa {
+  int x;
+  Baa(){};
+  Baa(int x) : x(x) {}
+  int getX() { return x; }
+};
+
+export Baa Sheep(10);
+
+// CHECK-M: define internal void @__cxx_global_var_init
+// CHECK-M: call void @_ZN3MooC1Ev
+// CHECK-M: define internal void @__cxx_global_var_init
+// CHECK-M: call void @_ZNW1M3BaaC1Ei
+// CHECK-M: declare void @_ZGIW1O()
+// CHECK-M: declare void @_ZGIW1N()
+// CHECK-M: declare void @_ZGIW1MWP4Part()
+// CHECK-M: define void @_ZGIW1M
+// CHECK-M: store i8 1, ptr @_ZGIW1M__in_chrg
+// CHECK-M: call void @_ZGIW1O()
+// CHECK-M: call void @_ZGIW1N()
+// CHECK-M: call void @

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu accepted this revision.
ChuanqiXu added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Iain Sandoe via Phabricator via cfe-commits]

iains added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:645-646
+  llvm::raw_svector_ostream Out(FnName);
+  cast(getCXXABI().getMangleContext())
+  .mangleModuleInitializer(M, Out);
+}

ChuanqiXu wrote:
> iains wrote:
> > ChuanqiXu wrote:
> > > mangleModuleInitializer is virtual function and we don't need to cast it.
> > Actually, 'mangleModuleInitializer' is not a virtual function of the 
> > 'MangleContext' class, only of the 'ItaniumMangleContext'.
> > (see D122741).
> > 
> > This because we do not know the mangling for MS and, in principle, there 
> > might never be such a mangling (e.g. that implementation might deal with 
> > P1874 in some different way).
> > 
> > I did have a version of this where I amended the mangling to make the 
> > function virtual in the 'MangleContext' class, but then that means 
> > generating a dummy MS version that, as noted above, might never exist in 
> > reality.
> > 
> I was just afraid of people might blame us to bring ABI specific things to 
> CodeGen* things. I found there similar uses in CGVTables.cpp and CGVTT.cpp. 
> So this might be acceptable too.
yes, let us see - if there are any other opinions - personally, I prefer 
**not** to generate a dummy function in the MS code.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:671
+ModuleInits.push_back(I->second);
+}
+PrioritizedCXXGlobalInits.clear();

ChuanqiXu wrote:
> iains wrote:
> > ChuanqiXu wrote:
> > > I find the process here for `PrioritizedCXXGlobalInits` is much simpler 
> > > than the one done in `CodeGenModule::EmitCXXGlobalInitFunc()`. It would 
> > > generate more global init function. Doesn't it matter?
> > In the general CXX initializer the prioritized inits are grouped into 
> > functions named in a way that allows them to be collated by other tools 
> > (e.g. the static linker) so that init priority can be honoured over 
> > multiple TUs. 
> > 
> > This is not feasible for module initializers, there is no way to fuze them 
> > across TUs (it would have no meaning).  So I think all we need to do here 
> > is to emit the calls in the correct order (in the end there are no more 
> > initializer functions, we just skip the intermediated grouping functions,
> > 
> > Of course, init priority is not mentioned in the standard, so that really 
> > what would matter is that compiler 'vendors' agree on a sensible approach 
> > to make sure code behaves in an expected manner for common toolchains.
> Yeah, the prioritized inits are really odd fashion to me..  I only saw such 
> things in assembly. So my understanding to this one is that:
> (1) If we have prioritized inits in modules' code , it is not guaranteed to 
> be initialized first. This is the vendors agreement.
> (2) If we link a static library (e.g., libgcc.a), the prioritized inits in 
> that would still be initialized first.
> 
> Do I understand right?
We should avoid side-tracking this patch with too much discussion of the 
problems of C++ global initialisers.  Behaviour of things like static libraries 
depends on the toolchain, binary container, platform 

1. prioritised initializers are not part of the standard
2. they are optional (some toolchains do not handle them at all - e.g. macOS)
3. where they are handled, usually the toolchain makes no guarantees about 
behaviour between TUs
4. (for a regular ELF-style C++ global init) there is a convention to group 
initializers for a specific priority into a function with a specific mangled 
name that allows (optionally) external tools - like the linker  -to group / 
order the inits _between_ TUs.

None of this is relevant to Module initializers, we just have to ensure that 
imported module inits are run before the current module's.




Comment at: clang/lib/CodeGen/CodeGenModule.cpp:3206-3208
+  bool MBEE = MayBeEmittedEagerly(Global);
+  if (MustBeEmitted(Global)) {
+if (MBEE) {

ChuanqiXu wrote:
> iains wrote:
> > ChuanqiXu wrote:
> > > Is this change needed? Could you elaborate more on this?
> > the test is quite heavy, and is used twice when the assertions are enabled 
> > (I do not mind to revert this part if that does not seem worthwhile).
> > 
> I prefer to do such changes in a standalone NFC patch.
OK .. fair enough, reverted for now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Iain Sandoe via Phabricator via cfe-commits]

iains updated this revision to Diff 433049.
iains marked 7 inline comments as done.
iains added a comment.

rebased, addressed review comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/Module.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Parse/ParseAST.cpp
  clang/test/CodeGen/module-intializer-pmf.cpp
  clang/test/CodeGen/module-intializer.cpp

Index: clang/test/CodeGen/module-intializer.cpp
===
--- /dev/null
+++ clang/test/CodeGen/module-intializer.cpp
@@ -0,0 +1,172 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 N.cpp \
+// RUN:-emit-module-interface -o N.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 N.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-N
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 O.cpp \
+// RUN:-emit-module-interface -o O.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 O.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-O
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M-part.cpp \
+// RUN:-emit-module-interface -o M-part.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M-part.pcm -S \
+// RUN: -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-P
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M.cpp \
+// RUN: -fmodule-file=N.pcm -fmodule-file=O.pcm -fmodule-file=M-part.pcm \
+// RUN:-emit-module-interface -o M.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-M
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 useM.cpp \
+// RUN: -fmodule-file=M.pcm -S -emit-llvm  -o - \
+// RUN: | FileCheck %s --check-prefix=CHECK-USE
+
+//--- N-h.h
+
+struct Oink {
+  Oink(){};
+};
+
+Oink Hog;
+
+//--- N.cpp
+
+module;
+#include "N-h.h"
+
+export module N;
+
+export struct Quack {
+  Quack(){};
+};
+
+export Quack Duck;
+
+// CHECK-N: define internal void @__cxx_global_var_init
+// CHECK-N: call void @_ZN4OinkC1Ev
+// CHECK-N: define internal void @__cxx_global_var_init
+// CHECK-N: call void @_ZNW1N5QuackC1Ev
+// CHECK-N: define void @_ZGIW1N
+// CHECK-N: store i8 1, ptr @_ZGIW1N__in_chrg
+// CHECK-N: call void @__cxx_global_var_init
+// CHECK-N: call void @__cxx_global_var_init
+
+//--- O-h.h
+
+struct Meow {
+  Meow(){};
+};
+
+Meow Cat;
+
+//--- O.cpp
+
+module;
+#include "O-h.h"
+
+export module O;
+
+export struct Bark {
+  Bark(){};
+};
+
+export Bark Dog;
+
+// CHECK-O: define internal void @__cxx_global_var_init
+// CHECK-O: call void @_ZN4MeowC2Ev
+// CHECK-O: define internal void @__cxx_global_var_init
+// CHECK-O: call void @_ZNW1O4BarkC1Ev
+// CHECK-O: define void @_ZGIW1O
+// CHECK-O: store i8 1, ptr @_ZGIW1O__in_chrg
+// CHECK-O: call void @__cxx_global_var_init
+// CHECK-O: call void @__cxx_global_var_init
+
+//--- P-h.h
+
+struct Croak {
+  Croak(){};
+};
+
+Croak Frog;
+
+//--- M-part.cpp
+
+module;
+#include "P-h.h"
+
+module M:Part;
+
+struct Squawk {
+  Squawk(){};
+};
+
+Squawk parrot;
+
+// CHECK-P: define internal void @__cxx_global_var_init
+// CHECK-P: call void @_ZN5CroakC1Ev
+// CHECK-P: define internal void @__cxx_global_var_init
+// CHECK-P: call void @_ZNW1M6SquawkC1Ev
+// CHECK-P: define void @_ZGIW1MWP4Part
+// CHECK-P: store i8 1, ptr @_ZGIW1MWP4Part__in_chrg
+// CHECK-P: call void @__cxx_global_var_init
+// CHECK-P: call void @__cxx_global_var_init
+
+//--- M-h.h
+
+struct Moo {
+  Moo(){};
+};
+
+Moo Cow;
+
+//--- M.cpp
+
+module;
+#include "M-h.h"
+
+export module M;
+import N;
+export import O;
+import :Part;
+
+export struct Baa {
+  int x;
+  Baa(){};
+  Baa(int x) : x(x) {}
+  int getX() { return x; }
+};
+
+export Baa Sheep(10);
+
+// CHECK-M: define internal void @__cxx_global_var_init
+// CHECK-M: call void @_ZN3MooC1Ev
+// CHECK-M: define internal void @__cxx_global_var_init
+// CHECK-M: call void @_ZNW1M3BaaC1Ei
+// CHECK-M: declare void @_ZGIW1O()
+// CHECK-M: declare void @_ZGIW1N()
+// CHECK-M: declare void @_ZGIW1MWP4Part()
+// CHECK-M: define void @_ZGIW1M
+// CHECK-M: store i8 1, ptr @_ZGIW1M__in_chrg
+// CHECK-M: call void @_ZGIW1O()
+// CHECK-M: call void @_ZGIW1N()
+// CHECK-M: call void @_ZGIW1MWP4Part()
+// CHECK-M: call void @__cxx_global_var_init
+// CHECK-M: call void @__cxx_global_var_init
+
+//--- useM.cpp
+
+import M;
+
+int main() {
+  return Sheep.getX();
+}
+
+// CHECK-USE: declare void @_ZGIW1M
+// CHECK-USE: define internal void @_GLOBAL__sub_I_useM.cpp
+// CHECK-USE: call void @_ZGIW1M()
Index: clang/test/CodeGen/module-intializer-pmf.cpp
==

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:621
 
+void CodeGenModule::EmitCXXModuleInitFunc(Module *Primary) {
+  while (!CXXGlobalInits.empty() && !CXXGlobalInits.back())

iains wrote:
> ChuanqiXu wrote:
> > iains wrote:
> > > ChuanqiXu wrote:
> > > > Recommend to comment related part in your summary. It should be much 
> > > > helpful for other to read the codes.
> > > Hmm. I am not sure exactly what you mean here - there is a comment about 
> > > the purpose of the method, where the method is declared.
> > > The commit message describes what this method does in the first part.  
> > > I'm happy to make things more clear, but not sure where you want to see 
> > > some change,
> > I mean the paragraph:
> > ```
> > For a module (instead of the generic CXX initializer) we emit a module init
> > that:
> > 
> > - wraps the contained initializations in a control variable to ensure that 
> > the inits only happen once, even if a module is imported many times by 
> > imports of the main unit.
> > - calls module initialisers for imported modules first. Note that the order 
> > of module import is not significant, and therefore neither is the order of 
> > imported module initializers.
> > - We then call initializers for the Global Module Fragment (if present)
> > - We then call initializers for the current module.
> > - We then call initializers for the Private Module Fragment (if present)
> > ```
> > 
> > I understand people might feel like this is wordy. But, **personally**, I 
> > prefer readability.
> so, to clarify - you would like me to add this description as a comment block 
> on the new initializer method (I am OK with doing this)
Yeah, I feel it is helpful.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Iain Sandoe via Phabricator via cfe-commits]

iains added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:621
 
+void CodeGenModule::EmitCXXModuleInitFunc(Module *Primary) {
+  while (!CXXGlobalInits.empty() && !CXXGlobalInits.back())

ChuanqiXu wrote:
> iains wrote:
> > ChuanqiXu wrote:
> > > Recommend to comment related part in your summary. It should be much 
> > > helpful for other to read the codes.
> > Hmm. I am not sure exactly what you mean here - there is a comment about 
> > the purpose of the method, where the method is declared.
> > The commit message describes what this method does in the first part.  I'm 
> > happy to make things more clear, but not sure where you want to see some 
> > change,
> I mean the paragraph:
> ```
> For a module (instead of the generic CXX initializer) we emit a module init
> that:
> 
> - wraps the contained initializations in a control variable to ensure that 
> the inits only happen once, even if a module is imported many times by 
> imports of the main unit.
> - calls module initialisers for imported modules first. Note that the order 
> of module import is not significant, and therefore neither is the order of 
> imported module initializers.
> - We then call initializers for the Global Module Fragment (if present)
> - We then call initializers for the current module.
> - We then call initializers for the Private Module Fragment (if present)
> ```
> 
> I understand people might feel like this is wordy. But, **personally**, I 
> prefer readability.
so, to clarify - you would like me to add this description as a comment block 
on the new initializer method (I am OK with doing this)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:621
 
+void CodeGenModule::EmitCXXModuleInitFunc(Module *Primary) {
+  while (!CXXGlobalInits.empty() && !CXXGlobalInits.back())

iains wrote:
> ChuanqiXu wrote:
> > Recommend to comment related part in your summary. It should be much 
> > helpful for other to read the codes.
> Hmm. I am not sure exactly what you mean here - there is a comment about the 
> purpose of the method, where the method is declared.
> The commit message describes what this method does in the first part.  I'm 
> happy to make things more clear, but not sure where you want to see some 
> change,
I mean the paragraph:
```
For a module (instead of the generic CXX initializer) we emit a module init
that:

- wraps the contained initializations in a control variable to ensure that the 
inits only happen once, even if a module is imported many times by imports of 
the main unit.
- calls module initialisers for imported modules first. Note that the order of 
module import is not significant, and therefore neither is the order of 
imported module initializers.
- We then call initializers for the Global Module Fragment (if present)
- We then call initializers for the current module.
- We then call initializers for the Private Module Fragment (if present)
```

I understand people might feel like this is wordy. But, **personally**, I 
prefer readability.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:645-646
+  llvm::raw_svector_ostream Out(FnName);
+  cast(getCXXABI().getMangleContext())
+  .mangleModuleInitializer(M, Out);
+}

iains wrote:
> ChuanqiXu wrote:
> > mangleModuleInitializer is virtual function and we don't need to cast it.
> Actually, 'mangleModuleInitializer' is not a virtual function of the 
> 'MangleContext' class, only of the 'ItaniumMangleContext'.
> (see D122741).
> 
> This because we do not know the mangling for MS and, in principle, there 
> might never be such a mangling (e.g. that implementation might deal with 
> P1874 in some different way).
> 
> I did have a version of this where I amended the mangling to make the 
> function virtual in the 'MangleContext' class, but then that means generating 
> a dummy MS version that, as noted above, might never exist in reality.
> 
I was just afraid of people might blame us to bring ABI specific things to 
CodeGen* things. I found there similar uses in CGVTables.cpp and CGVTT.cpp. So 
this might be acceptable too.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:671
+ModuleInits.push_back(I->second);
+}
+PrioritizedCXXGlobalInits.clear();

iains wrote:
> ChuanqiXu wrote:
> > I find the process here for `PrioritizedCXXGlobalInits` is much simpler 
> > than the one done in `CodeGenModule::EmitCXXGlobalInitFunc()`. It would 
> > generate more global init function. Doesn't it matter?
> In the general CXX initializer the prioritized inits are grouped into 
> functions named in a way that allows them to be collated by other tools (e.g. 
> the static linker) so that init priority can be honoured over multiple TUs. 
> 
> This is not feasible for module initializers, there is no way to fuze them 
> across TUs (it would have no meaning).  So I think all we need to do here is 
> to emit the calls in the correct order (in the end there are no more 
> initializer functions, we just skip the intermediated grouping functions,
> 
> Of course, init priority is not mentioned in the standard, so that really 
> what would matter is that compiler 'vendors' agree on a sensible approach to 
> make sure code behaves in an expected manner for common toolchains.
Yeah, the prioritized inits are really odd fashion to me..  I only saw such 
things in assembly. So my understanding to this one is that:
(1) If we have prioritized inits in modules' code , it is not guaranteed to be 
initialized first. This is the vendors agreement.
(2) If we link a static library (e.g., libgcc.a), the prioritized inits in that 
would still be initialized first.

Do I understand right?



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2489
+  // source, first Global Module Fragments, if present.
+  if (auto GMF = Primary->findSubmodule("")) {
+for (Decl *D : getContext().getModuleInitializers(GMF)) {

iains wrote:
> ChuanqiXu wrote:
> > Nit: I feel better to add methods like `Module 
> > *Module::getGlobalModuleFragment()`. I feel odd to use magic string here. 
> > But I am OK with it since there many magic string in CodeGen codes and we 
> > got in consensus before that we would need to do a lot of refactor work in 
> > the future.
> yeah, I was intending to do this actually - although it only moves the magic 
> strings to a different place.  It might be that we have enough bits in the 
> module type enumerator (already streamed) so that we could use up two values 
> with "GMF" and "PMF

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

not sure why the Debian clang-format is complaining - I am using llvm-14's 
clang-format with git clang-format + arc.




Comment at: clang/include/clang/AST/ASTContext.h:476
+  /// For module code-gen cases, this is the top-level module we are building.
+  mutable Module *PrimaryModule = nullptr;
+

ChuanqiXu wrote:
> The name `PrimaryModule` looks confusing. PrimaryModule looks like primary 
> module interface unit to me. I think it refers to current module unit only. I 
> think it is better to rename to `CurrentModuleUnit`.
> 
> Then why is it mutable? I don't find it is changed in a const member function.
> The name `PrimaryModule` looks confusing. PrimaryModule looks like primary 
> module interface unit to me. I think it refers to current module unit only. I 
> think it is better to rename to `CurrentModuleUnit`.

I think we want to be clear that this is the Top Level Module (and not some 
dependent or sub-module) - so I have renamed to "TopLevelModule".

> Then why is it mutable? I don't find it is changed in a const member function.

Ah well caught; the implementation went through several iterations and I missed 
to remove this.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:621
 
+void CodeGenModule::EmitCXXModuleInitFunc(Module *Primary) {
+  while (!CXXGlobalInits.empty() && !CXXGlobalInits.back())

ChuanqiXu wrote:
> Recommend to comment related part in your summary. It should be much helpful 
> for other to read the codes.
Hmm. I am not sure exactly what you mean here - there is a comment about the 
purpose of the method, where the method is declared.
The commit message describes what this method does in the first part.  I'm 
happy to make things more clear, but not sure where you want to see some change,



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:626-627
+  // We create the function, even if it is empty, since an importer of this
+  // module will refer to it unconditionally (there is no way for an importer
+  // to know if the function could be omitted at this time).
+

urnathan wrote:
> 'at this time' is ambiguous.  I think it means 'the current compiler 
> implementation state', but it could also mean 'at this point in the 
> compilation'.  I don't  think there's a general problem with such an 
> optimization -- we could emit a flag into the BMI.  It's just we don't have 
> the smarts to do that just yet.  right?
yeah, I meant "as currently implemented" - adding flags to the BMI attracts 
concomitant churn in the serialisation, so probably better done separately.  
Amended the comment.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:629
+
+  // Module initialisers for imported modules are emitted first.
+  // Collect the modules that we import

urnathan wrote:
> I'm with Morse about this, even before coming to live where I do.  You've 
> used both 'ise' and 'ize'. s/ise/ize/
tools tools ... my editor wants "ise" unless I remember to set it to USian .. 
and sometimes I miss a change,, hopefully will catch them all as I go.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:645-646
+  llvm::raw_svector_ostream Out(FnName);
+  cast(getCXXABI().getMangleContext())
+  .mangleModuleInitializer(M, Out);
+}

ChuanqiXu wrote:
> mangleModuleInitializer is virtual function and we don't need to cast it.
Actually, 'mangleModuleInitializer' is not a virtual function of the 
'MangleContext' class, only of the 'ItaniumMangleContext'.
(see D122741).

This because we do not know the mangling for MS and, in principle, there might 
never be such a mangling (e.g. that implementation might deal with P1874 in 
some different way).

I did have a version of this where I amended the mangling to make the function 
virtual in the 'MangleContext' class, but then that means generating a dummy MS 
version that, as noted above, might never exist in reality.




Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:671
+ModuleInits.push_back(I->second);
+}
+PrioritizedCXXGlobalInits.clear();

ChuanqiXu wrote:
> I find the process here for `PrioritizedCXXGlobalInits` is much simpler than 
> the one done in `CodeGenModule::EmitCXXGlobalInitFunc()`. It would generate 
> more global init function. Doesn't it matter?
In the general CXX initializer the prioritized inits are grouped into functions 
named in a way that allows them to be collated by other tools (e.g. the static 
linker) so that init priority can be honoured over multiple TUs. 

This is not feasible for module initializers, there is no way to fuze them 
across TUs (it would have no meaning).  So I think all we need to do here is to 
emit the calls in the correct order (in the end there are no more initializer 
functions, we just skip the intermediated grouping functions,

Of course, init priority is not mentioned in the standard

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Iain Sandoe via Phabricator via cfe-commits]

iains updated this revision to Diff 432241.
iains marked 14 inline comments as done.
iains added a comment.

address review comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/Module.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Parse/ParseAST.cpp
  clang/test/CodeGen/module-intializer-pmf.cpp
  clang/test/CodeGen/module-intializer.cpp

Index: clang/test/CodeGen/module-intializer.cpp
===
--- /dev/null
+++ clang/test/CodeGen/module-intializer.cpp
@@ -0,0 +1,172 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 N.cpp \
+// RUN:-emit-module-interface -o N.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 N.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-N
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 O.cpp \
+// RUN:-emit-module-interface -o O.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 O.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-O
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M-part.cpp \
+// RUN:-emit-module-interface -o M-part.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M-part.pcm -S \
+// RUN: -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-P
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M.cpp \
+// RUN: -fmodule-file=N.pcm -fmodule-file=O.pcm -fmodule-file=M-part.pcm \
+// RUN:-emit-module-interface -o M.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-M
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 useM.cpp \
+// RUN: -fmodule-file=M.pcm -S -emit-llvm  -o - \
+// RUN: | FileCheck %s --check-prefix=CHECK-USE
+
+//--- N-h.h
+
+struct Oink {
+  Oink(){};
+};
+
+Oink Hog;
+
+//--- N.cpp
+
+module;
+#include "N-h.h"
+
+export module N;
+
+export struct Quack {
+  Quack(){};
+};
+
+export Quack Duck;
+
+// CHECK-N: define internal void @__cxx_global_var_init
+// CHECK-N: call void @_ZN4OinkC1Ev
+// CHECK-N: define internal void @__cxx_global_var_init
+// CHECK-N: call void @_ZNW1N5QuackC1Ev
+// CHECK-N: define void @_ZGIW1N
+// CHECK-N: store i8 1, ptr @_ZGIW1N__in_chrg
+// CHECK-N: call void @__cxx_global_var_init
+// CHECK-N: call void @__cxx_global_var_init
+
+//--- O-h.h
+
+struct Meow {
+  Meow(){};
+};
+
+Meow Cat;
+
+//--- O.cpp
+
+module;
+#include "O-h.h"
+
+export module O;
+
+export struct Bark {
+  Bark(){};
+};
+
+export Bark Dog;
+
+// CHECK-O: define internal void @__cxx_global_var_init
+// CHECK-O: call void @_ZN4MeowC2Ev
+// CHECK-O: define internal void @__cxx_global_var_init
+// CHECK-O: call void @_ZNW1O4BarkC1Ev
+// CHECK-O: define void @_ZGIW1O
+// CHECK-O: store i8 1, ptr @_ZGIW1O__in_chrg
+// CHECK-O: call void @__cxx_global_var_init
+// CHECK-O: call void @__cxx_global_var_init
+
+//--- P-h.h
+
+struct Croak {
+  Croak(){};
+};
+
+Croak Frog;
+
+//--- M-part.cpp
+
+module;
+#include "P-h.h"
+
+module M:Part;
+
+struct Squawk {
+  Squawk(){};
+};
+
+Squawk parrot;
+
+// CHECK-P: define internal void @__cxx_global_var_init
+// CHECK-P: call void @_ZN5CroakC1Ev
+// CHECK-P: define internal void @__cxx_global_var_init
+// CHECK-P: call void @_ZNW1M6SquawkC1Ev
+// CHECK-P: define void @_ZGIW1MWP4Part
+// CHECK-P: store i8 1, ptr @_ZGIW1MWP4Part__in_chrg
+// CHECK-P: call void @__cxx_global_var_init
+// CHECK-P: call void @__cxx_global_var_init
+
+//--- M-h.h
+
+struct Moo {
+  Moo(){};
+};
+
+Moo Cow;
+
+//--- M.cpp
+
+module;
+#include "M-h.h"
+
+export module M;
+import N;
+export import O;
+import :Part;
+
+export struct Baa {
+  int x;
+  Baa(){};
+  Baa(int x) : x(x) {}
+  int getX() { return x; }
+};
+
+export Baa Sheep(10);
+
+// CHECK-M: define internal void @__cxx_global_var_init
+// CHECK-M: call void @_ZN3MooC1Ev
+// CHECK-M: define internal void @__cxx_global_var_init
+// CHECK-M: call void @_ZNW1M3BaaC1Ei
+// CHECK-M: declare void @_ZGIW1O()
+// CHECK-M: declare void @_ZGIW1N()
+// CHECK-M: declare void @_ZGIW1MWP4Part()
+// CHECK-M: define void @_ZGIW1M
+// CHECK-M: store i8 1, ptr @_ZGIW1M__in_chrg
+// CHECK-M: call void @_ZGIW1O()
+// CHECK-M: call void @_ZGIW1N()
+// CHECK-M: call void @_ZGIW1MWP4Part()
+// CHECK-M: call void @__cxx_global_var_init
+// CHECK-M: call void @__cxx_global_var_init
+
+//--- useM.cpp
+
+import M;
+
+int main() {
+  return Sheep.getX();
+}
+
+// CHECK-USE: declare void @_ZGIW1M
+// CHECK-USE: define internal void @_GLOBAL__sub_I_useM.cpp
+// CHECK-USE: call void @_ZGIW1M()
Index: clang/test/CodeGen/module-intializer-pmf.cpp
=

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:699
+Guard = new llvm::GlobalVariable(getModule(), Int8Ty, /*isConstant=*/false,
+ llvm::GlobalVariable::InternalLinkage,
+ llvm::ConstantInt::get(Int8Ty, 0),

ChuanqiXu wrote:
> Should the Guard be internal linkage? I image that it is possible to be 
> manipulated by different TUs. So I feel like it might be better to be 
> linkonce or linkonce_odr?
Oh, I realized I'm wrong. Don't remind this one.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/include/clang/AST/ASTContext.h:476
+  /// For module code-gen cases, this is the top-level module we are building.
+  mutable Module *PrimaryModule = nullptr;
+

The name `PrimaryModule` looks confusing. PrimaryModule looks like primary 
module interface unit to me. I think it refers to current module unit only. I 
think it is better to rename to `CurrentModuleUnit`.

Then why is it mutable? I don't find it is changed in a const member function.



Comment at: clang/include/clang/AST/ASTContext.h:1085
+  /// Get module under construction, nullptr if this is not a C++20 module.
+  Module *getModuleForCodeGen() { return PrimaryModule; }
+





Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:621
 
+void CodeGenModule::EmitCXXModuleInitFunc(Module *Primary) {
+  while (!CXXGlobalInits.empty() && !CXXGlobalInits.back())

Recommend to comment related part in your summary. It should be much helpful 
for other to read the codes.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:645-646
+  llvm::raw_svector_ostream Out(FnName);
+  cast(getCXXABI().getMangleContext())
+  .mangleModuleInitializer(M, Out);
+}

mangleModuleInitializer is virtual function and we don't need to cast it.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:671
+ModuleInits.push_back(I->second);
+}
+PrioritizedCXXGlobalInits.clear();

I find the process here for `PrioritizedCXXGlobalInits` is much simpler than 
the one done in `CodeGenModule::EmitCXXGlobalInitFunc()`. It would generate 
more global init function. Doesn't it matter?



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:684
+  // We now build the initialiser for this module, which has a mangled name
+  // as per the Itanium ABI .  The action of the initializer is guarded so that
+  // each init is run just once (even though a module might be imported

Given CodeGenModule is designed as ABI independent, I feel better to avoid see 
Itanium ABI in this function. (Although we don't have other ABI for modules 
now..)



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:692-693
+llvm::raw_svector_ostream Out(InitFnName);
+cast(getCXXABI().getMangleContext())
+.mangleModuleInitializer(getContext().getModuleForCodeGen(), Out);
+Fn = CreateGlobalInitOrCleanUpFunction(





Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:699
+Guard = new llvm::GlobalVariable(getModule(), Int8Ty, /*isConstant=*/false,
+ llvm::GlobalVariable::InternalLinkage,
+ llvm::ConstantInt::get(Int8Ty, 0),

Should the Guard be internal linkage? I image that it is possible to be 
manipulated by different TUs. So I feel like it might be better to be linkonce 
or linkonce_odr?



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:706-722
+  CodeGenFunction(*this).GenerateCXXGlobalInitFunc(
+  Fn, ModuleInits, ConstantAddress(Guard, Int8Ty, GuardAlign));
+  AddGlobalCtor(Fn);
+
+  // See the comment in EmitCXXGlobalInitFunc about OpenCL global init
+  // functions.
+  if (getLangOpts().OpenCL) {

The codes look highly similar to CodeGenModule::EmitCXXGlobalInitFunc. I feel 
it is better to hoist a new function to avoid repeating the same logic twice.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:846
+
+  CodeGenFunction(*this).GenerateCXXGlobalInitFunc(Fn, ModuleInits);
   AddGlobalCtor(Fn);

This looks odd since it might be possible to not handling modules. But I 
understand it might be time-consuming if we want to use `CXXGloablInits` and we 
want `ModuleInits` to be front.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2489
+  // source, first Global Module Fragments, if present.
+  if (auto GMF = Primary->findSubmodule("")) {
+for (Decl *D : getContext().getModuleInitializers(GMF)) {

Nit: I feel better to add methods like `Module 
*Module::getGlobalModuleFragment()`. I feel odd to use magic string here. But I 
am OK with it since there many magic string in CodeGen codes and we got in 
consensus before that we would need to do a lot of refactor work in the future.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2503
+  // Third any associated with the Privat eMOdule Fragment, if present.
+  if (auto PMF = Primary->findSubmodule("")) {
+for (Decl *D : getContext().getModuleInitializers(PMF)) {

Nit: with above: `Module *Module::getPrivateModuleFragment()`.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2933-2935
+  // llvm::dbgs() << "deferring: ";
+  // VD->dump();
+  return false;





Comment at: clang/lib/C

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Nathan Sidwell via Phabricator via cfe-commits]

urnathan added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:626-627
+  // We create the function, even if it is empty, since an importer of this
+  // module will refer to it unconditionally (there is no way for an importer
+  // to know if the function could be omitted at this time).
+

'at this time' is ambiguous.  I think it means 'the current compiler 
implementation state', but it could also mean 'at this point in the 
compilation'.  I don't  think there's a general problem with such an 
optimization -- we could emit a flag into the BMI.  It's just we don't have the 
smarts to do that just yet.  right?



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:629
+
+  // Module initialisers for imported modules are emitted first.
+  // Collect the modules that we import

I'm with Morse about this, even before coming to live where I do.  You've used 
both 'ise' and 'ize'. s/ise/ize/


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126189/new/

https://reviews.llvm.org/D126189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Iain Sandoe via Phabricator via cfe-commits]

iains created this revision.
Herald added a project: All.
iains added reviewers: urnathan, Bigcheese, ChuanqiXu, jansvoboda11.
iains added a subscriber: clang-modules.
iains edited the summary of this revision.
iains edited the summary of this revision.
iains edited the summary of this revision.
iains edited the summary of this revision.
iains edited the summary of this revision.
iains updated this revision to Diff 431313.
iains added a comment.
iains published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

just updated description


Currently we only implement this for the Itanium ABI since the correct
mangling for the initializers in other ABIs is not yet known.

Intended result:

For a module (instead of the generic CXX initializer) we emit a module init
that:

- wraps the contained initializations in a control variable to ensure that the 
inits only happen once, even if a module is imported many times by imports of 
the main unit.
- calls module initialisers for imported modules first.  Note that the order of 
module import is not significant, and therefore neither is the order of 
imported module initializers.
- We then call initializers for the Global Module Fragment (if present)
- We then call initializers for the current module.
- We then call initializers for the Private Module Fragment (if present)

For a non-module TU that imports at least one module we emit a regular CXX
init that:

- Calls the initializers for any imported modules first.
- Then proceeds as normal with remaining inits.

Implementation:

- We provide the module pointer in the AST Context so that CodeGen can act on 
it and its sub-modules.
- We need to account for module build lines like this: ` clang -cc1 -std=c++20 
Foo.pcm -emit-obj -o Foo.o` or ` clang -cc1 -std=c++20 -xc++-module Foo.cpp 
-emit-obj -o Foo.o`
- in order to do this, we add to ParseAST to set the module pointer in the 
ASTContext, once we establish that this is a module build and we know the 
module pointer. To be able to do this, we make the query for current module 
public in Sema.
- iIn CodeGen, we determine if the current build requires a CXX20-style module 
init and, if so, we defer any module initializers during the "Eagerly Emitted" 
phase.
- We then walk the module initializers at the end of the TU but before emitting 
deferred inits (which adds any hidden and static ones, fixing 
https://github.com/llvm/llvm-project/issues/51873 ).
- We then proceed to emit the deferred inits and continue to emit the CXX init 
function.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126189

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Parse/ParseAST.cpp
  clang/test/CodeGen/module-intializer.cpp

Index: clang/test/CodeGen/module-intializer.cpp
===
--- /dev/null
+++ clang/test/CodeGen/module-intializer.cpp
@@ -0,0 +1,172 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 N.cpp \
+// RUN:-emit-module-interface -o N.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 N.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-N
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 O.cpp \
+// RUN:-emit-module-interface -o O.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 O.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-O
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M-part.cpp \
+// RUN:-emit-module-interface -o M-part.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M-part.pcm -S \
+// RUN: -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-P
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M.cpp \
+// RUN: -fmodule-file=N.pcm -fmodule-file=O.pcm -fmodule-file=M-part.pcm \
+// RUN:-emit-module-interface -o M.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-M
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 useM.cpp \
+// RUN: -fmodule-file=M.pcm -S -emit-llvm  -o - \
+// RUN: | FileCheck %s --check-prefix=CHECK-USE
+
+//--- N-h.h
+
+struct Oink {
+  Oink(){};
+};
+
+Oink Hog;
+
+//--- N.cpp
+
+module;
+#include "N-h.h"
+
+export module N;
+
+export struct Quack {
+  Quack(){};
+};
+
+export Quack Duck;
+
+// CHECK-N: define internal void @__cxx_global_var_init
+// CHECK-N: call void @_ZN4OinkC1Ev
+// CHECK-N: define internal void @__cxx_global_var_init
+// CHECK-N: call void @_ZNW1N5QuackC1Ev
+// CHECK-N: define void @_ZGIW1N
+// CHECK-N: store i8 1, ptr @_ZGIW1N__in_chrg
+// CHECK-N: call void @__cxx_global_var_init
+// CHECK-N: call void @__cxx_global_var_init
+
+//--- O-h.h
+
+struct Meow