[PATCH] D134589: [C++20][Modules] Elide unused guard variables in Itanium ABI module initializers.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.


================
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:690-695
+  // If we have an empty initializer then we do not want to create a guard var.
+  // 'Empty' needs only to apply to init functions that we call directly, calls
+  // to imported module initializers need not be counted since those will
+  // contain local guards as needed (and might themselves be empty).
+  bool ElideGuard = PrioritizedCXXGlobalInits.empty() && CXXGlobalInits.empty();
+
----------------
Usually I'd expect this to be sunk down closer to its usage - any particular reason to have it up here? (not super far away, but seems further away than I'd expect)


================
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:738
+    if (ElideGuard) {
+      CodeGenFunction(*this).GenerateCXXGlobalInitFunc(Fn, ModuleInits);
+    } else {
----------------
Could we avoid duplicating this call, by doing something like this:
```
ConstantAddress Guard = ConstantAddress::invalid()
if (!PrioritizedCXXGlobalInits.empty() || !CXXGlobalInits.empty())
  Guard = ConstantAddress(new GlobalVariable(...), Int8Ty, GuardAlign);
CodeGenFunction(*this).GenerateCXXGlobalInitFunc(Fn, ModuleInits, Guard);
```
?


================
Comment at: clang/test/CodeGenCXX/module-initializer-guard-elision.cpp:22-43
+// This module has no global inits and does not import any other module
+//--- O.cpp
+
+export module O;
+
+export int foo ();
+
----------------
If it's pretty stable/reliable/not too brittle, maybe worth CHECKing the whole function in these cases (CHECK+CHECK-NEXT etc) to demonstrate they have nothing in them other than the return, or the call+return?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134589