[clang] [C++20] [Modules] Don't record implicitly declarations to BMI by default (PR #93459)

2024-05-27 Thread Chuanqi Xu via cfe-commits

https://github.com/ChuanqiXu9 closed 
https://github.com/llvm/llvm-project/pull/93459
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [C++20] [Modules] Don't record implicitly declarations to BMI by default (PR #93459)

2024-05-27 Thread via cfe-commits

llvmbot wrote:




@llvm/pr-subscribers-clang-modules

Author: Chuanqi Xu (ChuanqiXu9)


Changes

I found we may insert unused implciit declarations like AArch SVE declarations 
by default on AArch64 due to we will insert that by default. But it should be 
completely redundant and this patch tries to remove that.

---
Full diff: https://github.com/llvm/llvm-project/pull/93459.diff


2 Files Affected:

- (modified) clang/lib/Serialization/ASTWriter.cpp (+11-2) 
- (added) clang/test/Modules/no-implicit-declarations.cppm (+26) 


``diff
diff --git a/clang/lib/Serialization/ASTWriter.cpp 
b/clang/lib/Serialization/ASTWriter.cpp
index 00b0e48083217..a85cd94fd5b5a 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -5037,6 +5037,14 @@ void ASTWriter::PrepareWritingSpecialDecls(Sema 
) {
 continue;
 }
 
+// If we're writing C++ named modules, don't emit declarations which are
+// not from modules by default. They may be built in declarations (be
+// handled above) or implcit declarations (see the implementation of
+// `Sema::Initialize()` for example).
+if (isWritingStdCXXNamedModules() && !D->getOwningModule() &&
+D->isImplicit())
+  continue;
+
 GetDeclRef(D);
   }
 
@@ -6197,8 +6205,9 @@ bool ASTWriter::wasDeclEmitted(const Decl *D) const {
 return true;
 
   bool Emitted = DeclIDs.contains(D);
-  assert((Emitted || GeneratingReducedBMI) &&
- "The declaration can only be omitted in reduced BMI.");
+  assert((Emitted || (!D->getOwningModule() && isWritingStdCXXNamedModules()) 
||
+  GeneratingReducedBMI) &&
+ "The declaration within modules can only be omitted in reduced BMI.");
   return Emitted;
 }
 
diff --git a/clang/test/Modules/no-implicit-declarations.cppm 
b/clang/test/Modules/no-implicit-declarations.cppm
new file mode 100644
index 0..319d3a432ea23
--- /dev/null
+++ b/clang/test/Modules/no-implicit-declarations.cppm
@@ -0,0 +1,26 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+//
+// RUN: %clang_cc1 -std=c++20 %s -emit-module-interface -o %t/a.pcm
+// RUN: llvm-bcanalyzer --dump --disable-histogram --show-binary-blobs 
%t/a.pcm > %t/a.dump
+// RUN: cat %t/a.dump | FileCheck %s
+//
+// RUN: %clang_cc1 -std=c++20 %s -emit-reduced-module-interface -o %t/a.pcm
+// RUN: llvm-bcanalyzer --dump --disable-histogram --show-binary-blobs 
%t/a.pcm > %t/a.dump
+// RUN: cat %t/a.dump | FileCheck %s
+
+export module a;
+// Contain something at least to make sure the compiler won't
+// optimize this out.
+export int a = 43;
+
+// CHECK:  

``




https://github.com/llvm/llvm-project/pull/93459
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [C++20] [Modules] Don't record implicitly declarations to BMI by default (PR #93459)

2024-05-27 Thread Chuanqi Xu via cfe-commits

https://github.com/ChuanqiXu9 created 
https://github.com/llvm/llvm-project/pull/93459

I found we may insert unused implciit declarations like AArch SVE declarations 
by default on AArch64 due to we will insert that by default. But it should be 
completely redundant and this patch tries to remove that.

>From b9cc02ffea56214179fd70c3a0e206932a557f8f Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Mon, 27 May 2024 19:25:49 +0800
Subject: [PATCH] [C++20] [Modules] Don't record implicitly declarations to BMI
 by default

I found we may insert unused implciit declarations like AArch SVE
declarations by default on AArch64 due to we will insert that by
default. But it should be completely redundant and this patch tries to
remove that.
---
 clang/lib/Serialization/ASTWriter.cpp | 13 --
 .../Modules/no-implicit-declarations.cppm | 26 +++
 2 files changed, 37 insertions(+), 2 deletions(-)
 create mode 100644 clang/test/Modules/no-implicit-declarations.cppm

diff --git a/clang/lib/Serialization/ASTWriter.cpp 
b/clang/lib/Serialization/ASTWriter.cpp
index 00b0e48083217..a85cd94fd5b5a 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -5037,6 +5037,14 @@ void ASTWriter::PrepareWritingSpecialDecls(Sema 
) {
 continue;
 }
 
+// If we're writing C++ named modules, don't emit declarations which are
+// not from modules by default. They may be built in declarations (be
+// handled above) or implcit declarations (see the implementation of
+// `Sema::Initialize()` for example).
+if (isWritingStdCXXNamedModules() && !D->getOwningModule() &&
+D->isImplicit())
+  continue;
+
 GetDeclRef(D);
   }
 
@@ -6197,8 +6205,9 @@ bool ASTWriter::wasDeclEmitted(const Decl *D) const {
 return true;
 
   bool Emitted = DeclIDs.contains(D);
-  assert((Emitted || GeneratingReducedBMI) &&
- "The declaration can only be omitted in reduced BMI.");
+  assert((Emitted || (!D->getOwningModule() && isWritingStdCXXNamedModules()) 
||
+  GeneratingReducedBMI) &&
+ "The declaration within modules can only be omitted in reduced BMI.");
   return Emitted;
 }
 
diff --git a/clang/test/Modules/no-implicit-declarations.cppm 
b/clang/test/Modules/no-implicit-declarations.cppm
new file mode 100644
index 0..319d3a432ea23
--- /dev/null
+++ b/clang/test/Modules/no-implicit-declarations.cppm
@@ -0,0 +1,26 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+//
+// RUN: %clang_cc1 -std=c++20 %s -emit-module-interface -o %t/a.pcm
+// RUN: llvm-bcanalyzer --dump --disable-histogram --show-binary-blobs 
%t/a.pcm > %t/a.dump
+// RUN: cat %t/a.dump | FileCheck %s
+//
+// RUN: %clang_cc1 -std=c++20 %s -emit-reduced-module-interface -o %t/a.pcm
+// RUN: llvm-bcanalyzer --dump --disable-histogram --show-binary-blobs 
%t/a.pcm > %t/a.dump
+// RUN: cat %t/a.dump | FileCheck %s
+
+export module a;
+// Contain something at least to make sure the compiler won't
+// optimize this out.
+export int a = 43;
+
+// CHECK:  

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