[clang] [serialization] no transitive decl change (PR #92083)
ChuanqiXu9 wrote: > > I didn't recognize we may still have 32-bit machines > > I think 32 bit Arm are the last. > > > I'll revert it again. > > Sure, if you post the PR I can test the fix too. I know getting 32 bit > machines can be tricky. I sent https://github.com/llvm/llvm-project/pull/94603 . I feel it should at least be one of the problem but I am not sure if it can fix all the issues. https://github.com/llvm/llvm-project/pull/92083 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Testing32 bit for https://github.com/llvm/llvm-project/pull/92083 (PR #94603)
https://github.com/ChuanqiXu9 created
https://github.com/llvm/llvm-project/pull/94603
https://github.com/llvm/llvm-project/pull/92083 is failed on 32bit machine but
it is not easy to get a 32 bit machine.
And thanks for @DavidSpickett would love to test this.
I tried to review the code and I tried to find the position. But I can't test
it to make sure if can avoid the regression.
This is only a draft to locate the problems. I will refine it if we can make
sure where the problem is.
>From b8723369181fcc8f27b3a6ebcace44cefa3164cd Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Thu, 6 Jun 2024 11:51:05 +0800
Subject: [PATCH 1/2] [serialization] no transitive decl change (#92083)
Following of https://github.com/llvm/llvm-project/pull/86912
The motivation of the patch series is that, for a module interface unit
`X`, when the dependent modules of `X` changes, if the changes is not
relevant with `X`, we hope the BMI of `X` won't change. For the specific
patch, we hope if the changes was about irrelevant declaration changes,
we hope the BMI of `X` won't change. **However**, I found the patch
itself is not very useful in practice, since the adding or removing
declarations, will change the state of identifiers and types in most
cases.
That said, for the most simple example,
```
// partA.cppm
export module m:partA;
// partA.v1.cppm
export module m:partA;
export void a() {}
// partB.cppm
export module m:partB;
export void b() {}
// m.cppm
export module m;
export import :partA;
export import :partB;
// onlyUseB;
export module onlyUseB;
import m;
export inline void onluUseB() {
b();
}
```
the BMI of `onlyUseB` will change after we change the implementation of
`partA.cppm` to `partA.v1.cppm`. Since `partA.v1.cppm` introduces new
identifiers and types (the function prototype).
So in this patch, we have to write the tests as:
```
// partA.cppm
export module m:partA;
export int getA() { ... }
export int getA2(int) { ... }
// partA.v1.cppm
export module m:partA;
export int getA() { ... }
export int getA(int) { ... }
export int getA2(int) { ... }
// partB.cppm
export module m:partB;
export void b() {}
// m.cppm
export module m;
export import :partA;
export import :partB;
// onlyUseB;
export module onlyUseB;
import m;
export inline void onluUseB() {
b();
}
```
so that the new introduced declaration `int getA(int)` doesn't introduce
new identifiers and types, then the BMI of `onlyUseB` can keep
unchanged.
While it looks not so great, the patch should be the base of the patch
to erase the transitive change for identifiers and types since I don't
know how can we introduce new types and identifiers without introducing
new declarations. Given how tightly the relationship between
declarations, types and identifiers, I think we can only reach the ideal
state after we made the series for all of the three entties.
The design of the patch is similar to
https://github.com/llvm/llvm-project/pull/86912, which extends the
32-bit DeclID to 64-bit and use the higher bits to store the module file
index and the lower bits to store the Local Decl ID.
A slight difference is that we only use 48 bits to store the new DeclID
since we try to use the higher 16 bits to store the module ID in the
prefix of Decl class. Previously, we use 32 bits to store the module ID
and 32 bits to store the DeclID. I don't want to allocate additional
space so I tried to make the additional space the same as 64 bits. An
potential interesting thing here is about the relationship between the
module ID and the module file index. I feel we can get the module file
index by the module ID. But I didn't prove it or implement it. Since I
want to make the patch itself as small as possible. We can make it in
the future if we want.
Another change in the patch is the new concept Decl Index, which means
the index of the very big array `DeclsLoaded` in ASTReader. Previously,
the index of a loaded declaration is simply the Decl ID minus
PREDEFINED_DECL_NUMs. So there are some places they got used
ambiguously. But this patch tried to split these two concepts.
As https://github.com/llvm/llvm-project/pull/86912 did, the change will
increase the on-disk PCM file sizes. As the declaration ID may be the
most IDs in the PCM file, this can have the biggest impact on the size.
In my experiments, this change will bring 6.6% increase of the on-disk
PCM size. No compile-time performance regression observed. Given the
benefits in the motivation example, I think the cost is worthwhile.
---
clang/include/clang/AST/DeclBase.h| 17 +-
clang/include/clang/AST/DeclID.h | 18 +-
.../include/clang/Serialization/ASTBitCodes.h | 31 +++-
clang/include/clang/Serialization/ASTReader.h | 36 ++--
.../include/clang/Serialization/ModuleFile.h | 18 +-
.../clang/Serialization/ModuleManager.h | 2 +-
clang/lib/AST/DeclBase.cpp| 40 -
clang/lib/Serialization/ASTReader.cpp | 161 ++
[clang] e285818 - Revert "[serialization] no transitive decl change (#92083)"
Author: Chuanqi Xu
Date: 2024-06-06T17:49:59+08:00
New Revision: e2858189bd99e6914dc2f63ab55b053a74b4e58b
URL:
https://github.com/llvm/llvm-project/commit/e2858189bd99e6914dc2f63ab55b053a74b4e58b
DIFF:
https://github.com/llvm/llvm-project/commit/e2858189bd99e6914dc2f63ab55b053a74b4e58b.diff
LOG: Revert "[serialization] no transitive decl change (#92083)"
This reverts commit 97c866f6c86456b3316006e6beff47e68a81c00a.
This fails on 32bit machines. See
https://github.com/llvm/llvm-project/pull/92083
Added:
Modified:
clang/include/clang/AST/DeclBase.h
clang/include/clang/AST/DeclID.h
clang/include/clang/Serialization/ASTBitCodes.h
clang/include/clang/Serialization/ASTReader.h
clang/include/clang/Serialization/ModuleFile.h
clang/include/clang/Serialization/ModuleManager.h
clang/lib/AST/DeclBase.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTReaderDecl.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/lib/Serialization/ModuleFile.cpp
Removed:
clang/test/Modules/no-transitive-decls-change.cppm
diff --git a/clang/include/clang/AST/DeclBase.h
b/clang/include/clang/AST/DeclBase.h
index 5f19af1891b7..600ce73c7f01 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -708,7 +708,10 @@ class alignas(8) Decl {
/// Set the owning module ID. This may only be called for
/// deserialized Decls.
- void setOwningModuleID(unsigned ID);
+ void setOwningModuleID(unsigned ID) {
+assert(isFromASTFile() && "Only works on a deserialized declaration");
+*((unsigned*)this - 2) = ID;
+ }
public:
/// Determine the availability of the given declaration.
@@ -781,11 +784,19 @@ class alignas(8) Decl {
/// Retrieve the global declaration ID associated with this
/// declaration, which specifies where this Decl was loaded from.
- GlobalDeclID getGlobalID() const;
+ GlobalDeclID getGlobalID() const {
+if (isFromASTFile())
+ return (*((const GlobalDeclID *)this - 1));
+return GlobalDeclID();
+ }
/// Retrieve the global ID of the module that owns this particular
/// declaration.
- unsigned getOwningModuleID() const;
+ unsigned getOwningModuleID() const {
+if (isFromASTFile())
+ return *((const unsigned*)this - 2);
+return 0;
+ }
private:
Module *getOwningModuleSlow() const;
diff --git a/clang/include/clang/AST/DeclID.h
b/clang/include/clang/AST/DeclID.h
index 32d2ed41a374..614ba06b6386 100644
--- a/clang/include/clang/AST/DeclID.h
+++ b/clang/include/clang/AST/DeclID.h
@@ -19,8 +19,6 @@
#include "llvm/ADT/DenseMapInfo.h"
#include "llvm/ADT/iterator.h"
-#include
-
namespace clang {
/// Predefined declaration IDs.
@@ -109,16 +107,12 @@ class DeclIDBase {
///
/// DeclID should only be used directly in serialization. All other users
/// should use LocalDeclID or GlobalDeclID.
- using DeclID = uint64_t;
+ using DeclID = uint32_t;
protected:
DeclIDBase() : ID(PREDEF_DECL_NULL_ID) {}
explicit DeclIDBase(DeclID ID) : ID(ID) {}
- explicit DeclIDBase(unsigned LocalID, unsigned ModuleFileIndex) {
-ID = (DeclID)LocalID | ((DeclID)ModuleFileIndex << 32);
- }
-
public:
DeclID get() const { return ID; }
@@ -130,10 +124,6 @@ class DeclIDBase {
bool isInvalid() const { return ID == PREDEF_DECL_NULL_ID; }
- unsigned getModuleFileIndex() const { return ID >> 32; }
-
- unsigned getLocalDeclIndex() const;
-
friend bool operator==(const DeclIDBase , const DeclIDBase ) {
return LHS.ID == RHS.ID;
}
@@ -166,9 +156,6 @@ class LocalDeclID : public DeclIDBase {
LocalDeclID(PredefinedDeclIDs ID) : Base(ID) {}
explicit LocalDeclID(DeclID ID) : Base(ID) {}
- explicit LocalDeclID(unsigned LocalID, unsigned ModuleFileIndex)
- : Base(LocalID, ModuleFileIndex) {}
-
LocalDeclID ++() {
++ID;
return *this;
@@ -188,9 +175,6 @@ class GlobalDeclID : public DeclIDBase {
GlobalDeclID() : Base() {}
explicit GlobalDeclID(DeclID ID) : Base(ID) {}
- explicit GlobalDeclID(unsigned LocalID, unsigned ModuleFileIndex)
- : Base(LocalID, ModuleFileIndex) {}
-
// For DeclIDIterator to be able to convert a GlobalDeclID
// to a LocalDeclID.
explicit operator LocalDeclID() const { return LocalDeclID(this->ID); }
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h
b/clang/include/clang/Serialization/ASTBitCodes.h
index 3f3fb8869f5f..f59ff6af4c76 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -255,12 +255,6 @@ class DeclOffset {
}
};
-// The unaligned decl ID used in the Blobs of bistreams.
-using unaligned_decl_id_t =
-llvm::support::detail::packed_endian_specific_integral<
-serialization::DeclID, llvm::endianness::native,
-llvm::support::unaligned>;
-
/// The number of predefined preprocessed
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
ChuanqiXu9 wrote: > Just a note, I am building on Windows with MSVC cl.exe and ninja and get this: > > ``` > C:\Program Files\Microsoft Visual > Studio\2022\Community\VC\Tools\MSVC\14.39.33519\include\memory(3138): error > C2027: use of undefined type 'clang::clangd::ProjectModules' > ``` > > While building ConfigCompile.cpp.obj, I was able to solve it by adding > `#include "ProjectModules.h"` into `ConfigCompile.h` before `#include > "GlobalCompilationDatabase.h"`. > > Likewise with `BackgroundIndexLoader.cpp.obj` and adding to > `index\background.h`, it appears any time `GlobalCompilationDatabase.h` is > included that `ProjectModules.h` would need to be included first. It also > seems like `ProjectModules.h` cannot be included in > `GlobalCompilationDatabase.h` due to a circular dependency. I am still > building so there might be similar issues somewhere else in the codebase but > this is the base issue. > > Not sure how to resolve other than adding a `#include` everywhere, seems like > there an issue with include order on MSVC or something. I can't reproduce this. I can't find `ConfigCompile.h` even. But I made some changes about the interface to the patch from the suggestion of @kadircet . So I guess maybe it worth a new try. https://github.com/llvm/llvm-project/pull/66462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
ChuanqiXu9 wrote: > I think that needs to happen eventually as well Agreed. > similar to preambles supporting in-memory storage, and it's actually not that > hard, GenerateModuleInterfaceAction already has an overrideable > CreateOutputFile method. I am slightly confused. Do you say to move the IO to the memory instead of on-disk files? If true, we're close to that since we can write them to /tmp directories. I thought you're saying we need to get rid of the reading and writing process for module files. > Also this comment is not solely about virtiualizing CDB, but also about the > way we're setting up tests. I don't think we need any of the compile flags > you're explicitly setting in the tests. The whole purpose of current > implementation is to rely on new module paths we provide into the > header-search-options. So there isn't any value in spelling those out > explicitly, rendering test setup more complicated. Agreed. In the new added test, I meant to use `TestTU` with `AdditionalFiles` to mimic a multiple module units project, but it fails. It looks like it can only accept headers. I feel there are a lot of spaces to improve. But I think it may be fine to leave it as is and improve this later. My plan after this patch is to make the module files reusable. I think the functionality to support modules in clangd is at least usable after that. https://github.com/llvm/llvm-project/pull/66462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,87 @@
+//===- PrerequisiteModules.h -*-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_PREREQUISITEMODULES_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_PREREQUISITEMODULES_H
+
+#include "Compiler.h"
+#include "support/Path.h"
+
+#include "clang/Lex/HeaderSearchOptions.h"
+
+#include "llvm/ADT/StringSet.h"
+
+namespace clang {
+namespace clangd {
+
+class ModulesBuilder;
+
+/// Store all the needed module files information to parse a single
+/// source file. e.g.,
+///
+/// ```
+/// // a.cppm
+/// export module a;
+///
+/// // b.cppm
+/// export module b;
+/// import a;
+///
+/// // c.cppm
+/// export module c;
+/// import a;
+/// ```
+///
+/// For the source file `c.cppm`, an instance of the class will store
+/// the module files for `a.cppm` and `b.cppm`. But the module file for
`c.cppm`
ChuanqiXu9 wrote:
Oh, nice catch. The things written is not consistent with what I thought. I've
refactored it. Thanks.
https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
ChuanqiXu9 wrote: Done https://github.com/llvm/llvm-project/pull/66462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,370 @@
+//===- ModulesBuilder.cpp *-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ModulesBuilder.h"
+
+#include "Compiler.h"
+#include "support/Logger.h"
+
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/FrontendActions.h"
+
+#include "clang/Serialization/ASTReader.h"
+
+namespace clang {
+namespace clangd {
+
+namespace {
+
+// Create a path to store module files. Generally it should be:
+//
+// {TEMP_DIRS}/clangd/module_files/{PrefixDir}-%%-%%-%%-%%-%%-%%/.
+//
+// {TEMP_DIRS} is the temporary directory for the system, e.g., "/var/tmp"
+// or "C:/TEMP".
+//
+// '%%' means random value to make the generated path unique.
+//
+// \param MainFile is used to get the root of the project from global
+// compilation database. \param PrefixDir is used to get the user
+// defined prefix for module files. This is useful when we want to seperate
+// module files. e.g., we want to build module files for the same module unit
+// `a.cppm` with 2 different users `b.cpp` and `c.cpp` and we don't want the
+// module file for `b.cpp` be conflict with the module files for `c.cpp`. Then
+// we can put the 2 module files into different dirs like:
+//
+// ${TEMP_DIRS}/clangd/module_files/b.cpp/a.pcm
+// ${TEMP_DIRS}/clangd/module_files/c.cpp/a.pcm
+//
+// TODO: Move these module fils out of the temporary directory if the module
+// files are persistent.
+llvm::SmallString<256> getUniqueModuleFilesPath(PathRef MainFile,
+llvm::StringRef PrefixDir) {
+ llvm::SmallString<256> ResultPattern;
+
+ llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/true,
+ ResultPattern);
+
+ llvm::sys::path::append(ResultPattern, "clangd");
+ llvm::sys::path::append(ResultPattern, "module_files");
+
+ llvm::sys::path::append(ResultPattern, PrefixDir);
+
+ ResultPattern.append("-%%-%%-%%-%%-%%-%%");
+
+ llvm::SmallString<256> Result;
+ llvm::sys::fs::createUniquePath(ResultPattern, Result,
+ /*MakeAbsolute=*/false);
+
+ llvm::sys::fs::create_directories(Result);
+ return Result;
+}
+
+// Get the absolute path for the filename from the compile command.
+llvm::SmallString<128> getAbsolutePath(const tooling::CompileCommand ) {
+ llvm::SmallString<128> AbsolutePath;
+ if (llvm::sys::path::is_absolute(Cmd.Filename)) {
+AbsolutePath = Cmd.Filename;
+ } else {
+AbsolutePath = Cmd.Directory;
+llvm::sys::path::append(AbsolutePath, Cmd.Filename);
+llvm::sys::path::remove_dots(AbsolutePath, true);
+ }
+ return AbsolutePath;
+}
+
+// Get a unique module file path under \param ModuleFilesPrefix.
+std::string getModuleFilePath(llvm::StringRef ModuleName,
+ PathRef ModuleFilesPrefix) {
+ llvm::SmallString<256> ModuleFilePattern(ModuleFilesPrefix);
+ auto [PrimaryModuleName, PartitionName] = ModuleName.split(':');
+ llvm::sys::path::append(ModuleFilePattern, PrimaryModuleName);
+ if (!PartitionName.empty()) {
+ModuleFilePattern.append("-");
+ModuleFilePattern.append(PartitionName);
+ }
+
+ ModuleFilePattern.append(".pcm");
+
+ llvm::SmallString<256> ModuleFilePath;
+ llvm::sys::fs::createUniquePath(ModuleFilePattern, ModuleFilePath,
+ /*MakeAbsolute=*/false);
+
+ return std::string(ModuleFilePath);
+}
+} // namespace
+
+// FailedPrerequisiteModules - stands for the PrerequisiteModules which has
+// errors happened during the building process.
+class FailedPrerequisiteModules : public PrerequisiteModules {
+public:
+ ~FailedPrerequisiteModules() override = default;
+
+ // We shouldn't adjust the compilation commands based on
+ // FailedPrerequisiteModules.
+ void adjustHeaderSearchOptions(HeaderSearchOptions ) const override {
+ }
+
+ // FailedPrerequisiteModules can never be reused.
+ bool
+ canReuse(const CompilerInvocation ,
+ llvm::IntrusiveRefCntPtr) const override {
+return false;
+ }
+
+ // No module unit got built in FailedPrerequisiteModules.
+ bool isModuleUnitBuilt(llvm::StringRef ModuleName) const override {
+return false;
+ }
+};
+
+// StandalonePrerequisiteModules - stands for PrerequisiteModules for which all
+// the required modules are built successfully. All the module files
+// are owned by the StandalonePrerequisiteModules class.
+//
+// Any of the built module files won't be shared with other instances of the
+// class. So that we can avoid worrying thread safety.
+//
+// We don't need to worry about duplicated module names here since the standard
+// guarantees the module names should be unique to a program.
+class
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,115 @@
+//===- ModulesBuilder.h --*-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// Experimental support for C++20 Modules.
+//
+// Currently we simplify the implementations by preventing reusing module files
+// across different versions and different source files. But this is clearly a
+// waste of time and space in the end of the day.
+//
+// TODO: Supporting reusing module files across different versions and
+// different source files.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULES_BUILDER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULES_BUILDER_H
+
+#include "GlobalCompilationDatabase.h"
+#include "ProjectModules.h"
+
+#include "support/Path.h"
+#include "support/ThreadsafeFS.h"
+
+#include "clang/Frontend/CompilerInvocation.h"
+
+#include "llvm/ADT/SmallString.h"
+
+#include
+
+namespace clang {
+namespace clangd {
+
+/// Store all the needed module files information to parse a single
+/// source file. e.g.,
+///
+/// ```
+/// // a.cppm
+/// export module a;
+///
+/// // b.cppm
+/// export module b;
+/// import a;
+///
+/// // c.cppm
+/// export module c;
+/// import a;
+/// ```
+///
+/// For the source file `c.cppm`, an instance of the class will store
+/// the module files for `a.cppm` and `b.cppm`. But the module file for
`c.cppm`
+/// won't be stored. Since it is not needed to parse `c.cppm`.
+///
+/// Users should only get PrerequisiteModules from
+/// `ModulesBuilder::buildPrerequisiteModulesFor(...)`.
+///
+/// Users can detect whether the PrerequisiteModules is still up to date by
+/// calling the `canReuse()` member function.
+///
+/// The users should call `adjustHeaderSearchOptions(...)` to update the
+/// compilation commands to select the built module files first. Before calling
+/// `adjustHeaderSearchOptions()`, users should call `canReuse()` first to
check
+/// if all the stored module files are valid. In case they are not valid,
+/// users should call `ModulesBuilder::buildPrerequisiteModulesFor(...)` again
+/// to get the new PrerequisiteModules.
+class PrerequisiteModules {
+public:
+ /// Change commands to load the module files recorded in this
+ /// PrerequisiteModules first.
+ virtual void
+ adjustHeaderSearchOptions(HeaderSearchOptions ) const = 0;
+
+ /// Whether or not the built module files are up to date.
+ /// Note that this can only be used after building the module files.
+ virtual bool
+ canReuse(const CompilerInvocation ,
+ llvm::IntrusiveRefCntPtr) const = 0;
+
+ /// Return true if the modile file specified by ModuleName is built.
+ /// Note that this interface will only check the existence of the module
+ /// file instead of checking the validness of the module file.
+ virtual bool isModuleUnitBuilt(llvm::StringRef ModuleName) const = 0;
ChuanqiXu9 wrote:
Done
https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,370 @@
+//===- ModulesBuilder.cpp *-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ModulesBuilder.h"
+
+#include "Compiler.h"
+#include "support/Logger.h"
+
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/FrontendActions.h"
+
+#include "clang/Serialization/ASTReader.h"
+
+namespace clang {
+namespace clangd {
+
+namespace {
+
+// Create a path to store module files. Generally it should be:
+//
+// {TEMP_DIRS}/clangd/module_files/{PrefixDir}-%%-%%-%%-%%-%%-%%/.
+//
+// {TEMP_DIRS} is the temporary directory for the system, e.g., "/var/tmp"
+// or "C:/TEMP".
+//
+// '%%' means random value to make the generated path unique.
+//
+// \param MainFile is used to get the root of the project from global
+// compilation database. \param PrefixDir is used to get the user
+// defined prefix for module files. This is useful when we want to seperate
+// module files. e.g., we want to build module files for the same module unit
+// `a.cppm` with 2 different users `b.cpp` and `c.cpp` and we don't want the
+// module file for `b.cpp` be conflict with the module files for `c.cpp`. Then
+// we can put the 2 module files into different dirs like:
+//
+// ${TEMP_DIRS}/clangd/module_files/b.cpp/a.pcm
+// ${TEMP_DIRS}/clangd/module_files/c.cpp/a.pcm
+//
+// TODO: Move these module fils out of the temporary directory if the module
+// files are persistent.
+llvm::SmallString<256> getUniqueModuleFilesPath(PathRef MainFile,
+llvm::StringRef PrefixDir) {
+ llvm::SmallString<256> ResultPattern;
+
+ llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/true,
+ ResultPattern);
+
+ llvm::sys::path::append(ResultPattern, "clangd");
+ llvm::sys::path::append(ResultPattern, "module_files");
+
+ llvm::sys::path::append(ResultPattern, PrefixDir);
+
+ ResultPattern.append("-%%-%%-%%-%%-%%-%%");
+
+ llvm::SmallString<256> Result;
+ llvm::sys::fs::createUniquePath(ResultPattern, Result,
+ /*MakeAbsolute=*/false);
+
+ llvm::sys::fs::create_directories(Result);
+ return Result;
+}
+
+// Get the absolute path for the filename from the compile command.
+llvm::SmallString<128> getAbsolutePath(const tooling::CompileCommand ) {
+ llvm::SmallString<128> AbsolutePath;
+ if (llvm::sys::path::is_absolute(Cmd.Filename)) {
+AbsolutePath = Cmd.Filename;
+ } else {
+AbsolutePath = Cmd.Directory;
+llvm::sys::path::append(AbsolutePath, Cmd.Filename);
+llvm::sys::path::remove_dots(AbsolutePath, true);
+ }
+ return AbsolutePath;
+}
+
+// Get a unique module file path under \param ModuleFilesPrefix.
+std::string getModuleFilePath(llvm::StringRef ModuleName,
+ PathRef ModuleFilesPrefix) {
+ llvm::SmallString<256> ModuleFilePattern(ModuleFilesPrefix);
+ auto [PrimaryModuleName, PartitionName] = ModuleName.split(':');
+ llvm::sys::path::append(ModuleFilePattern, PrimaryModuleName);
+ if (!PartitionName.empty()) {
+ModuleFilePattern.append("-");
+ModuleFilePattern.append(PartitionName);
+ }
+
+ ModuleFilePattern.append(".pcm");
+
+ llvm::SmallString<256> ModuleFilePath;
+ llvm::sys::fs::createUniquePath(ModuleFilePattern, ModuleFilePath,
+ /*MakeAbsolute=*/false);
+
+ return std::string(ModuleFilePath);
+}
+} // namespace
+
+// FailedPrerequisiteModules - stands for the PrerequisiteModules which has
+// errors happened during the building process.
+class FailedPrerequisiteModules : public PrerequisiteModules {
+public:
+ ~FailedPrerequisiteModules() override = default;
+
+ // We shouldn't adjust the compilation commands based on
+ // FailedPrerequisiteModules.
+ void adjustHeaderSearchOptions(HeaderSearchOptions ) const override {
+ }
+
+ // FailedPrerequisiteModules can never be reused.
+ bool
+ canReuse(const CompilerInvocation ,
+ llvm::IntrusiveRefCntPtr) const override {
+return false;
+ }
+
+ // No module unit got built in FailedPrerequisiteModules.
+ bool isModuleUnitBuilt(llvm::StringRef ModuleName) const override {
+return false;
+ }
+};
+
+// StandalonePrerequisiteModules - stands for PrerequisiteModules for which all
+// the required modules are built successfully. All the module files
+// are owned by the StandalonePrerequisiteModules class.
+//
+// Any of the built module files won't be shared with other instances of the
+// class. So that we can avoid worrying thread safety.
+//
+// We don't need to worry about duplicated module names here since the standard
+// guarantees the module names should be unique to a program.
+class
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,370 @@
+//===- ModulesBuilder.cpp *-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ModulesBuilder.h"
+
+#include "Compiler.h"
+#include "support/Logger.h"
+
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/FrontendActions.h"
+
+#include "clang/Serialization/ASTReader.h"
+
+namespace clang {
+namespace clangd {
+
+namespace {
+
+// Create a path to store module files. Generally it should be:
+//
+// {TEMP_DIRS}/clangd/module_files/{PrefixDir}-%%-%%-%%-%%-%%-%%/.
+//
+// {TEMP_DIRS} is the temporary directory for the system, e.g., "/var/tmp"
+// or "C:/TEMP".
+//
+// '%%' means random value to make the generated path unique.
+//
+// \param MainFile is used to get the root of the project from global
+// compilation database. \param PrefixDir is used to get the user
+// defined prefix for module files. This is useful when we want to seperate
+// module files. e.g., we want to build module files for the same module unit
+// `a.cppm` with 2 different users `b.cpp` and `c.cpp` and we don't want the
+// module file for `b.cpp` be conflict with the module files for `c.cpp`. Then
+// we can put the 2 module files into different dirs like:
+//
+// ${TEMP_DIRS}/clangd/module_files/b.cpp/a.pcm
+// ${TEMP_DIRS}/clangd/module_files/c.cpp/a.pcm
+//
+// TODO: Move these module fils out of the temporary directory if the module
+// files are persistent.
+llvm::SmallString<256> getUniqueModuleFilesPath(PathRef MainFile,
+llvm::StringRef PrefixDir) {
+ llvm::SmallString<256> ResultPattern;
+
+ llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/true,
+ ResultPattern);
+
+ llvm::sys::path::append(ResultPattern, "clangd");
+ llvm::sys::path::append(ResultPattern, "module_files");
+
+ llvm::sys::path::append(ResultPattern, PrefixDir);
+
+ ResultPattern.append("-%%-%%-%%-%%-%%-%%");
+
+ llvm::SmallString<256> Result;
+ llvm::sys::fs::createUniquePath(ResultPattern, Result,
+ /*MakeAbsolute=*/false);
+
+ llvm::sys::fs::create_directories(Result);
+ return Result;
+}
+
+// Get the absolute path for the filename from the compile command.
+llvm::SmallString<128> getAbsolutePath(const tooling::CompileCommand ) {
+ llvm::SmallString<128> AbsolutePath;
+ if (llvm::sys::path::is_absolute(Cmd.Filename)) {
+AbsolutePath = Cmd.Filename;
+ } else {
+AbsolutePath = Cmd.Directory;
+llvm::sys::path::append(AbsolutePath, Cmd.Filename);
+llvm::sys::path::remove_dots(AbsolutePath, true);
+ }
+ return AbsolutePath;
+}
+
+// Get a unique module file path under \param ModuleFilesPrefix.
+std::string getModuleFilePath(llvm::StringRef ModuleName,
+ PathRef ModuleFilesPrefix) {
+ llvm::SmallString<256> ModuleFilePattern(ModuleFilesPrefix);
+ auto [PrimaryModuleName, PartitionName] = ModuleName.split(':');
+ llvm::sys::path::append(ModuleFilePattern, PrimaryModuleName);
+ if (!PartitionName.empty()) {
+ModuleFilePattern.append("-");
+ModuleFilePattern.append(PartitionName);
+ }
+
+ ModuleFilePattern.append(".pcm");
+
+ llvm::SmallString<256> ModuleFilePath;
+ llvm::sys::fs::createUniquePath(ModuleFilePattern, ModuleFilePath,
+ /*MakeAbsolute=*/false);
+
+ return std::string(ModuleFilePath);
+}
+} // namespace
+
+// FailedPrerequisiteModules - stands for the PrerequisiteModules which has
+// errors happened during the building process.
+class FailedPrerequisiteModules : public PrerequisiteModules {
+public:
+ ~FailedPrerequisiteModules() override = default;
+
+ // We shouldn't adjust the compilation commands based on
+ // FailedPrerequisiteModules.
+ void adjustHeaderSearchOptions(HeaderSearchOptions ) const override {
+ }
+
+ // FailedPrerequisiteModules can never be reused.
+ bool
+ canReuse(const CompilerInvocation ,
+ llvm::IntrusiveRefCntPtr) const override {
+return false;
+ }
+
+ // No module unit got built in FailedPrerequisiteModules.
+ bool isModuleUnitBuilt(llvm::StringRef ModuleName) const override {
+return false;
+ }
+};
+
+// StandalonePrerequisiteModules - stands for PrerequisiteModules for which all
+// the required modules are built successfully. All the module files
+// are owned by the StandalonePrerequisiteModules class.
+//
+// Any of the built module files won't be shared with other instances of the
+// class. So that we can avoid worrying thread safety.
+//
+// We don't need to worry about duplicated module names here since the standard
+// guarantees the module names should be unique to a program.
+class
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,115 @@
+//===- ModulesBuilder.h --*-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// Experimental support for C++20 Modules.
+//
+// Currently we simplify the implementations by preventing reusing module files
+// across different versions and different source files. But this is clearly a
+// waste of time and space in the end of the day.
+//
+// TODO: Supporting reusing module files across different versions and
+// different source files.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULES_BUILDER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULES_BUILDER_H
+
+#include "GlobalCompilationDatabase.h"
+#include "ProjectModules.h"
+
+#include "support/Path.h"
+#include "support/ThreadsafeFS.h"
+
+#include "clang/Frontend/CompilerInvocation.h"
+
+#include "llvm/ADT/SmallString.h"
+
+#include
+
+namespace clang {
+namespace clangd {
+
+/// Store all the needed module files information to parse a single
+/// source file. e.g.,
+///
+/// ```
+/// // a.cppm
+/// export module a;
+///
+/// // b.cppm
+/// export module b;
+/// import a;
+///
+/// // c.cppm
+/// export module c;
+/// import a;
+/// ```
+///
+/// For the source file `c.cppm`, an instance of the class will store
+/// the module files for `a.cppm` and `b.cppm`. But the module file for
`c.cppm`
+/// won't be stored. Since it is not needed to parse `c.cppm`.
+///
+/// Users should only get PrerequisiteModules from
+/// `ModulesBuilder::buildPrerequisiteModulesFor(...)`.
+///
+/// Users can detect whether the PrerequisiteModules is still up to date by
+/// calling the `canReuse()` member function.
+///
+/// The users should call `adjustHeaderSearchOptions(...)` to update the
+/// compilation commands to select the built module files first. Before calling
+/// `adjustHeaderSearchOptions()`, users should call `canReuse()` first to
check
+/// if all the stored module files are valid. In case they are not valid,
+/// users should call `ModulesBuilder::buildPrerequisiteModulesFor(...)` again
+/// to get the new PrerequisiteModules.
+class PrerequisiteModules {
+public:
+ /// Change commands to load the module files recorded in this
+ /// PrerequisiteModules first.
+ virtual void
+ adjustHeaderSearchOptions(HeaderSearchOptions ) const = 0;
+
+ /// Whether or not the built module files are up to date.
+ /// Note that this can only be used after building the module files.
+ virtual bool
+ canReuse(const CompilerInvocation ,
+ llvm::IntrusiveRefCntPtr) const = 0;
+
+ /// Return true if the modile file specified by ModuleName is built.
+ /// Note that this interface will only check the existence of the module
+ /// file instead of checking the validness of the module file.
+ virtual bool isModuleUnitBuilt(llvm::StringRef ModuleName) const = 0;
+
+ virtual ~PrerequisiteModules() = default;
+};
+
+/// This class handles building module files for a given source file.
+///
+/// In the future, we want the class to manage the module files acorss
+/// different versions and different source files.
+class ModulesBuilder {
+public:
+ ModulesBuilder(const GlobalCompilationDatabase ) : CDB(CDB) {}
+
+ ModulesBuilder(const ModulesBuilder &) = delete;
+ ModulesBuilder(ModulesBuilder &&) = delete;
+
+ ModulesBuilder =(const ModulesBuilder &) = delete;
+ ModulesBuilder =(ModulesBuilder &&) = delete;
+
+ std::unique_ptr
+ buildPrerequisiteModulesFor(PathRef File, const ThreadsafeFS *TFS) const;
ChuanqiXu9 wrote:
Done
https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -208,15 +208,16 @@ ClangdServer::Options::operator TUScheduler::Options()
const {
Opts.UpdateDebounce = UpdateDebounce;
Opts.ContextProvider = ContextProvider;
Opts.PreambleThrottler = PreambleThrottler;
+ Opts.ExperimentalModulesSupport = ExperimentalModulesSupport;
ChuanqiXu9 wrote:
Done
https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -44,6 +44,8 @@ struct ParseOptions {
bool ImportInsertions = false;
};
+class ModulesBuilder;
ChuanqiXu9 wrote:
Done
https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -192,8 +192,10 @@ TEST(PreamblePatchTest, PatchesPreambleIncludes) {
TU.AdditionalFiles["b.h"] = "";
TU.AdditionalFiles["c.h"] = "";
auto PI = TU.inputs(FS);
- auto BaselinePreamble = buildPreamble(
- TU.Filename, *buildCompilerInvocation(PI, Diags), PI, true, nullptr);
+ MockCompilationDatabase CDB;
+ auto BaselinePreamble =
+ buildPreamble(TU.Filename, *buildCompilerInvocation(PI, Diags), PI, true,
+/*RequiredModuleBuilder=*/nullptr, nullptr);
ChuanqiXu9 wrote:
Agreed. Done by adding an end-to-end test in the end of
`PrerequisiteModulesTest.cpp`.
https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
ChuanqiXu9 wrote: Done https://github.com/llvm/llvm-project/pull/66462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -149,9 +154,13 @@ struct PreambleBuildStats {
/// If \p PreambleCallback is set, it will be run on top of the AST while
/// building the preamble.
/// If Stats is not non-null, build statistics will be exported there.
+/// If \p RequiredModuleBuilder is not null, it will scan the source file
+/// to see if it is related to modules, and if yes, modules related things
+/// will be built.
std::shared_ptr
buildPreamble(PathRef FileName, CompilerInvocation CI,
const ParseInputs , bool StoreInMemory,
+ ModulesBuilder *RequiredModuleBuilder,
ChuanqiXu9 wrote:
Done
https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,339 @@
+//===- ModulesBuilder.cpp *-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ModulesBuilder.h"
+#include "PrerequisiteModules.h"
+#include "support/Logger.h"
+
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/FrontendActions.h"
+
+namespace clang {
+namespace clangd {
+
+namespace {
+
+/// Get or create a path to store module files. Generally it should be:
+///
+/// project_root/.cache/clangd/module_files/{RequiredPrefixDir}/.
+///
+/// \param MainFile is used to get the root of the project from global
+/// compilation database. \param RequiredPrefixDir is used to get the user
+/// defined prefix for module files. This is useful when we want to seperate
+/// module files. e.g., we want to build module files for the same module unit
+/// `a.cppm` with 2 different users `b.cpp` and `c.cpp` and we don't want the
+/// module file for `b.cpp` be conflict with the module files for `c.cpp`. Then
+/// we can put the 2 module files into different dirs like:
+///
+/// project_root/.cache/clangd/module_files/b.cpp/a.pcm
+/// project_root/.cache/clangd/module_files/c.cpp/a.pcm
+llvm::SmallString<256> getModuleFilesPath(PathRef MainFile,
+ const GlobalCompilationDatabase ,
+ StringRef RequiredPrefixDir) {
+ std::optional PI = CDB.getProjectInfo(MainFile);
+ if (!PI)
+return {};
+
+ // FIXME: PI->SourceRoot may be empty, depending on the CDB strategy.
+ llvm::SmallString<256> Result(PI->SourceRoot);
+
+ llvm::sys::path::append(Result, ".cache");
+ llvm::sys::path::append(Result, "clangd");
+ llvm::sys::path::append(Result, "module_files");
+
+ llvm::sys::path::append(Result, RequiredPrefixDir);
+
+ llvm::sys::fs::create_directories(Result, /*IgnoreExisting=*/true);
+
+ return Result;
+}
+
+/// Get the absolute path for the filename from the compile command.
+llvm::SmallString<128> getAbsolutePath(const tooling::CompileCommand ) {
+ llvm::SmallString<128> AbsolutePath;
+ if (llvm::sys::path::is_absolute(Cmd.Filename)) {
+AbsolutePath = Cmd.Filename;
+ } else {
+AbsolutePath = Cmd.Directory;
+llvm::sys::path::append(AbsolutePath, Cmd.Filename);
+llvm::sys::path::remove_dots(AbsolutePath, true);
+ }
+ return AbsolutePath;
+}
+
+/// Get a unique module file path under \param ModuleFilesPrefix.
+std::string getUniqueModuleFilePath(StringRef ModuleName,
+PathRef ModuleFilesPrefix) {
+ llvm::SmallString<256> ModuleFilePattern(ModuleFilesPrefix);
+ auto [PrimaryModuleName, PartitionName] = ModuleName.split(':');
+ llvm::sys::path::append(ModuleFilePattern, PrimaryModuleName);
+ if (!PartitionName.empty()) {
+ModuleFilePattern.append("-");
+ModuleFilePattern.append(PartitionName);
+ }
+
+ ModuleFilePattern.append("-%%-%%-%%-%%-%%-%%");
+ ModuleFilePattern.append(".pcm");
+
+ llvm::SmallString<256> ModuleFilePath;
+ llvm::sys::fs::createUniquePath(ModuleFilePattern, ModuleFilePath,
+ /*MakeAbsolute=*/false);
+
+ return (std::string)ModuleFilePath;
+}
+} // namespace
+
+bool ModulesBuilder::buildModuleFile(StringRef ModuleName,
+ const ThreadsafeFS *TFS,
+ std::shared_ptr MDB,
+ PathRef ModuleFilesPrefix,
+ PrerequisiteModules ) {
+ if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName))
+return true;
+
+ PathRef ModuleUnitFileName = MDB->getSourceForModuleName(ModuleName);
+ /// It is possible that we're meeting third party modules (modules whose
+ /// source are not in the project. e.g, the std module may be a third-party
+ /// module for most project) or something wrong with the implementation of
+ /// ProjectModules.
+ /// FIXME: How should we treat third party modules here? If we want to ignore
+ /// third party modules, we should return true instead of false here.
+ /// Currently we simply bail out.
+ if (ModuleUnitFileName.empty())
+return false;
+
+ for (auto : MDB->getRequiredModules(ModuleUnitFileName))
{
+// Return early if there are errors building the module file.
+if (!buildModuleFile(RequiredModuleName, TFS, MDB, ModuleFilesPrefix,
+ BuiltModuleFiles)) {
+ log("Failed to build module {0}", RequiredModuleName);
+ return false;
+}
+ }
+
+ auto Cmd = CDB.getCompileCommand(ModuleUnitFileName);
+ if (!Cmd)
+return false;
+
+ std::string ModuleFileName =
+ getUniqueModuleFilePath(ModuleName, ModuleFilesPrefix);
+ Cmd->Output =
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
ChuanqiXu9 wrote: Done https://github.com/llvm/llvm-project/pull/66462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -42,6 +42,8 @@
namespace clang {
namespace clangd {
+
+class ModulesBuilder;
ChuanqiXu9 wrote:
Done
https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -222,6 +222,9 @@ class TUScheduler {
/// Cache (large) preamble data in RAM rather than temporary files on disk.
bool StorePreamblesInMemory = false;
+/// Enable experimental support for modules.
+bool ExperimentalModulesSupport = false;
ChuanqiXu9 wrote:
Done
https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -740,6 +741,21 @@
DirectoryBasedGlobalCompilationDatabase::getProjectInfo(PathRef File) const {
return Res->PI;
}
+std::shared_ptr
+DirectoryBasedGlobalCompilationDatabase::getProjectModules(PathRef File) const
{
+ CDBLookupRequest Req;
+ Req.FileName = File;
+ Req.ShouldBroadcast = false;
+ Req.FreshTime = Req.FreshTimeMissing =
+ std::chrono::steady_clock::time_point::min();
+ auto Res = lookupCDB(Req);
+ if (!Res)
+return {};
+ return ProjectModules::create(
+ ProjectModules::ProjectModulesKind::ScanningAllFiles,
+ Res->CDB->getAllFiles(), *this, Opts.TFS);
ChuanqiXu9 wrote:
Done by following other comments.
https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,81 @@
+//=== ModuleDependencyScanner.cpp *-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ModuleDependencyScanner.h"
+#include "support/Logger.h"
+
+namespace clang {
+namespace clangd {
+
+std::optional
+ModuleDependencyScanner::scan(PathRef FilePath) {
+ std::optional Cmd = CDB.getCompileCommand(FilePath);
+
+ if (!Cmd)
+return std::nullopt;
+
+ using namespace clang::tooling::dependencies;
+
+ llvm::SmallString<128> FilePathDir(FilePath);
+ llvm::sys::path::remove_filename(FilePathDir);
+ DependencyScanningTool ScanningTool(Service, TFS.view(FilePathDir));
+
+ llvm::Expected ScanningResult =
+ ScanningTool.getP1689ModuleDependencyFile(*Cmd, Cmd->Directory);
+
+ if (auto E = ScanningResult.takeError()) {
+log("Scanning modules dependencies for {0} failed: {1}", FilePath,
+llvm::toString(std::move(E)));
+return std::nullopt;
+ }
+
+ ModuleDependencyInfo Result;
+
+ if (ScanningResult->Provides) {
+ModuleNameToSource[ScanningResult->Provides->ModuleName] = FilePath;
ChuanqiXu9 wrote:
> Are we really going into all this complexity to optimize the case of files
> with no modular dependencies?
Yes, this is primarily the reason why we delay it.
> files with no modular dependencies is going to be so rare in practice
This assumption is not true to me unless we're talking about the ecosystem 10+
years later. (There is a website (https://arewemodulesyet.org/) tracking the
progress. Although its progress bar is almost a joke).
Since programmers needs to refactor their code to use C++20 modules, we believe
there will still be a lot of files unrelated to modules exists. Especially for
headers.
To make this more clear, currently, there are 2 ways to use modules. One is to
use modules extensively and get rid of headers. Examples are
https://github.com/infiniflow/infinity and
https://github.com/davidstone/technical-machine, where we can see rare headers
and almost all the files are related to modules.
However, there are more projects are simply wrapped themselves with modules, so
that the users can consume the library by including or by importing by their
interest. (Or even importing in some .cc and including in some other .cc if the
size of the code bases it too big). Most projects on the previous mentioned
website uses this model. From the perspective, I think the assumption that
modules-native (I called it) style won't take the place all over the world soon.
> so having this extra mental load and requirements about an implementation
> detail that doesn't bring much benefits in practice, and planned to be
> stripped away is just too much for maintenance.
Of course, since the first patch is not focus on performance and the design, I
can drop it if you really don't like it.
https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)
@@ -1185,6 +1190,21 @@ bool CodeGenVTables::isVTableExternal(const
CXXRecordDecl *RD) {
TSK == TSK_ExplicitInstantiationDefinition)
return false;
+ // Itanium C++ ABI [5.2.3]:
+ // Virtual tables for dynamic classes are emitted as follows:
+ //
+ // - If the class is templated, the tables are emitted in every object that
+ // references any of them.
+ // - Otherwise, if the class is attached to a module, the tables are uniquely
+ // emitted in the object for the module unit in which it is defined.
+ // - Otherwise, if the class has a key function (see below), the tables are
+ // emitted in the object for the translation unit containing the definition
of
+ // the key function. This is unique if the key function is not inline.
+ // - Otherwise, the tables are emitted in every object that references any of
+ // them.
+ if (RD->isInNamedModule())
+return RD->shouldEmitInExternalSource();
ChuanqiXu9 wrote:
I tried. But it can't work the case we compile the AST directly to the object
file. In this case, `hasExternalDefinitions` returns EK_ReplyHazy, but we need
to return false directly here. I think it looks good now. If we want to extend
this to header modules with object files, it will be easy.
https://github.com/llvm/llvm-project/pull/75912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)
@@ -3239,6 +3239,12 @@ bool ASTReader::isConsumerInterestedIn(Decl *D) {
if (ES->hasExternalDefinitions(D) == ExternalASTSource::EK_Never)
return true;
+ // The dynamic class defined in a named module is interesting.
+ // The code generator needs to emit its vtable there.
+ if (const auto *Class = dyn_cast(D))
+return Class->isInCurrentModuleUnit() &&
+ Class->getDefinition() && Class->isDynamicClass();
+
ChuanqiXu9 wrote:
On, nice catch. I didn't notice this. I've removed it.
https://github.com/llvm/llvm-project/pull/75912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)
https://github.com/ChuanqiXu9 updated
https://github.com/llvm/llvm-project/pull/75912
>From cf8be3c418dde67b74d4a5a4ea98a33f0e2fbd72 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Tue, 19 Dec 2023 17:00:59 +0800
Subject: [PATCH 1/2] [C++20] [Modules] [Itanium ABI] Generate the vtable in
the module unit of dynamic classes
Close https://github.com/llvm/llvm-project/issues/70585 and reflect
https://github.com/itanium-cxx-abi/cxx-abi/issues/170.
The significant change of the patch is: for dynamic classes attached to
module units, we generate the vtable to the attached module units
directly and the key functions for such classes is meaningless.
---
clang/include/clang/AST/DeclBase.h| 3 ++
clang/lib/AST/DeclBase.cpp| 9 +
clang/lib/CodeGen/CGVTables.cpp | 28 ++
clang/lib/CodeGen/CodeGenModule.cpp | 7
clang/lib/CodeGen/ItaniumCXXABI.cpp | 3 ++
clang/lib/Sema/SemaDecl.cpp | 9 +
clang/lib/Sema/SemaDeclCXX.cpp| 12 --
clang/lib/Serialization/ASTReaderDecl.cpp | 6 +++
clang/lib/Serialization/ASTWriterDecl.cpp | 6 +++
clang/test/CodeGenCXX/modules-vtable.cppm | 31 +--
clang/test/CodeGenCXX/pr70585.cppm| 47 +++
11 files changed, 138 insertions(+), 23 deletions(-)
create mode 100644 clang/test/CodeGenCXX/pr70585.cppm
diff --git a/clang/include/clang/AST/DeclBase.h
b/clang/include/clang/AST/DeclBase.h
index 600ce73c7f019..f38386381853b 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -670,6 +670,9 @@ class alignas(8) Decl {
/// Whether this declaration comes from another module unit.
bool isInAnotherModuleUnit() const;
+ /// Whether this declaration comes from the same module unit being compiled.
+ bool isInCurrentModuleUnit() const;
+
/// Whether the definition of the declaration should be emitted in external
/// sources.
bool shouldEmitInExternalSource() const;
diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index 1e9c879e371bc..153dc3351dae5 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -1106,6 +1106,15 @@ bool Decl::isInAnotherModuleUnit() const {
return M != getASTContext().getCurrentNamedModule();
}
+bool Decl::isInCurrentModuleUnit() const {
+ auto *M = getOwningModule();
+
+ if (!M || !M->isNamedModule())
+return false;
+
+ return M == getASTContext().getCurrentNamedModule();
+}
+
bool Decl::shouldEmitInExternalSource() const {
ExternalASTSource *Source = getASTContext().getExternalSource();
if (!Source)
diff --git a/clang/lib/CodeGen/CGVTables.cpp b/clang/lib/CodeGen/CGVTables.cpp
index 001633453f242..55c3032dc9332 100644
--- a/clang/lib/CodeGen/CGVTables.cpp
+++ b/clang/lib/CodeGen/CGVTables.cpp
@@ -1051,6 +1051,11 @@ CodeGenModule::getVTableLinkage(const CXXRecordDecl *RD)
{
if (!RD->isExternallyVisible())
return llvm::GlobalVariable::InternalLinkage;
+ // V-tables for non-template classes with an owning module are always
+ // uniquely emitted in that module.
+ if (RD->isInNamedModule())
+return llvm::GlobalVariable::ExternalLinkage;
+
// We're at the end of the translation unit, so the current key
// function is fully correct.
const CXXMethodDecl *keyFunction = Context.getCurrentKeyFunction(RD);
@@ -1185,6 +1190,21 @@ bool CodeGenVTables::isVTableExternal(const
CXXRecordDecl *RD) {
TSK == TSK_ExplicitInstantiationDefinition)
return false;
+ // Itanium C++ ABI [5.2.3]:
+ // Virtual tables for dynamic classes are emitted as follows:
+ //
+ // - If the class is templated, the tables are emitted in every object that
+ // references any of them.
+ // - Otherwise, if the class is attached to a module, the tables are uniquely
+ // emitted in the object for the module unit in which it is defined.
+ // - Otherwise, if the class has a key function (see below), the tables are
+ // emitted in the object for the translation unit containing the definition
of
+ // the key function. This is unique if the key function is not inline.
+ // - Otherwise, the tables are emitted in every object that references any of
+ // them.
+ if (RD->isInNamedModule())
+return RD->shouldEmitInExternalSource();
+
// Otherwise, if the class doesn't have a key function (possibly
// anymore), the vtable must be defined here.
const CXXMethodDecl *keyFunction =
CGM.getContext().getCurrentKeyFunction(RD);
@@ -1194,13 +1214,7 @@ bool CodeGenVTables::isVTableExternal(const
CXXRecordDecl *RD) {
const FunctionDecl *Def;
// Otherwise, if we don't have a definition of the key function, the
// vtable must be defined somewhere else.
- if (!keyFunction->hasBody(Def))
-return true;
-
- assert(Def && "The body of the key function is not assigned to Def?");
- // If the non-inline key function comes from another module unit, the vtable
- // must be defined there.
- return
[clang] [serialization] no transitive decl change (PR #92083)
ChuanqiXu9 wrote: I've fixed the lldb failure and resent https://github.com/llvm/llvm-project/commit/97c866f6c86456b3316006e6beff47e68a81c00a. https://github.com/llvm/llvm-project/pull/92083 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 97c866f - [serialization] no transitive decl change (#92083)
Author: Chuanqi Xu
Date: 2024-06-06T11:51:05+08:00
New Revision: 97c866f6c86456b3316006e6beff47e68a81c00a
URL:
https://github.com/llvm/llvm-project/commit/97c866f6c86456b3316006e6beff47e68a81c00a
DIFF:
https://github.com/llvm/llvm-project/commit/97c866f6c86456b3316006e6beff47e68a81c00a.diff
LOG: [serialization] no transitive decl change (#92083)
Following of https://github.com/llvm/llvm-project/pull/86912
The motivation of the patch series is that, for a module interface unit
`X`, when the dependent modules of `X` changes, if the changes is not
relevant with `X`, we hope the BMI of `X` won't change. For the specific
patch, we hope if the changes was about irrelevant declaration changes,
we hope the BMI of `X` won't change. **However**, I found the patch
itself is not very useful in practice, since the adding or removing
declarations, will change the state of identifiers and types in most
cases.
That said, for the most simple example,
```
// partA.cppm
export module m:partA;
// partA.v1.cppm
export module m:partA;
export void a() {}
// partB.cppm
export module m:partB;
export void b() {}
// m.cppm
export module m;
export import :partA;
export import :partB;
// onlyUseB;
export module onlyUseB;
import m;
export inline void onluUseB() {
b();
}
```
the BMI of `onlyUseB` will change after we change the implementation of
`partA.cppm` to `partA.v1.cppm`. Since `partA.v1.cppm` introduces new
identifiers and types (the function prototype).
So in this patch, we have to write the tests as:
```
// partA.cppm
export module m:partA;
export int getA() { ... }
export int getA2(int) { ... }
// partA.v1.cppm
export module m:partA;
export int getA() { ... }
export int getA(int) { ... }
export int getA2(int) { ... }
// partB.cppm
export module m:partB;
export void b() {}
// m.cppm
export module m;
export import :partA;
export import :partB;
// onlyUseB;
export module onlyUseB;
import m;
export inline void onluUseB() {
b();
}
```
so that the new introduced declaration `int getA(int)` doesn't introduce
new identifiers and types, then the BMI of `onlyUseB` can keep
unchanged.
While it looks not so great, the patch should be the base of the patch
to erase the transitive change for identifiers and types since I don't
know how can we introduce new types and identifiers without introducing
new declarations. Given how tightly the relationship between
declarations, types and identifiers, I think we can only reach the ideal
state after we made the series for all of the three entties.
The design of the patch is similar to
https://github.com/llvm/llvm-project/pull/86912, which extends the
32-bit DeclID to 64-bit and use the higher bits to store the module file
index and the lower bits to store the Local Decl ID.
A slight difference is that we only use 48 bits to store the new DeclID
since we try to use the higher 16 bits to store the module ID in the
prefix of Decl class. Previously, we use 32 bits to store the module ID
and 32 bits to store the DeclID. I don't want to allocate additional
space so I tried to make the additional space the same as 64 bits. An
potential interesting thing here is about the relationship between the
module ID and the module file index. I feel we can get the module file
index by the module ID. But I didn't prove it or implement it. Since I
want to make the patch itself as small as possible. We can make it in
the future if we want.
Another change in the patch is the new concept Decl Index, which means
the index of the very big array `DeclsLoaded` in ASTReader. Previously,
the index of a loaded declaration is simply the Decl ID minus
PREDEFINED_DECL_NUMs. So there are some places they got used
ambiguously. But this patch tried to split these two concepts.
As https://github.com/llvm/llvm-project/pull/86912 did, the change will
increase the on-disk PCM file sizes. As the declaration ID may be the
most IDs in the PCM file, this can have the biggest impact on the size.
In my experiments, this change will bring 6.6% increase of the on-disk
PCM size. No compile-time performance regression observed. Given the
benefits in the motivation example, I think the cost is worthwhile.
Added:
clang/test/Modules/no-transitive-decls-change.cppm
Modified:
clang/include/clang/AST/DeclBase.h
clang/include/clang/AST/DeclID.h
clang/include/clang/Serialization/ASTBitCodes.h
clang/include/clang/Serialization/ASTReader.h
clang/include/clang/Serialization/ModuleFile.h
clang/include/clang/Serialization/ModuleManager.h
clang/lib/AST/DeclBase.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTReaderDecl.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/lib/Serialization/ModuleFile.cpp
Removed:
diff --git a/clang/include/clang/AST/DeclBase.h
b/clang/include/clang/AST/DeclBase.h
index 600ce73c7f019..5f19af1891b74 100644
---
[clang] [serialization] no transitive decl change (PR #92083)
ChuanqiXu9 wrote: > Maybe you can run individual tests with lit but generally I use `lldb-dotest` > instead: > > ``` > ./bin/lldb-dotest.py -p TestTemplateWithSameArg.py > ``` > > (you only need the filename) Thanks. Reproduced. But it surprised me that I can't run the commands you mentioned, but I need to run: ``` ./bin/lldb-dotest -p TestTemplateWithSameArg.py -G gmodules ``` And I am also slightly surprised that after I change the code, it doesn't work if I run `ninja lldb-test` only I need to run `ninja clang lldb`. Do I misconfigure anything? I build lldb by: ``` CC=clang CXX=clang++ cmake -DLLVM_ENABLE_PROJECTS="clang;lldb" -GNinja ../llvm -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_ASSERTIONS=On -DLLVM_ENABLE_LLD=ON -DPython3_ROOT_DIR= ``` https://github.com/llvm/llvm-project/pull/92083 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [serialization] no transitive decl change (PR #92083)
ChuanqiXu9 wrote:
> Unfortunately this is still failing one test on the LLDB macOS CI:
> https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake/5083/execution/node/97/log
>
> ```
> ==
> FAIL: test_duplicate_decls_gmodules
> (TestTemplateWithSameArg.TestTemplateWithSameArg)
> --
> Traceback (most recent call last):
> File
> "/Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py",
> line 1756, in test_method
> return attrvalue(self)
> File
> "/Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/llvm-project/lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py",
> line 66, in test_duplicate_decls
> self.filecheck("target module dump ast", __file__)
> File
> "/Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py",
> line 2293, in filecheck
> self.assertTrue(cmd_status == 0)
> AssertionError: False is not true
> Config=arm64-/Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/lldb-build/bin/clang
> --
> Ran 2 tests in 1.682s
>
> FAILED (failures=1)
> ```
>
> Let me know if you need help reproducing this.
>
> Looks like we're doing a FileCheck on the imported LLDB AST:
>
> ```
> FileCheck output:
>
> /Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/llvm-project/lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py:69:10:
> error: CHECK: expected string not found in input
> # CHECK: ClassTemplateSpecializationDecl {{.*}} imported in Module2 struct
> ClassInMod3 definition
> ```
>
> But instead of `imported in Module2 struct ClassInMod3 definition` it's now
> just `imported struct ClassInMod3 definition`. Maybe we're not setting the
> module ID correctly in LLDB or something? Does that ring any bells?
@Michael137 hi, how can I run test for it? I tried
```
bin/llvm-lit
../lldb/test/API/lang/cpp/gmodules/template-with-same-arg/TestTemplateWithSameArg.py
```
but it shows it can't work
https://github.com/llvm/llvm-project/pull/92083
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)
@@ -1180,6 +1185,21 @@ bool CodeGenVTables::isVTableExternal(const
CXXRecordDecl *RD) {
TSK == TSK_ExplicitInstantiationDefinition)
return false;
+ // Itanium C++ ABI [5.2.3]:
+ // Virtual tables for dynamic classes are emitted as follows:
+ //
+ // - If the class is templated, the tables are emitted in every object that
+ // references any of them.
+ // - Otherwise, if the class is attached to a module, the tables are uniquely
+ // emitted in the object for the module unit in which it is defined.
+ // - Otherwise, if the class has a key function (see below), the tables are
+ // emitted in the object for the translation unit containing the definition
of
+ // the key function. This is unique if the key function is not inline.
+ // - Otherwise, the tables are emitted in every object that references any of
+ // them.
+ if (Module *M = RD->getOwningModule(); M && M->isNamedModule())
ChuanqiXu9 wrote:
Done. I add the interfaces in precommit:
https://github.com/llvm/llvm-project/commit/99873b35da7ecb905143c8a6b8deca4d4416f1a9
https://github.com/llvm/llvm-project/pull/75912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)
https://github.com/ChuanqiXu9 updated
https://github.com/llvm/llvm-project/pull/75912
>From cf8be3c418dde67b74d4a5a4ea98a33f0e2fbd72 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Tue, 19 Dec 2023 17:00:59 +0800
Subject: [PATCH] [C++20] [Modules] [Itanium ABI] Generate the vtable in the
module unit of dynamic classes
Close https://github.com/llvm/llvm-project/issues/70585 and reflect
https://github.com/itanium-cxx-abi/cxx-abi/issues/170.
The significant change of the patch is: for dynamic classes attached to
module units, we generate the vtable to the attached module units
directly and the key functions for such classes is meaningless.
---
clang/include/clang/AST/DeclBase.h| 3 ++
clang/lib/AST/DeclBase.cpp| 9 +
clang/lib/CodeGen/CGVTables.cpp | 28 ++
clang/lib/CodeGen/CodeGenModule.cpp | 7
clang/lib/CodeGen/ItaniumCXXABI.cpp | 3 ++
clang/lib/Sema/SemaDecl.cpp | 9 +
clang/lib/Sema/SemaDeclCXX.cpp| 12 --
clang/lib/Serialization/ASTReaderDecl.cpp | 6 +++
clang/lib/Serialization/ASTWriterDecl.cpp | 6 +++
clang/test/CodeGenCXX/modules-vtable.cppm | 31 +--
clang/test/CodeGenCXX/pr70585.cppm| 47 +++
11 files changed, 138 insertions(+), 23 deletions(-)
create mode 100644 clang/test/CodeGenCXX/pr70585.cppm
diff --git a/clang/include/clang/AST/DeclBase.h
b/clang/include/clang/AST/DeclBase.h
index 600ce73c7f019..f38386381853b 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -670,6 +670,9 @@ class alignas(8) Decl {
/// Whether this declaration comes from another module unit.
bool isInAnotherModuleUnit() const;
+ /// Whether this declaration comes from the same module unit being compiled.
+ bool isInCurrentModuleUnit() const;
+
/// Whether the definition of the declaration should be emitted in external
/// sources.
bool shouldEmitInExternalSource() const;
diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index 1e9c879e371bc..153dc3351dae5 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -1106,6 +1106,15 @@ bool Decl::isInAnotherModuleUnit() const {
return M != getASTContext().getCurrentNamedModule();
}
+bool Decl::isInCurrentModuleUnit() const {
+ auto *M = getOwningModule();
+
+ if (!M || !M->isNamedModule())
+return false;
+
+ return M == getASTContext().getCurrentNamedModule();
+}
+
bool Decl::shouldEmitInExternalSource() const {
ExternalASTSource *Source = getASTContext().getExternalSource();
if (!Source)
diff --git a/clang/lib/CodeGen/CGVTables.cpp b/clang/lib/CodeGen/CGVTables.cpp
index 001633453f242..55c3032dc9332 100644
--- a/clang/lib/CodeGen/CGVTables.cpp
+++ b/clang/lib/CodeGen/CGVTables.cpp
@@ -1051,6 +1051,11 @@ CodeGenModule::getVTableLinkage(const CXXRecordDecl *RD)
{
if (!RD->isExternallyVisible())
return llvm::GlobalVariable::InternalLinkage;
+ // V-tables for non-template classes with an owning module are always
+ // uniquely emitted in that module.
+ if (RD->isInNamedModule())
+return llvm::GlobalVariable::ExternalLinkage;
+
// We're at the end of the translation unit, so the current key
// function is fully correct.
const CXXMethodDecl *keyFunction = Context.getCurrentKeyFunction(RD);
@@ -1185,6 +1190,21 @@ bool CodeGenVTables::isVTableExternal(const
CXXRecordDecl *RD) {
TSK == TSK_ExplicitInstantiationDefinition)
return false;
+ // Itanium C++ ABI [5.2.3]:
+ // Virtual tables for dynamic classes are emitted as follows:
+ //
+ // - If the class is templated, the tables are emitted in every object that
+ // references any of them.
+ // - Otherwise, if the class is attached to a module, the tables are uniquely
+ // emitted in the object for the module unit in which it is defined.
+ // - Otherwise, if the class has a key function (see below), the tables are
+ // emitted in the object for the translation unit containing the definition
of
+ // the key function. This is unique if the key function is not inline.
+ // - Otherwise, the tables are emitted in every object that references any of
+ // them.
+ if (RD->isInNamedModule())
+return RD->shouldEmitInExternalSource();
+
// Otherwise, if the class doesn't have a key function (possibly
// anymore), the vtable must be defined here.
const CXXMethodDecl *keyFunction =
CGM.getContext().getCurrentKeyFunction(RD);
@@ -1194,13 +1214,7 @@ bool CodeGenVTables::isVTableExternal(const
CXXRecordDecl *RD) {
const FunctionDecl *Def;
// Otherwise, if we don't have a definition of the key function, the
// vtable must be defined somewhere else.
- if (!keyFunction->hasBody(Def))
-return true;
-
- assert(Def && "The body of the key function is not assigned to Def?");
- // If the non-inline key function comes from another module unit, the vtable
- // must be defined there.
- return
[clang] 99873b3 - [NFC] [AST] Introduce Decl::isInAnotherModuleUnit and Decl::shouldEmitInExternalSource
Author: Chuanqi Xu
Date: 2024-06-04T17:08:21+08:00
New Revision: 99873b35da7ecb905143c8a6b8deca4d4416f1a9
URL:
https://github.com/llvm/llvm-project/commit/99873b35da7ecb905143c8a6b8deca4d4416f1a9
DIFF:
https://github.com/llvm/llvm-project/commit/99873b35da7ecb905143c8a6b8deca4d4416f1a9.diff
LOG: [NFC] [AST] Introduce Decl::isInAnotherModuleUnit and
Decl::shouldEmitInExternalSource
Motivated by the review process in
https://github.com/llvm/llvm-project/pull/75912. This can also help to
simplify the code slightly.
Added:
Modified:
clang/include/clang/AST/DeclBase.h
clang/lib/AST/ASTContext.cpp
clang/lib/AST/Decl.cpp
clang/lib/AST/DeclBase.cpp
clang/lib/CodeGen/CGVTables.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Serialization/ASTWriter.cpp
Removed:
diff --git a/clang/include/clang/AST/DeclBase.h
b/clang/include/clang/AST/DeclBase.h
index 3a311d4c55916..600ce73c7f019 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -670,6 +670,13 @@ class alignas(8) Decl {
/// Whether this declaration comes from another module unit.
bool isInAnotherModuleUnit() const;
+ /// Whether the definition of the declaration should be emitted in external
+ /// sources.
+ bool shouldEmitInExternalSource() const;
+
+ /// Whether this declaration comes from a named module;
+ bool isInNamedModule() const;
+
/// Whether this declaration comes from explicit global module.
bool isFromExplicitGlobalModule() const;
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 73d3b152c49f1..bf74e56a14799 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -12018,7 +12018,7 @@ bool ASTContext::DeclMustBeEmitted(const Decl *D) {
return false;
// Variables in other module units shouldn't be forced to be emitted.
- if (VD->isInAnotherModuleUnit())
+ if (VD->shouldEmitInExternalSource())
return false;
// Variables that can be needed in other TUs are required.
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 0a35ed536a6a7..1f19dadafa44e 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -1174,13 +1174,6 @@ Linkage NamedDecl::getLinkageInternal() const {
.getLinkage();
}
-/// Determine whether D is attached to a named module.
-static bool isInNamedModule(const NamedDecl *D) {
- if (auto *M = D->getOwningModule())
-return M->isNamedModule();
- return false;
-}
-
static bool isExportedFromModuleInterfaceUnit(const NamedDecl *D) {
// FIXME: Handle isModulePrivate.
switch (D->getModuleOwnershipKind()) {
@@ -1190,7 +1183,7 @@ static bool isExportedFromModuleInterfaceUnit(const
NamedDecl *D) {
return false;
case Decl::ModuleOwnershipKind::Visible:
case Decl::ModuleOwnershipKind::VisibleWhenImported:
-return isInNamedModule(D);
+return D->isInNamedModule();
}
llvm_unreachable("unexpected module ownership kind");
}
@@ -1208,7 +1201,7 @@ Linkage NamedDecl::getFormalLinkage() const {
// [basic.namespace.general]/p2
// A namespace is never attached to a named module and never has a name
with
// module linkage.
- if (isInNamedModule(this) && InternalLinkage == Linkage::External &&
+ if (isInNamedModule() && InternalLinkage == Linkage::External &&
!isExportedFromModuleInterfaceUnit(
cast(this->getCanonicalDecl())) &&
!isa(this))
diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index ffb22194bce52..1e9c879e371bc 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -1100,23 +1100,22 @@ bool Decl::isInExportDeclContext() const {
bool Decl::isInAnotherModuleUnit() const {
auto *M = getOwningModule();
- if (!M)
+ if (!M || !M->isNamedModule())
return false;
- M = M->getTopLevelModule();
- // FIXME: It is problematic if the header module lives in another module
- // unit. Consider to fix this by techniques like
- // ExternalASTSource::hasExternalDefinitions.
- if (M->isHeaderLikeModule())
-return false;
+ return M != getASTContext().getCurrentNamedModule();
+}
- // A global module without parent implies that we're parsing the global
- // module. So it can't be in another module unit.
- if (M->isGlobalModule())
+bool Decl::shouldEmitInExternalSource() const {
+ ExternalASTSource *Source = getASTContext().getExternalSource();
+ if (!Source)
return false;
- assert(M->isNamedModule() && "New module kind?");
- return M != getASTContext().getCurrentNamedModule();
+ return Source->hasExternalDefinitions(this) == ExternalASTSource::EK_Always;
+}
+
+bool Decl::isInNamedModule() const {
+ return getOwningModule() && getOwningModule()->isNamedModule();
}
bool Decl::isFromExplicitGlobalModule() const {
diff --git a/clang/lib/CodeGen/CGVTables.cpp b/clang/lib/CodeGen/CGVTables.cpp
index
[clang] cb60667 - Revert "[serialization] no transitive decl change (#92083)"
Author: Chuanqi Xu
Date: 2024-06-04T16:10:38+08:00
New Revision: cb60667b6e762aa172b6ad06332465d69f0fd803
URL:
https://github.com/llvm/llvm-project/commit/cb60667b6e762aa172b6ad06332465d69f0fd803
DIFF:
https://github.com/llvm/llvm-project/commit/cb60667b6e762aa172b6ad06332465d69f0fd803.diff
LOG: Revert "[serialization] no transitive decl change (#92083)"
This reverts commit d8ec452db016f359feeec28994f6560b30b49824.
This fails on LLDB macOS CI. See
https://github.com/llvm/llvm-project/pull/92083 for details.
Added:
Modified:
clang/include/clang/AST/DeclBase.h
clang/include/clang/AST/DeclID.h
clang/include/clang/Serialization/ASTBitCodes.h
clang/include/clang/Serialization/ASTReader.h
clang/include/clang/Serialization/ModuleFile.h
clang/include/clang/Serialization/ModuleManager.h
clang/lib/AST/DeclBase.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTReaderDecl.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/lib/Serialization/ModuleFile.cpp
Removed:
clang/test/Modules/no-transitive-decls-change.cppm
diff --git a/clang/include/clang/AST/DeclBase.h
b/clang/include/clang/AST/DeclBase.h
index c4eb053146d32..3a311d4c55916 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -701,7 +701,10 @@ class alignas(8) Decl {
/// Set the owning module ID. This may only be called for
/// deserialized Decls.
- void setOwningModuleID(unsigned ID);
+ void setOwningModuleID(unsigned ID) {
+assert(isFromASTFile() && "Only works on a deserialized declaration");
+*((unsigned*)this - 2) = ID;
+ }
public:
/// Determine the availability of the given declaration.
@@ -774,11 +777,19 @@ class alignas(8) Decl {
/// Retrieve the global declaration ID associated with this
/// declaration, which specifies where this Decl was loaded from.
- GlobalDeclID getGlobalID() const;
+ GlobalDeclID getGlobalID() const {
+if (isFromASTFile())
+ return (*((const GlobalDeclID *)this - 1));
+return GlobalDeclID();
+ }
/// Retrieve the global ID of the module that owns this particular
/// declaration.
- unsigned getOwningModuleID() const;
+ unsigned getOwningModuleID() const {
+if (isFromASTFile())
+ return *((const unsigned*)this - 2);
+return 0;
+ }
private:
Module *getOwningModuleSlow() const;
diff --git a/clang/include/clang/AST/DeclID.h
b/clang/include/clang/AST/DeclID.h
index 32d2ed41a374a..614ba06b63860 100644
--- a/clang/include/clang/AST/DeclID.h
+++ b/clang/include/clang/AST/DeclID.h
@@ -19,8 +19,6 @@
#include "llvm/ADT/DenseMapInfo.h"
#include "llvm/ADT/iterator.h"
-#include
-
namespace clang {
/// Predefined declaration IDs.
@@ -109,16 +107,12 @@ class DeclIDBase {
///
/// DeclID should only be used directly in serialization. All other users
/// should use LocalDeclID or GlobalDeclID.
- using DeclID = uint64_t;
+ using DeclID = uint32_t;
protected:
DeclIDBase() : ID(PREDEF_DECL_NULL_ID) {}
explicit DeclIDBase(DeclID ID) : ID(ID) {}
- explicit DeclIDBase(unsigned LocalID, unsigned ModuleFileIndex) {
-ID = (DeclID)LocalID | ((DeclID)ModuleFileIndex << 32);
- }
-
public:
DeclID get() const { return ID; }
@@ -130,10 +124,6 @@ class DeclIDBase {
bool isInvalid() const { return ID == PREDEF_DECL_NULL_ID; }
- unsigned getModuleFileIndex() const { return ID >> 32; }
-
- unsigned getLocalDeclIndex() const;
-
friend bool operator==(const DeclIDBase , const DeclIDBase ) {
return LHS.ID == RHS.ID;
}
@@ -166,9 +156,6 @@ class LocalDeclID : public DeclIDBase {
LocalDeclID(PredefinedDeclIDs ID) : Base(ID) {}
explicit LocalDeclID(DeclID ID) : Base(ID) {}
- explicit LocalDeclID(unsigned LocalID, unsigned ModuleFileIndex)
- : Base(LocalID, ModuleFileIndex) {}
-
LocalDeclID ++() {
++ID;
return *this;
@@ -188,9 +175,6 @@ class GlobalDeclID : public DeclIDBase {
GlobalDeclID() : Base() {}
explicit GlobalDeclID(DeclID ID) : Base(ID) {}
- explicit GlobalDeclID(unsigned LocalID, unsigned ModuleFileIndex)
- : Base(LocalID, ModuleFileIndex) {}
-
// For DeclIDIterator to be able to convert a GlobalDeclID
// to a LocalDeclID.
explicit operator LocalDeclID() const { return LocalDeclID(this->ID); }
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h
b/clang/include/clang/Serialization/ASTBitCodes.h
index 902c4470650c5..fe1bd47348be1 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -255,12 +255,6 @@ class DeclOffset {
}
};
-// The unaligned decl ID used in the Blobs of bistreams.
-using unaligned_decl_id_t =
-llvm::support::detail::packed_endian_specific_integral<
-serialization::DeclID, llvm::endianness::native,
-llvm::support::unaligned>;
-
/// The number of predefined
[clang] [serialization] no transitive decl change (PR #92083)
ChuanqiXu9 wrote: Oh, this is not wanted. It should be almost a NFC patch for users. I'll revert it. https://github.com/llvm/llvm-project/pull/92083 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] No transitive identifier change (PR #92085)
https://github.com/ChuanqiXu9 updated
https://github.com/llvm/llvm-project/pull/92085
>From e8f756ec7f8ea7e5bf18cc122a965fb2f258fd15 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Tue, 14 May 2024 15:33:12 +0800
Subject: [PATCH] [Serialization] No transitive identifier change
---
.../clang/Lex/ExternalPreprocessorSource.h| 54 -
clang/include/clang/Lex/HeaderSearch.h| 12 +-
.../include/clang/Serialization/ASTBitCodes.h | 2 +-
clang/include/clang/Serialization/ASTReader.h | 19 ++-
.../include/clang/Serialization/ModuleFile.h | 3 -
clang/lib/Lex/HeaderSearch.cpp| 33 +++---
clang/lib/Serialization/ASTReader.cpp | 98
clang/lib/Serialization/ASTWriter.cpp | 63 ++
clang/lib/Serialization/GlobalModuleIndex.cpp | 3 +-
clang/lib/Serialization/ModuleFile.cpp| 1 -
.../no-transitive-identifier-change.cppm | 110 ++
11 files changed, 286 insertions(+), 112 deletions(-)
create mode 100644 clang/test/Modules/no-transitive-identifier-change.cppm
diff --git a/clang/include/clang/Lex/ExternalPreprocessorSource.h
b/clang/include/clang/Lex/ExternalPreprocessorSource.h
index 6775841860373..48429948dbffe 100644
--- a/clang/include/clang/Lex/ExternalPreprocessorSource.h
+++ b/clang/include/clang/Lex/ExternalPreprocessorSource.h
@@ -36,12 +36,64 @@ class ExternalPreprocessorSource {
/// Return the identifier associated with the given ID number.
///
/// The ID 0 is associated with the NULL identifier.
- virtual IdentifierInfo *GetIdentifier(unsigned ID) = 0;
+ virtual IdentifierInfo *GetIdentifier(uint64_t ID) = 0;
/// Map a module ID to a module.
virtual Module *getModule(unsigned ModuleID) = 0;
};
+// Either a pointer to an IdentifierInfo of the controlling macro or the ID
+// number of the controlling macro.
+class LazyIdentifierInfoPtr {
+ // If the low bit is clear, a pointer to the IdentifierInfo. If the low
+ // bit is set, the upper 63 bits are the ID number.
+ mutable uint64_t Ptr = 0;
+
+public:
+ LazyIdentifierInfoPtr() = default;
+
+ explicit LazyIdentifierInfoPtr(const IdentifierInfo *Ptr)
+ : Ptr(reinterpret_cast(Ptr)) {}
+
+ explicit LazyIdentifierInfoPtr(uint64_t ID) : Ptr((ID << 1) | 0x01) {
+assert((ID << 1 >> 1) == ID && "ID must require < 63 bits");
+if (ID == 0)
+ Ptr = 0;
+ }
+
+ LazyIdentifierInfoPtr =(const IdentifierInfo *Ptr) {
+this->Ptr = reinterpret_cast(Ptr);
+return *this;
+ }
+
+ LazyIdentifierInfoPtr =(uint64_t ID) {
+assert((ID << 1 >> 1) == ID && "IDs must require < 63 bits");
+if (ID == 0)
+ Ptr = 0;
+else
+ Ptr = (ID << 1) | 0x01;
+
+return *this;
+ }
+
+ /// Whether this pointer is non-NULL.
+ ///
+ /// This operation does not require the AST node to be deserialized.
+ bool isValid() const { return Ptr != 0; }
+
+ /// Whether this pointer is currently stored as ID.
+ bool isID() const { return Ptr & 0x01; }
+
+ IdentifierInfo *getPtr() const {
+assert(!isID());
+return reinterpret_cast(Ptr);
+ }
+
+ uint64_t getID() const {
+assert(isID());
+return Ptr >> 1;
+ }
+};
}
#endif
diff --git a/clang/include/clang/Lex/HeaderSearch.h
b/clang/include/clang/Lex/HeaderSearch.h
index 5ac634d4e..65700b8f9dc11 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -16,6 +16,7 @@
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Lex/DirectoryLookup.h"
+#include "clang/Lex/ExternalPreprocessorSource.h"
#include "clang/Lex/HeaderMap.h"
#include "clang/Lex/ModuleMap.h"
#include "llvm/ADT/ArrayRef.h"
@@ -119,13 +120,6 @@ struct HeaderFileInfo {
LLVM_PREFERRED_TYPE(bool)
unsigned IsValid : 1;
- /// The ID number of the controlling macro.
- ///
- /// This ID number will be non-zero when there is a controlling
- /// macro whose IdentifierInfo may not yet have been loaded from
- /// external storage.
- unsigned ControllingMacroID = 0;
-
/// If this file has a \#ifndef XXX (or equivalent) guard that
/// protects the entire contents of the file, this is the identifier
/// for the macro that controls whether or not it has any effect.
@@ -134,7 +128,7 @@ struct HeaderFileInfo {
/// the controlling macro of this header, since
/// getControllingMacro() is able to load a controlling macro from
/// external storage.
- const IdentifierInfo *ControllingMacro = nullptr;
+ LazyIdentifierInfoPtr LazyControllingMacro;
/// If this header came from a framework include, this is the name
/// of the framework.
@@ -580,7 +574,7 @@ class HeaderSearch {
/// no-op \#includes.
void SetFileControllingMacro(FileEntryRef File,
const IdentifierInfo *ControllingMacro) {
-getFileInfo(File).ControllingMacro = ControllingMacro;
+getFileInfo(File).LazyControllingMacro = ControllingMacro;
}
/// Determine
[clang] [Serialization] No transitive identifier change (PR #92085)
@@ -3896,7 +3903,7 @@ void ASTWriter::WriteIdentifierTable(Preprocessor , // Write out identifiers if either the ID is local or the identifier has // changed since it was loaded. - if (ID >= FirstIdentID || !Chain || !II->isFromAST() || + if (isLocalIdentifierID(ID) || !Chain || !II->isFromAST() || ChuanqiXu9 wrote: Nice catch! The `!II->isFromAST()` check looks redundant indeed. I've made a NFC patch to address this: https://github.com/llvm/llvm-project/commit/799ae77993fa5d7b0638f10b3895090f8748de92 https://github.com/llvm/llvm-project/pull/92085 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 799ae77 - [NFC] [Serialization] Avoid unnecessary check for if Identifier from AST
Author: Chuanqi Xu Date: 2024-06-04T15:28:24+08:00 New Revision: 799ae77993fa5d7b0638f10b3895090f8748de92 URL: https://github.com/llvm/llvm-project/commit/799ae77993fa5d7b0638f10b3895090f8748de92 DIFF: https://github.com/llvm/llvm-project/commit/799ae77993fa5d7b0638f10b3895090f8748de92.diff LOG: [NFC] [Serialization] Avoid unnecessary check for if Identifier from AST Inspired by the review process in https://github.com/llvm/llvm-project/pull/92085. The check `ID >= FirstIdentID` can cover the following check `!II->isFromAST()`. Added: Modified: clang/lib/Serialization/ASTWriter.cpp Removed: diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 4f1d2c532bc91..b8b613db712f4 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -3895,8 +3895,7 @@ void ASTWriter::WriteIdentifierTable(Preprocessor , // Write out identifiers if either the ID is local or the identifier has // changed since it was loaded. - if (ID >= FirstIdentID || !Chain || !II->isFromAST() || - II->hasChangedSinceDeserialization() || + if (ID >= FirstIdentID || II->hasChangedSinceDeserialization() || (Trait.needDecls() && II->hasFETokenInfoChangedSinceDeserialization())) Generator.insert(II, ID, Trait); ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] d8ec452 - [serialization] no transitive decl change (#92083)
Author: Chuanqi Xu
Date: 2024-06-04T14:45:00+08:00
New Revision: d8ec452db016f359feeec28994f6560b30b49824
URL:
https://github.com/llvm/llvm-project/commit/d8ec452db016f359feeec28994f6560b30b49824
DIFF:
https://github.com/llvm/llvm-project/commit/d8ec452db016f359feeec28994f6560b30b49824.diff
LOG: [serialization] no transitive decl change (#92083)
Following of https://github.com/llvm/llvm-project/pull/86912
The motivation of the patch series is that, for a module interface unit
`X`, when the dependent modules of `X` changes, if the changes is not
relevant with `X`, we hope the BMI of `X` won't change. For the specific
patch, we hope if the changes was about irrelevant declaration changes,
we hope the BMI of `X` won't change. **However**, I found the patch
itself is not very useful in practice, since the adding or removing
declarations, will change the state of identifiers and types in most
cases.
That said, for the most simple example,
```
// partA.cppm
export module m:partA;
// partA.v1.cppm
export module m:partA;
export void a() {}
// partB.cppm
export module m:partB;
export void b() {}
// m.cppm
export module m;
export import :partA;
export import :partB;
// onlyUseB;
export module onlyUseB;
import m;
export inline void onluUseB() {
b();
}
```
the BMI of `onlyUseB` will change after we change the implementation of
`partA.cppm` to `partA.v1.cppm`. Since `partA.v1.cppm` introduces new
identifiers and types (the function prototype).
So in this patch, we have to write the tests as:
```
// partA.cppm
export module m:partA;
export int getA() { ... }
export int getA2(int) { ... }
// partA.v1.cppm
export module m:partA;
export int getA() { ... }
export int getA(int) { ... }
export int getA2(int) { ... }
// partB.cppm
export module m:partB;
export void b() {}
// m.cppm
export module m;
export import :partA;
export import :partB;
// onlyUseB;
export module onlyUseB;
import m;
export inline void onluUseB() {
b();
}
```
so that the new introduced declaration `int getA(int)` doesn't introduce
new identifiers and types, then the BMI of `onlyUseB` can keep
unchanged.
While it looks not so great, the patch should be the base of the patch
to erase the transitive change for identifiers and types since I don't
know how can we introduce new types and identifiers without introducing
new declarations. Given how tightly the relationship between
declarations, types and identifiers, I think we can only reach the ideal
state after we made the series for all of the three entties.
The design of the patch is similar to
https://github.com/llvm/llvm-project/pull/86912, which extends the
32-bit DeclID to 64-bit and use the higher bits to store the module file
index and the lower bits to store the Local Decl ID.
A slight difference is that we only use 48 bits to store the new DeclID
since we try to use the higher 16 bits to store the module ID in the
prefix of Decl class. Previously, we use 32 bits to store the module ID
and 32 bits to store the DeclID. I don't want to allocate additional
space so I tried to make the additional space the same as 64 bits. An
potential interesting thing here is about the relationship between the
module ID and the module file index. I feel we can get the module file
index by the module ID. But I didn't prove it or implement it. Since I
want to make the patch itself as small as possible. We can make it in
the future if we want.
Another change in the patch is the new concept Decl Index, which means
the index of the very big array `DeclsLoaded` in ASTReader. Previously,
the index of a loaded declaration is simply the Decl ID minus
PREDEFINED_DECL_NUMs. So there are some places they got used
ambiguously. But this patch tried to split these two concepts.
As https://github.com/llvm/llvm-project/pull/86912 did, the change will
increase the on-disk PCM file sizes. As the declaration ID may be the
most IDs in the PCM file, this can have the biggest impact on the size.
In my experiments, this change will bring 6.6% increase of the on-disk
PCM size. No compile-time performance regression observed. Given the
benefits in the motivation example, I think the cost is worthwhile.
Added:
clang/test/Modules/no-transitive-decls-change.cppm
Modified:
clang/include/clang/AST/DeclBase.h
clang/include/clang/AST/DeclID.h
clang/include/clang/Serialization/ASTBitCodes.h
clang/include/clang/Serialization/ASTReader.h
clang/include/clang/Serialization/ModuleFile.h
clang/include/clang/Serialization/ModuleManager.h
clang/lib/AST/DeclBase.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTReaderDecl.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/lib/Serialization/ModuleFile.cpp
Removed:
diff --git a/clang/include/clang/AST/DeclBase.h
b/clang/include/clang/AST/DeclBase.h
index 3a311d4c55916..c4eb053146d32 100644
---
[clang] [serialization] no transitive decl change (PR #92083)
ChuanqiXu9 wrote: Thanks for reporting. I've reproduced them (including the lldb test) locally and fixed them. I'll try to land it again to see what happens. https://github.com/llvm/llvm-project/pull/92083 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 6b30180 - Revert "[serialization] no transitive decl change (#92083)"
Author: Chuanqi Xu
Date: 2024-06-03T18:49:18+08:00
New Revision: 6b30180b663e1fe4de32046398581a374c8a54f2
URL:
https://github.com/llvm/llvm-project/commit/6b30180b663e1fe4de32046398581a374c8a54f2
DIFF:
https://github.com/llvm/llvm-project/commit/6b30180b663e1fe4de32046398581a374c8a54f2.diff
LOG: Revert "[serialization] no transitive decl change (#92083)"
This reverts commit ccb73e882b2d727877cfda42a14a6979cfd31f04.
It looks like there are some bots complaining about the patch.
See the post commit comment in
https://github.com/llvm/llvm-project/pull/92083 to track it.
Added:
Modified:
clang/include/clang/AST/DeclBase.h
clang/include/clang/AST/DeclID.h
clang/include/clang/Serialization/ASTBitCodes.h
clang/include/clang/Serialization/ASTReader.h
clang/include/clang/Serialization/ModuleFile.h
clang/include/clang/Serialization/ModuleManager.h
clang/lib/AST/DeclBase.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTReaderDecl.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/lib/Serialization/ModuleFile.cpp
Removed:
clang/test/Modules/no-transitive-decls-change.cppm
diff --git a/clang/include/clang/AST/DeclBase.h
b/clang/include/clang/AST/DeclBase.h
index 4bdf27aa99405..e43e812cd9455 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -701,7 +701,10 @@ class alignas(8) Decl {
/// Set the owning module ID. This may only be called for
/// deserialized Decls.
- void setOwningModuleID(unsigned ID);
+ void setOwningModuleID(unsigned ID) {
+assert(isFromASTFile() && "Only works on a deserialized declaration");
+*((unsigned*)this - 2) = ID;
+ }
public:
/// Determine the availability of the given declaration.
@@ -774,11 +777,19 @@ class alignas(8) Decl {
/// Retrieve the global declaration ID associated with this
/// declaration, which specifies where this Decl was loaded from.
- GlobalDeclID getGlobalID() const;
+ GlobalDeclID getGlobalID() const {
+if (isFromASTFile())
+ return (*((const GlobalDeclID *)this - 1));
+return GlobalDeclID();
+ }
/// Retrieve the global ID of the module that owns this particular
/// declaration.
- unsigned getOwningModuleID() const;
+ unsigned getOwningModuleID() const {
+if (isFromASTFile())
+ return *((const unsigned*)this - 2);
+return 0;
+ }
private:
Module *getOwningModuleSlow() const;
diff --git a/clang/include/clang/AST/DeclID.h
b/clang/include/clang/AST/DeclID.h
index 32d2ed41a374a..614ba06b63860 100644
--- a/clang/include/clang/AST/DeclID.h
+++ b/clang/include/clang/AST/DeclID.h
@@ -19,8 +19,6 @@
#include "llvm/ADT/DenseMapInfo.h"
#include "llvm/ADT/iterator.h"
-#include
-
namespace clang {
/// Predefined declaration IDs.
@@ -109,16 +107,12 @@ class DeclIDBase {
///
/// DeclID should only be used directly in serialization. All other users
/// should use LocalDeclID or GlobalDeclID.
- using DeclID = uint64_t;
+ using DeclID = uint32_t;
protected:
DeclIDBase() : ID(PREDEF_DECL_NULL_ID) {}
explicit DeclIDBase(DeclID ID) : ID(ID) {}
- explicit DeclIDBase(unsigned LocalID, unsigned ModuleFileIndex) {
-ID = (DeclID)LocalID | ((DeclID)ModuleFileIndex << 32);
- }
-
public:
DeclID get() const { return ID; }
@@ -130,10 +124,6 @@ class DeclIDBase {
bool isInvalid() const { return ID == PREDEF_DECL_NULL_ID; }
- unsigned getModuleFileIndex() const { return ID >> 32; }
-
- unsigned getLocalDeclIndex() const;
-
friend bool operator==(const DeclIDBase , const DeclIDBase ) {
return LHS.ID == RHS.ID;
}
@@ -166,9 +156,6 @@ class LocalDeclID : public DeclIDBase {
LocalDeclID(PredefinedDeclIDs ID) : Base(ID) {}
explicit LocalDeclID(DeclID ID) : Base(ID) {}
- explicit LocalDeclID(unsigned LocalID, unsigned ModuleFileIndex)
- : Base(LocalID, ModuleFileIndex) {}
-
LocalDeclID ++() {
++ID;
return *this;
@@ -188,9 +175,6 @@ class GlobalDeclID : public DeclIDBase {
GlobalDeclID() : Base() {}
explicit GlobalDeclID(DeclID ID) : Base(ID) {}
- explicit GlobalDeclID(unsigned LocalID, unsigned ModuleFileIndex)
- : Base(LocalID, ModuleFileIndex) {}
-
// For DeclIDIterator to be able to convert a GlobalDeclID
// to a LocalDeclID.
explicit operator LocalDeclID() const { return LocalDeclID(this->ID); }
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h
b/clang/include/clang/Serialization/ASTBitCodes.h
index 9e4b21baa7d20..fe1bd47348be1 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -255,12 +255,6 @@ class DeclOffset {
}
};
-// The unaligned decl ID used in the Blobs of bistreams.
-using unaligned_decl_id_t =
-llvm::support::detail::packed_endian_specific_integral<
-serialization::DeclID, llvm::endianness::native,
-
[clang] [serialization] no transitive decl change (PR #92083)
ChuanqiXu9 wrote: I'll close it as there are some crashes. --- The crash log: ``` ** TEST 'Clang :: Modules/redecl-ivars.m' FAILED Exit Code: 1 Command Output (stderr): -- RUN: at line 2: rm -rf /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/tools/clang/test/Modules/Output/redecl-ivars.m.tmp + rm -rf /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/tools/clang/test/Modules/Output/redecl-ivars.m.tmp RUN: at line 3: split-file /b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/test/Modules/redecl-ivars.m /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/tools/clang/test/Modules/Output/redecl-ivars.m.tmp + split-file /b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/test/Modules/redecl-ivars.m /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/tools/clang/test/Modules/Output/redecl-ivars.m.tmp RUN: at line 4: /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/bin/clang -cc1 -internal-isystem /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/lib/clang/19/include -nostdsysteminc -fsyntax-only -fobjc-runtime=macosx-10.9 -verify -I/b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/tools/clang/test/Modules/Output/redecl-ivars.m.tmp/include /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/tools/clang/test/Modules/Output/redecl-ivars.m.tmp/test-mismatch-in-extension.m + /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/bin/clang -cc1 -internal-isystem /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/lib/clang/19/include -nostdsysteminc -fsyntax-only -fobjc-runtime=macosx-10.9 -verify -I/b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/tools/clang/test/Modules/Output/redecl-ivars.m.tmp/include /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/tools/clang/test/Modules/Output/redecl-ivars.m.tmp/test-mismatch-in-extension.m RUN: at line 5: /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/bin/clang -cc1 -internal-isystem /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/lib/clang/19/include -nostdsysteminc -fsyntax-only -fobjc-runtime=macosx-10.9 -verify -I/b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/tools/clang/test/Modules/Output/redecl-ivars.m.tmp/include /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/tools/clang/test/Modules/Output/redecl-ivars.m.tmp/test-mismatch-in-extension.m -fmodules -fimplicit-module-maps -fmodules-cache-path=/b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/tools/clang/test/Modules/Output/redecl-ivars.m.tmp/modules.cache + /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/bin/clang -cc1 -internal-isystem /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/lib/clang/19/include -nostdsysteminc -fsyntax-only -fobjc-runtime=macosx-10.9 -verify -I/b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/tools/clang/test/Modules/Output/redecl-ivars.m.tmp/include /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/tools/clang/test/Modules/Output/redecl-ivars.m.tmp/test-mismatch-in-extension.m -fmodules -fimplicit-module-maps -fmodules-cache-path=/b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/tools/clang/test/Modules/Output/redecl-ivars.m.tmp/modules.cache /b/sanitizer-x86_64-linux-fast/build/libcxx_build_asan_ubsan/include/c++/v1/__algorithm/lower_bound.h:39:53: runtime error: reference binding to misaligned address 0x52945c44 for type 'const clang::serialization::ObjCCategoriesInfo', which requires 8 byte alignment 0x52945c44: note: pointer points here 00 00 00 00 02 00 00 00 01 00 00 00 02 00 00 00 00 00 00 00 c3 0d 88 60 0a a8 81 10 03 20 08 00 ^ SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /b/sanitizer-x86_64-linux-fast/build/libcxx_build_asan_ubsan/include/c++/v1/__algorithm/lower_bound.h:39:53 -- It looks like misaligned access too. I'll try to fix it. (It is somewhat hurting that we can't find them on the trunk.) ``` https://github.com/llvm/llvm-project/pull/92083 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] No transitive identifier change (PR #92085)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/92085 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [serialization] no transitive decl change (PR #92083)
https://github.com/ChuanqiXu9 closed https://github.com/llvm/llvm-project/pull/92083 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] a41a20b - [NFC] [C++20] [Modules] [Reduced BMI] Reorder Emitting reduced BMI and normal BMI for named modules
Author: Chuanqi Xu
Date: 2024-06-03T15:47:34+08:00
New Revision: a41a20bd47968b16bb84761578628752080e9f24
URL:
https://github.com/llvm/llvm-project/commit/a41a20bd47968b16bb84761578628752080e9f24
DIFF:
https://github.com/llvm/llvm-project/commit/a41a20bd47968b16bb84761578628752080e9f24.diff
LOG: [NFC] [C++20] [Modules] [Reduced BMI] Reorder Emitting reduced BMI and
normal BMI for named modules
When we generate the reduced BMI on the fly, the order of the emitting
phase is different within `-emit-obj` and `-emit-module-interface`.
Although this is meant to be fine, we observed it in
https://github.com/llvm/llvm-project/issues/93859 (that the different phase
order may cause problems).
Also it turns out to be a different fundamental reason to the orders.
But it might be fine to make the order of emitting reducing BMI at first
to avoid such confusions in the future.
Added:
Modified:
clang/lib/Frontend/FrontendActions.cpp
Removed:
diff --git a/clang/lib/Frontend/FrontendActions.cpp
b/clang/lib/Frontend/FrontendActions.cpp
index 454653a31534c..4f064321997a2 100644
--- a/clang/lib/Frontend/FrontendActions.cpp
+++ b/clang/lib/Frontend/FrontendActions.cpp
@@ -273,9 +273,6 @@ std::unique_ptr
GenerateModuleInterfaceAction::CreateASTConsumer(CompilerInstance ,
StringRef InFile) {
std::vector> Consumers;
- Consumers.push_back(std::make_unique(
- CI.getPreprocessor(), CI.getModuleCache(),
- CI.getFrontendOpts().OutputFile));
if (CI.getFrontendOpts().GenReducedBMI &&
!CI.getFrontendOpts().ModuleOutputPath.empty()) {
@@ -284,6 +281,10 @@
GenerateModuleInterfaceAction::CreateASTConsumer(CompilerInstance ,
CI.getFrontendOpts().ModuleOutputPath));
}
+ Consumers.push_back(std::make_unique(
+ CI.getPreprocessor(), CI.getModuleCache(),
+ CI.getFrontendOpts().OutputFile));
+
return std::make_unique(std::move(Consumers));
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [serialization] no transitive decl change (PR #92083)
ChuanqiXu9 wrote: I'd like to land this since: - I want to give more time testing this - the design of the patch is pretty similar with the previous one (https://github.com/llvm/llvm-project/pull/86912) - the scale of the patch is not very big (100+ lines of code except new tests) - I do think the functionality is very very important to modules. https://github.com/llvm/llvm-project/pull/92083 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] a68638b - [C++20] [Modules] [Reduced BMI] Handling Deduction Guide in reduced BMI
Author: Chuanqi Xu
Date: 2024-06-03T14:59:23+08:00
New Revision: a68638bf6a6a5cb60947753ccaf7d1de80f6c89e
URL:
https://github.com/llvm/llvm-project/commit/a68638bf6a6a5cb60947753ccaf7d1de80f6c89e
DIFF:
https://github.com/llvm/llvm-project/commit/a68638bf6a6a5cb60947753ccaf7d1de80f6c89e.diff
LOG: [C++20] [Modules] [Reduced BMI] Handling Deduction Guide in reduced BMI
carefully
Close https://github.com/llvm/llvm-project/issues/93859
The direct pattern of the issue is that, in a reduced BMI, we're going
to wrtie a class but we didn't write the deduction guide. Although we
handled deduction guide, but we tried to record the found deduction
guide from `noload_lookup` directly.
It is slightly problematic if the found deduction guide is from AST.
e.g.,
```
module;
export module m;
import xxx; // Also contains the class and the deduction guide
...
```
Then when we writes the class in the current file, we tried to record
the deduction guide, but `noload_lookup` returns the deduction guide
from the AST file then we didn't record the local deduction guide. Then
mismatch happens.
To mitiagte the problem, we tried to record the canonical declaration
for the decution guide.
Added:
clang/test/Modules/pr93859.cppm
Modified:
clang/lib/Serialization/ASTWriterDecl.cpp
Removed:
diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp
b/clang/lib/Serialization/ASTWriterDecl.cpp
index bbd16dbdb8fff..5a6ab4408eb2b 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1733,7 +1733,7 @@ void
ASTDeclWriter::VisitClassTemplateDecl(ClassTemplateDecl *D) {
if (Writer.isGeneratingReducedBMI()) {
auto Name = Context.DeclarationNames.getCXXDeductionGuideName(D);
for (auto *DG : D->getDeclContext()->noload_lookup(Name))
- Writer.GetDeclRef(DG);
+ Writer.GetDeclRef(DG->getCanonicalDecl());
}
Code = serialization::DECL_CLASS_TEMPLATE;
diff --git a/clang/test/Modules/pr93859.cppm b/clang/test/Modules/pr93859.cppm
new file mode 100644
index 0..d1d45bb975308
--- /dev/null
+++ b/clang/test/Modules/pr93859.cppm
@@ -0,0 +1,146 @@
+// Reduced from https://github.com/llvm/llvm-project/issues/93859
+//
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/reduced_std.cppm
-emit-reduced-module-interface -o %t/reduced_std.pcm
+// RUN: %clang_cc1 -std=c++20 %t/Misc.cppm -emit-reduced-module-interface -o
%t/Misc.pcm \
+// RUN: -fprebuilt-module-path=%t
+// RUN: %clang_cc1 -std=c++20 %t/Instance.cppm -emit-reduced-module-interface
-o %t/Instance.pcm \
+// RUN: -fprebuilt-module-path=%t
+// RUN: %clang_cc1 -std=c++20 %t/Device.cppm -emit-reduced-module-interface -o
%t/Device.pcm \
+// RUN: -fprebuilt-module-path=%t
+// RUN: %clang_cc1 -std=c++20 %t/Overlay.cppm -emit-reduced-module-interface
-o %t/Overlay.pcm \
+// RUN: -fprebuilt-module-path=%t
+// RUN: %clang_cc1 -std=c++20 %t/App.cppm -emit-module-interface -o /dev/null \
+// RUN: -fexperimental-modules-reduced-bmi -fmodule-output=%t/App.pcm \
+// RUN: -fprebuilt-module-path=%t
+// RUN: %clang_cc1 -std=c++20 %t/test.cc -fsyntax-only -verify \
+// RUN: -fprebuilt-module-path=%t
+
+//--- header.h
+namespace std {
+
+template
+struct pair
+{
+ _T1 first;
+ _T2 second;
+
+ constexpr pair()
+ : first(), second() {}
+
+ constexpr pair(_T1 const& __t1, _T2 const& __t2)
+ : first(__t1), second(__t2) {}
+};
+
+template
+pair(_T1, _T2) -> pair<_T1, _T2>;
+
+template
+class __tree_const_iterator {
+public:
+ template
+ friend class __tree;
+};
+
+template
+class __tree {
+public:
+ typedef _Tp value_type;
+ typedef __tree_const_iterator const_iterator;
+
+ template
+ friend class map;
+};
+
+template
+class set {
+public:
+ typedef __tree<_Key> __base;
+
+ typedef typename __base::const_iterator iterator;
+
+ set() {}
+
+ pair
+ insert(const _Key& __v);
+};
+
+template
+inline constexpr _OutputIterator
+copy(_InputIterator __first, _InputIterator __last, _OutputIterator __result) {
+ return pair{__first, __last}.second;
+}
+
+}
+
+//--- reduced_std.cppm
+module;
+#include "header.h"
+export module reduced_std;
+
+export namespace std {
+using std::set;
+using std::copy;
+}
+
+//--- Misc.cppm
+export module Misc;
+import reduced_std;
+
+export void check_result(int res) {
+std::set extensions;
+extensions.insert('f');
+}
+
+//--- Instance.cppm
+export module Instance;
+import reduced_std;
+
+export class Instance {
+public:
+Instance() {
+std::set extensions;
+extensions.insert("foo");
+}
+};
+
+//--- Device.cppm
+export module Device;
+import reduced_std;
+import Instance;
+import Misc;
+
+std::set wtf_set;
+
+//--- Overlay.cppm
+export module Overlay;
+
+import reduced_std;
+import Device;
+
+void overlay_vector_use() {
+std::set nums;
+
[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)
@@ -1180,6 +1185,21 @@ bool CodeGenVTables::isVTableExternal(const
CXXRecordDecl *RD) {
TSK == TSK_ExplicitInstantiationDefinition)
return false;
+ // Itanium C++ ABI [5.2.3]:
+ // Virtual tables for dynamic classes are emitted as follows:
+ //
+ // - If the class is templated, the tables are emitted in every object that
+ // references any of them.
+ // - Otherwise, if the class is attached to a module, the tables are uniquely
+ // emitted in the object for the module unit in which it is defined.
+ // - Otherwise, if the class has a key function (see below), the tables are
+ // emitted in the object for the translation unit containing the definition
of
+ // the key function. This is unique if the key function is not inline.
+ // - Otherwise, the tables are emitted in every object that references any of
+ // them.
+ if (Module *M = RD->getOwningModule(); M && M->isNamedModule())
ChuanqiXu9 wrote:
Yeah, I agree it might be good conceptly. But given the clang header modules
with object files are not widely used and the current patch may be somewhat
impactful, I feel better to do that in the future when we really need to.
https://github.com/llvm/llvm-project/pull/75912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [libcxx] [clang][Modules] Remove unnecessary includes of `Module.h` (PR #93417)
https://github.com/ChuanqiXu9 closed https://github.com/llvm/llvm-project/pull/93417 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [libcxx] [clang][Modules] Remove unnecessary includes of `Module.h` (PR #93417)
ChuanqiXu9 wrote: > Is there any additional work needed on this before it can be merged? No, it looks good. I'll merge this. https://github.com/llvm/llvm-project/pull/93417 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [libcxx] [clang][Modules] Remove unnecessary includes of `Module.h` (PR #93417)
https://github.com/ChuanqiXu9 approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/93417 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Be const-correct with all uses of `Module *`. (PR #93493)
https://github.com/ChuanqiXu9 approved this pull request. I feel good with this too. But if later you have similar multiple consecutive large NFC patches like this, I'll recommend you send the PR as stacked PR and then we can land the continuously. It'll help the downstream project slightly. https://github.com/llvm/llvm-project/pull/93493 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)
@@ -1802,6 +1802,12 @@ void ItaniumCXXABI::emitVTableDefinitions(CodeGenVTables , if (VTable->hasInitializer()) return; + // If the class is attached to a C++ named module other than the one + // we're currently compiling, the vtable should be defined there. + if (Module *M = RD->getOwningModule(); + M && M->isNamedModule() && M != CGM.getContext().getCurrentNamedModule()) ChuanqiXu9 wrote: Yeah, understood. I just wanted to avoid checkout the old branch so that I can save the building time locally. And this point looks minor. I feel good to make it in following patches or with other required changes together in this patch. https://github.com/llvm/llvm-project/pull/75912 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)
@@ -1180,6 +1185,21 @@ bool CodeGenVTables::isVTableExternal(const
CXXRecordDecl *RD) {
TSK == TSK_ExplicitInstantiationDefinition)
return false;
+ // Itanium C++ ABI [5.2.3]:
+ // Virtual tables for dynamic classes are emitted as follows:
+ //
+ // - If the class is templated, the tables are emitted in every object that
+ // references any of them.
+ // - Otherwise, if the class is attached to a module, the tables are uniquely
+ // emitted in the object for the module unit in which it is defined.
+ // - Otherwise, if the class has a key function (see below), the tables are
+ // emitted in the object for the translation unit containing the definition
of
+ // the key function. This is unique if the key function is not inline.
+ // - Otherwise, the tables are emitted in every object that references any of
+ // them.
+ if (Module *M = RD->getOwningModule(); M && M->isNamedModule())
ChuanqiXu9 wrote:
> hasExternalDefinitions isn't "is this AST from some other AST file" but "did
> the AST file say it'd handle the object-file definition of this thing" - the
> pcm file specifies which things it'll handle.
> So if the intent is not to home anything in the GMF, then pcms could be
> written out that don't set "hasExternalDefinitions" to true for those
> entities.
No, it is not the intent. The intent is to keep GMF as legacy code instead of
named modules.
For example,
```
// header.h
void func() { } // ! Non inline
// x.cppm
module;
#include "header.h"
export module x;
...
```
Then in the object file of `x.cppm`, (if `func()` is not discarded out), we
should be able to see the definition of `func()`. Although it is rare since the
definitions in the headers are generally inline, but we as the compiler can't
assume that.
I think the current implementation is more straight forward and clear.
https://github.com/llvm/llvm-project/pull/75912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] fix merging of UsingShadowDecl (PR #80245)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/80245 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] fix merging of UsingShadowDecl (PR #80245)
https://github.com/ChuanqiXu9 approved this pull request. LG. It looks not wise to block this good PR. After all, it is a change 6 lines. Let's go ahead. https://github.com/llvm/llvm-project/pull/80245 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)
@@ -1802,6 +1802,12 @@ void ItaniumCXXABI::emitVTableDefinitions(CodeGenVTables , if (VTable->hasInitializer()) return; + // If the class is attached to a C++ named module other than the one + // we're currently compiling, the vtable should be defined there. + if (Module *M = RD->getOwningModule(); + M && M->isNamedModule() && M != CGM.getContext().getCurrentNamedModule()) ChuanqiXu9 wrote: If we feel this is too long, I can wrap them into a member function of Decl in a following patch. https://github.com/llvm/llvm-project/pull/75912 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)
@@ -1180,6 +1185,21 @@ bool CodeGenVTables::isVTableExternal(const
CXXRecordDecl *RD) {
TSK == TSK_ExplicitInstantiationDefinition)
return false;
+ // Itanium C++ ABI [5.2.3]:
+ // Virtual tables for dynamic classes are emitted as follows:
+ //
+ // - If the class is templated, the tables are emitted in every object that
+ // references any of them.
+ // - Otherwise, if the class is attached to a module, the tables are uniquely
+ // emitted in the object for the module unit in which it is defined.
+ // - Otherwise, if the class has a key function (see below), the tables are
+ // emitted in the object for the translation unit containing the definition
of
+ // the key function. This is unique if the key function is not inline.
+ // - Otherwise, the tables are emitted in every object that references any of
+ // them.
+ if (Module *M = RD->getOwningModule(); M && M->isNamedModule())
ChuanqiXu9 wrote:
Maybe it is not strictly correct to use `hasExternalDefinitions()` since it
can't handle the global module case. I mean, from the current wording, the
class in the global module should follow the old ABI rules (key functions). But
`hasExternalDefinitions()` won't make that clear.
https://github.com/llvm/llvm-project/pull/75912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Check for stack exhaustion when reading declarations (PR #79875)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/79875 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Check for stack exhaustion when reading declarations (PR #79875)
https://github.com/ChuanqiXu9 approved this pull request. If we don't insist on testing this in this commit, them LGTM. https://github.com/llvm/llvm-project/pull/79875 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Check for stack exhaustion when reading declarations (PR #79875)
@@ -9403,6 +9404,20 @@ DiagnosticBuilder ASTReader::Diag(SourceLocation Loc,
unsigned DiagID) const {
return Diags.Report(Loc, DiagID);
}
+void ASTReader::warnStackExhausted(SourceLocation Loc) {
+ // When Sema is available, avoid duplicate errors. This should be rare.
ChuanqiXu9 wrote:
```suggestion
// When Sema is available, avoid duplicate errors.
```
https://github.com/llvm/llvm-project/pull/79875
___
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 generate the defintition for non-const available external variables (PR #93530)
https://github.com/ChuanqiXu9 closed https://github.com/llvm/llvm-project/pull/93530 ___ 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 generate the defintition for non-const available external variables (PR #93530)
@@ -5341,6 +5341,15 @@ void CodeGenModule::EmitGlobalVarDefinition(const VarDecl *D, !IsDefinitionAvailableExternally && D->needsDestruction(getContext()) == QualType::DK_cxx_destructor; + // It is helpless to emit the definition for an available_externally variable + // which can't be marked as const. + // We don't need to check if it needs global ctor or dtor. See the above + // comment for ideas. + if (IsDefinitionAvailableExternally && + (!D->hasConstantInitialization() || + !D->getType().isConstantStorage(getContext(), true, true))) ChuanqiXu9 wrote: Got it. Makes sense. Done. https://github.com/llvm/llvm-project/pull/93530 ___ 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 generate the defintition for non-const available external variables (PR #93530)
https://github.com/ChuanqiXu9 updated
https://github.com/llvm/llvm-project/pull/93530
>From ebe47b2b411d7623ddafadad45a9be25913fe1c1 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Wed, 29 May 2024 10:48:51 +0800
Subject: [PATCH] [C++20] [Modules] Don't generate the defintition for
non-const available external variables
Close #93497
The root cause of the problem is, we mark the variable from other
modules as constnant in LLVM incorrectly. This patch fixes this problem
by not emitting the defintition for non-const available external
variables. Since the non const available externally variable is not
helpful to the optimization.
---
clang/lib/CodeGen/CodeGenModule.cpp | 12 +++
clang/test/CodeGenCXX/partitions.cpp | 8 +-
clang/test/Modules/pr93497.cppm | 106 +++
3 files changed, 122 insertions(+), 4 deletions(-)
create mode 100644 clang/test/Modules/pr93497.cppm
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp
b/clang/lib/CodeGen/CodeGenModule.cpp
index e4774a587707a..0b0b659e1fd49 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -5341,6 +5341,18 @@ void CodeGenModule::EmitGlobalVarDefinition(const
VarDecl *D,
!IsDefinitionAvailableExternally &&
D->needsDestruction(getContext()) == QualType::DK_cxx_destructor;
+ // It is helpless to emit the definition for an available_externally variable
+ // which can't be marked as const.
+ // We don't need to check if it needs global ctor or dtor. See the above
+ // comment for ideas.
+ if (IsDefinitionAvailableExternally &&
+ (!D->hasConstantInitialization() ||
+ // TODO: Update this when we have interface to check constexpr
+ // destructor.
+ D->needsDestruction(getContext()) ||
+ !D->getType().isConstantStorage(getContext(), true, true)))
+return;
+
const VarDecl *InitDecl;
const Expr *InitExpr = D->getAnyInitializer(InitDecl);
diff --git a/clang/test/CodeGenCXX/partitions.cpp
b/clang/test/CodeGenCXX/partitions.cpp
index d283dd071f6b2..e80e68f82974b 100644
--- a/clang/test/CodeGenCXX/partitions.cpp
+++ b/clang/test/CodeGenCXX/partitions.cpp
@@ -40,12 +40,12 @@ export int use() {
}
// FIXME: The definition of the variables shouldn't be exported too.
-// CHECK: @_ZW3mod1a = available_externally global
-// CHECK: @_ZW3mod1b = available_externally global
+// CHECK: @_ZW3mod1a = external global
+// CHECK: @_ZW3mod1b = external global
// CHECK: declare{{.*}} i32 @_ZW3mod3foov
// CHECK: declare{{.*}} i32 @_ZW3mod3barv
-// CHECK-OPT: @_ZW3mod1a = available_externally global
-// CHECK-OPT: @_ZW3mod1b = available_externally global
+// CHECK-OPT: @_ZW3mod1a = external global
+// CHECK-OPT: @_ZW3mod1b = external global
// CHECK-OPT: declare{{.*}} i32 @_ZW3mod3foov
// CHECK-OPT: declare{{.*}} i32 @_ZW3mod3barv
diff --git a/clang/test/Modules/pr93497.cppm b/clang/test/Modules/pr93497.cppm
new file mode 100644
index 0..64a08e2a85e63
--- /dev/null
+++ b/clang/test/Modules/pr93497.cppm
@@ -0,0 +1,106 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 %t/mod.cppm \
+// RUN: -emit-module-interface -o %t/mod.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 %t/use.cpp \
+// RUN: -fmodule-file=mod=%t/mod.pcm -emit-llvm \
+// RUN: -o - | opt -S --passes=simplifycfg | FileCheck %t/use.cpp
+
+//--- mod.cppm
+export module mod;
+
+export struct Thing {
+static const Thing One;
+explicit Thing(int raw) :raw(raw) { }
+int raw;
+};
+
+const Thing Thing::One = Thing(1);
+
+export struct C {
+int value;
+};
+export const C ConstantValue = {1};
+
+export const C *ConstantPtr =
+
+C NonConstantValue = {1};
+export const C = NonConstantValue;
+
+export struct NonConstexprDtor {
+constexpr NonConstexprDtor(int raw) : raw(raw) {}
+~NonConstexprDtor();
+
+int raw;
+};
+
+export const NonConstexprDtor NonConstexprDtorValue = {1};
+
+//--- use.cpp
+import mod;
+
+int consume(int);
+int consumeC(C);
+
+extern "C" __attribute__((noinline)) inline int unneeded() {
+return consume(43);
+}
+
+extern "C" __attribute__((noinline)) inline int needed() {
+return consume(43);
+}
+
+int use() {
+Thing t1 = Thing::One;
+return consume(t1.raw);
+}
+
+int use2() {
+if (ConstantValue.value)
+return consumeC(ConstantValue);
+return unneeded();
+}
+
+int use3() {
+auto Ptr = ConstantPtr;
+if (Ptr->value)
+return consumeC(*Ptr);
+return needed();
+}
+
+int use4() {
+auto Ref = ConstantRef;
+if (Ref.value)
+return consumeC(Ref);
+return needed();
+}
+
+int use5() {
+NonConstexprDtor V = NonConstexprDtorValue;
+if (V.raw)
+return consume(V.raw);
+return needed();
+}
+
+// CHECK: @_ZNW3mod5Thing3OneE = external
+// CHECK: @_ZW3mod13ConstantValue ={{.*}}available_externally{{.*}} constant
+// CHECK: @_ZW3mod11ConstantPtr =
[clang] [C++20] [Modules] Don't generate the defintition for non-const available external variables (PR #93530)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/93530 ___ 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 generate the defintition for non-const available external variables (PR #93530)
@@ -5341,6 +5341,15 @@ void CodeGenModule::EmitGlobalVarDefinition(const VarDecl *D, !IsDefinitionAvailableExternally && D->needsDestruction(getContext()) == QualType::DK_cxx_destructor; + // It is helpless to emit the definition for an available_externally variable + // which can't be marked as const. + // We don't need to check if it needs global ctor or dtor. See the above + // comment for ideas. + if (IsDefinitionAvailableExternally && + (!D->hasConstantInitialization() || + !D->getType().isConstantStorage(getContext(), true, true))) ChuanqiXu9 wrote: (we think) the available externally variables don't need ctor or dtor in the current TU. We mentioned this in the comment. https://github.com/llvm/llvm-project/pull/93530 ___ 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 generate the defintition for non-const available external variables (PR #93530)
ChuanqiXu9 wrote: > I think if a variable is GVA_AvailableExternally, and we can't emit a > constant, we should just completely skip emitting the definition: there isn't > any point to emitting an available_externally definition that doesn't > actually contain any information the optimizer can use. > > Not sure off the top of my head where that check belongs; might be okay to > just stick it into EmitGlobalVarDefinition itself. Neat suggestion! I've applied it and the generated code looks better. https://github.com/llvm/llvm-project/pull/93530 ___ 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 generate the defintition for non-const available external variables (PR #93530)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/93530 ___ 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 generate the defintition for non-const available external variables (PR #93530)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/93530 ___ 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 mark variables from other modules as constant if its initializer is not constant (PR #93530)
https://github.com/ChuanqiXu9 updated
https://github.com/llvm/llvm-project/pull/93530
>From 05542b6176a84888438dd8c6cc9a86cb6f1b7282 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Wed, 29 May 2024 10:48:51 +0800
Subject: [PATCH] [C++20] [Modules] Don't generate the defintition for
non-const available external variables
Close #93497
The root cause of the problem is, we mark the variable from other
modules as constnant in LLVM incorrectly. This patch fixes this problem
by not emitting the defintition for non-const available external
variables. Since the non const available externally variable is not
helpful to the optimization.
---
clang/lib/CodeGen/CodeGenModule.cpp | 9 +++
clang/test/CodeGenCXX/partitions.cpp | 8 +--
clang/test/Modules/pr93497.cppm | 83
3 files changed, 96 insertions(+), 4 deletions(-)
create mode 100644 clang/test/Modules/pr93497.cppm
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp
b/clang/lib/CodeGen/CodeGenModule.cpp
index e4774a587707a..1661a195d5a38 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -5341,6 +5341,15 @@ void CodeGenModule::EmitGlobalVarDefinition(const
VarDecl *D,
!IsDefinitionAvailableExternally &&
D->needsDestruction(getContext()) == QualType::DK_cxx_destructor;
+ // It is helpless to emit the definition for an available_externally variable
+ // which can't be marked as const.
+ // We don't need to check if it needs global ctor or dtor. See the above
+ // comment for ideas.
+ if (IsDefinitionAvailableExternally &&
+ (!D->hasConstantInitialization() ||
+ !D->getType().isConstantStorage(getContext(), true, true)))
+return;
+
const VarDecl *InitDecl;
const Expr *InitExpr = D->getAnyInitializer(InitDecl);
diff --git a/clang/test/CodeGenCXX/partitions.cpp
b/clang/test/CodeGenCXX/partitions.cpp
index d283dd071f6b2..e80e68f82974b 100644
--- a/clang/test/CodeGenCXX/partitions.cpp
+++ b/clang/test/CodeGenCXX/partitions.cpp
@@ -40,12 +40,12 @@ export int use() {
}
// FIXME: The definition of the variables shouldn't be exported too.
-// CHECK: @_ZW3mod1a = available_externally global
-// CHECK: @_ZW3mod1b = available_externally global
+// CHECK: @_ZW3mod1a = external global
+// CHECK: @_ZW3mod1b = external global
// CHECK: declare{{.*}} i32 @_ZW3mod3foov
// CHECK: declare{{.*}} i32 @_ZW3mod3barv
-// CHECK-OPT: @_ZW3mod1a = available_externally global
-// CHECK-OPT: @_ZW3mod1b = available_externally global
+// CHECK-OPT: @_ZW3mod1a = external global
+// CHECK-OPT: @_ZW3mod1b = external global
// CHECK-OPT: declare{{.*}} i32 @_ZW3mod3foov
// CHECK-OPT: declare{{.*}} i32 @_ZW3mod3barv
diff --git a/clang/test/Modules/pr93497.cppm b/clang/test/Modules/pr93497.cppm
new file mode 100644
index 0..fa88eadabdcd7
--- /dev/null
+++ b/clang/test/Modules/pr93497.cppm
@@ -0,0 +1,83 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 %t/mod.cppm \
+// RUN: -emit-module-interface -o %t/mod.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 %t/use.cpp \
+// RUN: -fmodule-file=mod=%t/mod.pcm -emit-llvm \
+// RUN: -o - | FileCheck %t/use.cpp
+
+//--- mod.cppm
+export module mod;
+
+export struct Thing {
+static const Thing One;
+explicit Thing(int raw) :raw(raw) { }
+int raw;
+};
+
+const Thing Thing::One = Thing(1);
+
+export struct C {
+int value;
+};
+export const C ConstantValue = {1};
+
+export const C *ConstantPtr =
+
+C NonConstantValue = {1};
+export const C = NonConstantValue;
+
+//--- use.cpp
+import mod;
+
+int consume(int);
+int consumeC(C);
+
+extern "C" __attribute__((noinline)) inline int unneeded() {
+return consume(43);
+}
+
+extern "C" __attribute__((noinline)) inline int needed() {
+return consume(43);
+}
+
+int use() {
+Thing t1 = Thing::One;
+return consume(t1.raw);
+}
+
+int use2() {
+if (ConstantValue.value)
+return consumeC(ConstantValue);
+return unneeded();
+}
+
+int use3() {
+if (ConstantPtr->value)
+return consumeC(*ConstantPtr);
+return needed();
+}
+
+int use4() {
+if (ConstantRef.value)
+return consumeC(ConstantRef);
+return needed();
+}
+
+// CHECK: @_ZNW3mod5Thing3OneE = external
+// CHECK: @_ZW3mod13ConstantValue ={{.*}}available_externally{{.*}} constant
+// CHECK: @_ZW3mod11ConstantPtr = external
+// CHECK: @_ZW3mod16NonConstantValue = external
+
+// Check that the middle end can optimize the program by the constant
information.
+// CHECK-NOT: @unneeded(
+
+// Check that the use of ConstantPtr won't get optimized incorrectly.
+// CHECK-LABEL: @_Z4use3v(
+// CHECK: @needed(
+
+// Check that the use of ConstantRef won't get optimized incorrectly.
+// CHECK-LABEL: @_Z4use4v(
+// CHECK: @needed(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
[clang] [C++20] [Modules] Don't mark variables from other modules as constant if its initializer is not constant (PR #93530)
https://github.com/ChuanqiXu9 updated
https://github.com/llvm/llvm-project/pull/93530
>From eaf720ca9d3a6a576eefbf9ac553b108611ec00d Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Wed, 29 May 2024 10:48:51 +0800
Subject: [PATCH] [C++20] [Modules] Don't generate the defintition for
non-const available external variables
Close #93497
The root cause of the problem is, we mark the variable from other
modules as constnant in LLVM incorrectly. This patch fixes this problem
by not emitting the defintition for non-const available external
variables. Since the non const available externally variable is not
helpful to the optimization.
---
clang/lib/CodeGen/CodeGenModule.cpp | 9 +++
clang/test/CodeGenCXX/partitions.cpp | 8 +--
clang/test/Modules/pr93497.cppm | 83
3 files changed, 96 insertions(+), 4 deletions(-)
create mode 100644 clang/test/Modules/pr93497.cppm
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp
b/clang/lib/CodeGen/CodeGenModule.cpp
index e4774a587707a..77ce49ea46120 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -5341,6 +5341,15 @@ void CodeGenModule::EmitGlobalVarDefinition(const
VarDecl *D,
!IsDefinitionAvailableExternally &&
D->needsDestruction(getContext()) == QualType::DK_cxx_destructor;
+ // It is helpless to emit the definition for an available_externally variable
+ // which can't be marked as const.
+ // We don't need to check if it needs global ctor or dtor. See the above
comment
+ // for ideas.
+ if (IsDefinitionAvailableExternally &&
+ (!D->hasConstantInitialization() ||
+ !D->getType().isConstantStorage(getContext(), true, true)))
+return;
+
const VarDecl *InitDecl;
const Expr *InitExpr = D->getAnyInitializer(InitDecl);
diff --git a/clang/test/CodeGenCXX/partitions.cpp
b/clang/test/CodeGenCXX/partitions.cpp
index d283dd071f6b2..e80e68f82974b 100644
--- a/clang/test/CodeGenCXX/partitions.cpp
+++ b/clang/test/CodeGenCXX/partitions.cpp
@@ -40,12 +40,12 @@ export int use() {
}
// FIXME: The definition of the variables shouldn't be exported too.
-// CHECK: @_ZW3mod1a = available_externally global
-// CHECK: @_ZW3mod1b = available_externally global
+// CHECK: @_ZW3mod1a = external global
+// CHECK: @_ZW3mod1b = external global
// CHECK: declare{{.*}} i32 @_ZW3mod3foov
// CHECK: declare{{.*}} i32 @_ZW3mod3barv
-// CHECK-OPT: @_ZW3mod1a = available_externally global
-// CHECK-OPT: @_ZW3mod1b = available_externally global
+// CHECK-OPT: @_ZW3mod1a = external global
+// CHECK-OPT: @_ZW3mod1b = external global
// CHECK-OPT: declare{{.*}} i32 @_ZW3mod3foov
// CHECK-OPT: declare{{.*}} i32 @_ZW3mod3barv
diff --git a/clang/test/Modules/pr93497.cppm b/clang/test/Modules/pr93497.cppm
new file mode 100644
index 0..fa88eadabdcd7
--- /dev/null
+++ b/clang/test/Modules/pr93497.cppm
@@ -0,0 +1,83 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 %t/mod.cppm \
+// RUN: -emit-module-interface -o %t/mod.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 %t/use.cpp \
+// RUN: -fmodule-file=mod=%t/mod.pcm -emit-llvm \
+// RUN: -o - | FileCheck %t/use.cpp
+
+//--- mod.cppm
+export module mod;
+
+export struct Thing {
+static const Thing One;
+explicit Thing(int raw) :raw(raw) { }
+int raw;
+};
+
+const Thing Thing::One = Thing(1);
+
+export struct C {
+int value;
+};
+export const C ConstantValue = {1};
+
+export const C *ConstantPtr =
+
+C NonConstantValue = {1};
+export const C = NonConstantValue;
+
+//--- use.cpp
+import mod;
+
+int consume(int);
+int consumeC(C);
+
+extern "C" __attribute__((noinline)) inline int unneeded() {
+return consume(43);
+}
+
+extern "C" __attribute__((noinline)) inline int needed() {
+return consume(43);
+}
+
+int use() {
+Thing t1 = Thing::One;
+return consume(t1.raw);
+}
+
+int use2() {
+if (ConstantValue.value)
+return consumeC(ConstantValue);
+return unneeded();
+}
+
+int use3() {
+if (ConstantPtr->value)
+return consumeC(*ConstantPtr);
+return needed();
+}
+
+int use4() {
+if (ConstantRef.value)
+return consumeC(ConstantRef);
+return needed();
+}
+
+// CHECK: @_ZNW3mod5Thing3OneE = external
+// CHECK: @_ZW3mod13ConstantValue ={{.*}}available_externally{{.*}} constant
+// CHECK: @_ZW3mod11ConstantPtr = external
+// CHECK: @_ZW3mod16NonConstantValue = external
+
+// Check that the middle end can optimize the program by the constant
information.
+// CHECK-NOT: @unneeded(
+
+// Check that the use of ConstantPtr won't get optimized incorrectly.
+// CHECK-LABEL: @_Z4use3v(
+// CHECK: @needed(
+
+// Check that the use of ConstantRef won't get optimized incorrectly.
+// CHECK-LABEL: @_Z4use4v(
+// CHECK: @needed(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
[clang] [clang] Be const-correct with all uses of `Module *`. (PR #93493)
ChuanqiXu9 wrote: BTW, for the series patches, it may be better to use stacked PR (we use stacked PR by the method in https://llvm.org/docs/GitHub.html) if you have following patches (and it looks like you have a lot). Also, as a downstream vendor, I hope we can land the (large, NFC) patch series together. Since it shows some pains in backporting such patch series. https://github.com/llvm/llvm-project/pull/93493 ___ 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 mark variables from other modules as constant if its initializer is not constant (PR #93530)
https://github.com/ChuanqiXu9 created
https://github.com/llvm/llvm-project/pull/93530
Close https://github.com/llvm/llvm-project/issues/93497
The root cause of the problem is, we mark the variable from other modules as
constnant in LLVM incorrectly. This patch fixes this problem and only mark the
variable from other modules as constant if its initializer is const.
>From 778205df51376ddd38253b2ce5bab9dcab512774 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Tue, 28 May 2024 18:44:18 +0800
Subject: [PATCH] [C++20] [Modules] Don't mark variables from other modules as
constant if its initializer is not constant
Close https://github.com/llvm/llvm-project/issues/93497
The root cause of the problem is, we mark the variable from other
modules as constnant in LLVM incorrectly. This patch fixes this problem
and only mark the variable from other modules as constant if its
initializer is const.
---
clang/lib/CodeGen/CodeGenModule.cpp | 2 +
clang/test/Modules/pr93497.cppm | 81 +
2 files changed, 83 insertions(+)
create mode 100644 clang/test/Modules/pr93497.cppm
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp
b/clang/lib/CodeGen/CodeGenModule.cpp
index e4774a587707a..5596a6708e4a1 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -5492,6 +5492,8 @@ void CodeGenModule::EmitGlobalVarDefinition(const VarDecl
*D,
// If it is safe to mark the global 'constant', do so now.
GV->setConstant(!NeedsGlobalCtor && !NeedsGlobalDtor &&
+ (!IsDefinitionAvailableExternally ||
+ D->hasConstantInitialization()) &&
D->getType().isConstantStorage(getContext(), true, true));
// If it is in a read-only section, mark it 'constant'.
diff --git a/clang/test/Modules/pr93497.cppm b/clang/test/Modules/pr93497.cppm
new file mode 100644
index 0..dc0d8eb462e27
--- /dev/null
+++ b/clang/test/Modules/pr93497.cppm
@@ -0,0 +1,81 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 %t/mod.cppm \
+// RUN: -emit-module-interface -o %t/mod.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 %t/use.cpp \
+// RUN: -fmodule-file=mod=%t/mod.pcm -emit-llvm \
+// RUN: -o - | FileCheck %t/use.cpp
+
+//--- mod.cppm
+export module mod;
+
+export struct Thing {
+static const Thing One;
+explicit Thing(int raw) :raw(raw) { }
+int raw;
+};
+
+const Thing Thing::One = Thing(1);
+
+export struct C {
+int value;
+};
+export const C ConstantValue = {1};
+
+export const C *ConstantPtr =
+
+C NonConstantValue = {1};
+export const C = NonConstantValue;
+
+//--- use.cpp
+import mod;
+
+int consume(int);
+int consumeC(C);
+
+extern "C" __attribute__((noinline)) inline int unneeded() {
+return consume(43);
+}
+
+extern "C" __attribute__((noinline)) inline int needed() {
+return consume(43);
+}
+
+int use() {
+Thing t1 = Thing::One;
+return consume(t1.raw);
+}
+
+int use2() {
+if (ConstantValue.value)
+return consumeC(ConstantValue);
+return unneeded();
+}
+
+int use3() {
+if (ConstantPtr->value)
+return consumeC(*ConstantPtr);
+return needed();
+}
+
+int use4() {
+if (ConstantRef.value)
+return consumeC(ConstantRef);
+return needed();
+}
+
+// CHECK-NOT: @_ZNW3mod5Thing3OneE = {{.*}}constant
+// CHECK: @_ZW3mod13ConstantValue ={{.*}}available_externally{{.*}} constant
+
+// Check that the middle end can optimize the program by the constant
information.
+// CHECK-NOT: @unneeded(
+
+// Check that the use of ConstantPtr won't get optimized incorrectly.
+// CHECK-LABEL: @_Z4use3v(
+// CHECK: @needed(
+
+// Check that the use of ConstantRef won't get optimized incorrectly.
+// CHECK-LABEL: @_Z4use4v(
+// CHECK: @needed(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 34ba1c0 - [NFC] [Serialization] Emit Name for DECL_EXPORT
Author: Chuanqi Xu
Date: 2024-05-28T14:27:48+08:00
New Revision: 34ba1c043af0c3bbcbc1c9e66fbcc6509e4b8e9d
URL:
https://github.com/llvm/llvm-project/commit/34ba1c043af0c3bbcbc1c9e66fbcc6509e4b8e9d
DIFF:
https://github.com/llvm/llvm-project/commit/34ba1c043af0c3bbcbc1c9e66fbcc6509e4b8e9d.diff
LOG: [NFC] [Serialization] Emit Name for DECL_EXPORT
Added:
Modified:
clang/lib/Serialization/ASTWriter.cpp
clang/test/Modules/no-implicit-declarations.cppm
Removed:
diff --git a/clang/lib/Serialization/ASTWriter.cpp
b/clang/lib/Serialization/ASTWriter.cpp
index a85cd94fd5b5a..dd548fabfd955 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -1049,6 +1049,7 @@ void ASTWriter::WriteBlockInfoBlock() {
RECORD(DECL_UNRESOLVED_USING_VALUE);
RECORD(DECL_UNRESOLVED_USING_TYPENAME);
RECORD(DECL_LINKAGE_SPEC);
+ RECORD(DECL_EXPORT);
RECORD(DECL_CXX_RECORD);
RECORD(DECL_CXX_METHOD);
RECORD(DECL_CXX_CONSTRUCTOR);
diff --git a/clang/test/Modules/no-implicit-declarations.cppm
b/clang/test/Modules/no-implicit-declarations.cppm
index 319d3a432ea23..79c3c5e76f63e 100644
--- a/clang/test/Modules/no-implicit-declarations.cppm
+++ b/clang/test/Modules/no-implicit-declarations.cppm
@@ -17,7 +17,7 @@ export int a = 43;
// CHECK: https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/91007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,88 @@
+//===--- CIRGenAction.cpp - LLVM Code generation Frontend Action -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/CIRFrontendAction/CIRGenAction.h"
+#include "clang/CIR/CIRGenerator.h"
+#include "clang/Frontend/CompilerInstance.h"
+
+#include "mlir/IR/MLIRContext.h"
+#include "mlir/IR/OwningOpRef.h"
+
+using namespace cir;
+using namespace clang;
+
+namespace cir {
+
+class CIRGenConsumer : public clang::ASTConsumer {
+
+ virtual void anchor();
+
+ [[maybe_unused]] CIRGenAction::OutputType action;
+
+ [[maybe_unused]] DiagnosticsEngine
+ [[maybe_unused]] const HeaderSearchOptions
+ [[maybe_unused]] const CodeGenOptions
+ [[maybe_unused]] const TargetOptions
+ [[maybe_unused]] const LangOptions
+ [[maybe_unused]] const FrontendOptions
+
+ std::unique_ptr outputStream;
+
+ [[maybe_unused]] ASTContext *astContext{nullptr};
+ IntrusiveRefCntPtr FS;
+ std::unique_ptr gen;
+
+public:
+ CIRGenConsumer(CIRGenAction::OutputType action,
+ DiagnosticsEngine ,
+ IntrusiveRefCntPtr VFS,
+ const HeaderSearchOptions ,
+ const CodeGenOptions ,
+ const TargetOptions ,
+ const LangOptions ,
+ const FrontendOptions ,
+ std::unique_ptr os)
+ : action(action), diagnosticsEngine(diagnosticsEngine),
+headerSearchOptions(headerSearchOptions),
+codeGenOptions(codeGenOptions), targetOptions(targetOptions),
+langOptions(langOptions), feOptions(feOptions),
+outputStream(std::move(os)), FS(VFS),
+gen(std::make_unique(diagnosticsEngine, std::move(VFS),
+ codeGenOptions)) {}
+
+ bool HandleTopLevelDecl(DeclGroupRef D) override {
+gen->HandleTopLevelDecl(D);
+return true;
+ }
+};
+} // namespace cir
+
+void CIRGenConsumer::anchor() {}
+
+CIRGenAction::CIRGenAction(OutputType act, mlir::MLIRContext *mlirContext)
+: mlirContext(mlirContext ? mlirContext : new mlir::MLIRContext),
+ action(act) {}
+
+CIRGenAction::~CIRGenAction() { mlirModule.release(); }
+
+std::unique_ptr
+CIRGenAction::CreateASTConsumer(CompilerInstance , StringRef inputFile) {
+ auto out = ci.takeOutputStream();
+
+ auto Result = std::make_unique(
+ action, ci.getDiagnostics(), (),
+ ci.getHeaderSearchOpts(), ci.getCodeGenOpts(), ci.getTargetOpts(),
+ ci.getLangOpts(), ci.getFrontendOpts(), std::move(out));
+ cgConsumer = Result.get();
+
+ return std::move(Result);
ChuanqiXu9 wrote:
It may be pessimizing move
```suggestion
return Result;
```
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,59 @@
+//===- CIRGenerator.h - CIR Generation from Clang AST
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file declares a simple interface to perform CIR generation from Clang
+// AST
+//
+//===--===//
+
+#ifndef CLANG_CIRGENERATOR_H_
+#define CLANG_CIRGENERATOR_H_
+
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/DeclGroup.h"
+#include "clang/Basic/CodeGenOptions.h"
+#include "clang/Basic/Diagnostic.h"
+
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/Support/VirtualFileSystem.h"
+
+#include
+
+namespace mlir {
+class MLIRContext;
+} // namespace mlir
+namespace cir {
+class CIRGenModule;
+
+class CIRGenerator : public clang::ASTConsumer {
+ virtual void anchor();
+ clang::DiagnosticsEngine
+ clang::ASTContext *astCtx;
+ llvm::IntrusiveRefCntPtr
+ fs; // Only used for debug info.
+
+ const clang::CodeGenOptions codeGenOpts; // Intentionally copied in.
ChuanqiXu9 wrote:
It may be helpful to add a comment to explain why this is intentionally copied
in?
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,61 @@ +//=== CIRGenAction.h - CIR Code Generation Frontend Action -*- C++ -*--===// ChuanqiXu9 wrote: Should we move this header to `CIR` or `FrontendAction`? Currently it lives in `CIRFrontendAction` but its implementation file lives in `FrontendAction` https://github.com/llvm/llvm-project/pull/91007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -42,6 +47,14 @@ CreateFrontendBaseAction(CompilerInstance ) {
StringRef Action("unknown");
(void)Action;
+ auto UseCIR = CI.getFrontendOpts().UseClangIRPipeline;
+ auto Act = CI.getFrontendOpts().ProgramAction;
+ auto EmitsCIR = Act == EmitCIR;
+
+ if (!UseCIR && EmitsCIR)
+llvm::report_fatal_error(
+"-emit-cir and -emit-cir-only only valid when using -fclangir");
ChuanqiXu9 wrote:
What is `-emit-cir-only`? Should we remove that?
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,88 @@
+//===--- CIRGenAction.cpp - LLVM Code generation Frontend Action -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/CIRFrontendAction/CIRGenAction.h"
+#include "clang/CIR/CIRGenerator.h"
+#include "clang/Frontend/CompilerInstance.h"
+
+#include "mlir/IR/MLIRContext.h"
+#include "mlir/IR/OwningOpRef.h"
+
+using namespace cir;
+using namespace clang;
+
+namespace cir {
+
+class CIRGenConsumer : public clang::ASTConsumer {
+
+ virtual void anchor();
+
+ [[maybe_unused]] CIRGenAction::OutputType action;
+
+ [[maybe_unused]] DiagnosticsEngine
+ [[maybe_unused]] const HeaderSearchOptions
+ [[maybe_unused]] const CodeGenOptions
+ [[maybe_unused]] const TargetOptions
+ [[maybe_unused]] const LangOptions
+ [[maybe_unused]] const FrontendOptions
+
+ std::unique_ptr outputStream;
+
+ [[maybe_unused]] ASTContext *astContext{nullptr};
+ IntrusiveRefCntPtr FS;
+ std::unique_ptr gen;
+
+public:
+ CIRGenConsumer(CIRGenAction::OutputType action,
+ DiagnosticsEngine ,
+ IntrusiveRefCntPtr VFS,
+ const HeaderSearchOptions ,
+ const CodeGenOptions ,
+ const TargetOptions ,
+ const LangOptions ,
+ const FrontendOptions ,
+ std::unique_ptr os)
+ : action(action), diagnosticsEngine(diagnosticsEngine),
+headerSearchOptions(headerSearchOptions),
+codeGenOptions(codeGenOptions), targetOptions(targetOptions),
+langOptions(langOptions), feOptions(feOptions),
+outputStream(std::move(os)), FS(VFS),
+gen(std::make_unique(diagnosticsEngine, std::move(VFS),
+ codeGenOptions)) {}
+
+ bool HandleTopLevelDecl(DeclGroupRef D) override {
+gen->HandleTopLevelDecl(D);
+return true;
+ }
+};
+} // namespace cir
+
+void CIRGenConsumer::anchor() {}
+
+CIRGenAction::CIRGenAction(OutputType act, mlir::MLIRContext *mlirContext)
+: mlirContext(mlirContext ? mlirContext : new mlir::MLIRContext),
+ action(act) {}
+
+CIRGenAction::~CIRGenAction() { mlirModule.release(); }
+
+std::unique_ptr
+CIRGenAction::CreateASTConsumer(CompilerInstance , StringRef inputFile) {
+ auto out = ci.takeOutputStream();
+
+ auto Result = std::make_unique(
+ action, ci.getDiagnostics(), (),
+ ci.getHeaderSearchOpts(), ci.getCodeGenOpts(), ci.getTargetOpts(),
+ ci.getLangOpts(), ci.getFrontendOpts(), std::move(out));
+ cgConsumer = Result.get();
ChuanqiXu9 wrote:
If I read correctly, `cgConsumer` is only used here? I guess it is needed in
following patches. But slightly odd.
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/91007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
https://github.com/ChuanqiXu9 commented: BTW, it will be helpful to create subscribing team to help people to get informed in time. (I just saw the patch accidently.) https://github.com/llvm/llvm-project/pull/91007 ___ 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)
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] [clang] Be const-correct with all uses of `Module *`. (PR #93493)
ChuanqiXu9 wrote: > > Can you make sure that at every place this PR touches `const` makes sense? > > I found out recently that we can be quite good at pretending that something > > is `const`, all the way down until we realize we need a `const_cast`, > > because modification is required in that one place. > > I'm not quite sure I understand the question. This PR doesn't add any > `const_cast`, and `const` is checked by the compiler so a successful build > shows that we're never modifying something declared `const`. What additional > work are you wanting? The question is that it may be fine to be `const` today but it becomes not the case later. So we may have to make const function back to non-const function again. So one style to do such things is to understand that the new `const` decorated places are meant to be `const`. Otherwise I'll suggest to only mark the places that need to be change by the following patch as `const`. https://github.com/llvm/llvm-project/pull/93493 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [libcxx] [clang][Modules] Remove unnecessary includes of `Module.h` (PR #93417)
@@ -159,7 +159,8 @@ class APINotesManager {
ArrayRef getCurrentModuleReaders() const {
bool HasPublic = CurrentModuleReaders[ReaderKind::Public];
bool HasPrivate = CurrentModuleReaders[ReaderKind::Private];
-assert((!HasPrivate || HasPublic) && "private module requires public
module");
+assert((!HasPrivate || HasPublic) &&
+ "private module requires public module");
ChuanqiXu9 wrote:
Yes, we prefer to format the changed line only. Otherwise the backporting may
be problematic. And git blaming will be harder.
One possible way maybe:
> git diff -U0 --no-color --relative HEAD^ |
> clang/tools/clang-format/clang-format-diff.py -p1 -i
https://github.com/llvm/llvm-project/pull/93417
___
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)
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
[clang] [serialization] no transitive decl change (PR #92083)
ChuanqiXu9 wrote: Thanks for reviewing : ) https://github.com/llvm/llvm-project/pull/92083 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [serialization] no transitive decl change (PR #92083)
@@ -111,6 +109,28 @@ void *Decl::operator new(std::size_t Size, const
ASTContext ,
return ::operator new(Size + Extra, Ctx);
}
+GlobalDeclID Decl::getGlobalID() const {
+ if (!isFromASTFile())
+return GlobalDeclID();
+ uint64_t ID = *((const uint64_t *)this - 1);
ChuanqiXu9 wrote:
Previously, for deserialized declaration, clang would apply an additional 64
bits for it. And clang would store the global declaration ID at the lower 32
bits and store the module index at the higher 32 bits.
And after this patch, we extend the global declaration ID to 64 bits and I
don't want to apply additional spaces. So I decided to use the lower 48 bits to
store the declaration ID and the higher 16 bits to store the module index.
I added a comment here to clarify that.
And as I mentioned in the message, it looks like there are relationships
between module index and module file index (the upper 16 bits of the new
declaration ID). But on the one hand, I want to make the patch as simple as
possible to make it land faster. On the other hand, it looks like 16 bits is
enough for both module index and module file index. And we can't get any
improvements by merge it. So I just leave it as is.
https://github.com/llvm/llvm-project/pull/92083
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [serialization] no transitive decl change (PR #92083)
@@ -7802,20 +7800,31 @@ Decl *ASTReader::GetDecl(GlobalDeclID ID) {
LocalDeclID ASTReader::mapGlobalIDToModuleFileGlobalID(ModuleFile ,
GlobalDeclID GlobalID) {
- DeclID ID = GlobalID.get();
- if (ID < NUM_PREDEF_DECL_IDS)
+ if (GlobalID.get() < NUM_PREDEF_DECL_IDS)
+return LocalDeclID(GlobalID.get());
+
+ if (!M.ModuleOffsetMap.empty())
+ReadModuleOffsetMap(M);
+
+ ModuleFile *Owner = getOwningModuleFile(GlobalID);
+ DeclID ID = GlobalID.getLocalDeclIndex();
+
+ if (Owner == ) {
+ID += NUM_PREDEF_DECL_IDS;
return LocalDeclID(ID);
+ }
- GlobalDeclMapType::const_iterator I = GlobalDeclMap.find(GlobalID);
- assert(I != GlobalDeclMap.end() && "Corrupted global declaration map");
- ModuleFile *Owner = I->second;
+ uint64_t OrignalModuleFileIndex = 0;
+ for (unsigned I = 0; I < M.DependentModules.size(); I++)
ChuanqiXu9 wrote:
Done in
https://github.com/llvm/llvm-project/commit/b590ba73a76609bace9949ea8195d2ee8213cb3f
https://github.com/llvm/llvm-project/pull/92083
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [serialization] no transitive decl change (PR #92083)
@@ -255,6 +255,12 @@ class DeclOffset {
}
};
+// The unaligned decl ID used in the Blobs of bistreams.
+using unalighed_decl_id_t =
ChuanqiXu9 wrote:
Nice catch! Done in
https://github.com/llvm/llvm-project/commit/e73e4951b20c70f24354e2a2820876c818dcaee3
https://github.com/llvm/llvm-project/pull/92083
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [serialization] no transitive decl change (PR #92083)
@@ -124,6 +130,15 @@ class DeclIDBase {
bool isInvalid() const { return ID == PREDEF_DECL_NULL_ID; }
+ unsigned getModuleFileIndex() const { return ID >> 32; }
+
+ unsigned getLocalDeclIndex() const {
+// Implement it directly instead of calling `llvm::maskTrailingOnes` since
+// we don't want `MathExtras.h` to be inclued here.
ChuanqiXu9 wrote:
Maybe I just didn't want to create an implementation file for such a single
simple function. But I just realized I can append it in `DeclBase.cpp` just
like many other classes did. So done.
https://github.com/llvm/llvm-project/pull/92083
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [serialization] no transitive decl change (PR #92083)
https://github.com/ChuanqiXu9 updated
https://github.com/llvm/llvm-project/pull/92083
>From 8b7a650e128d6f2bfec9b0768169cf4aaa4af337 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Fri, 10 May 2024 15:36:31 +0800
Subject: [PATCH] [serialization] no transitive decl change
---
clang/include/clang/AST/DeclBase.h| 17 +-
clang/include/clang/AST/DeclID.h | 18 +-
.../include/clang/Serialization/ASTBitCodes.h | 6 +
clang/include/clang/Serialization/ASTReader.h | 36 ++--
.../include/clang/Serialization/ModuleFile.h | 18 +-
.../clang/Serialization/ModuleManager.h | 2 +-
clang/lib/AST/DeclBase.cpp| 40 -
clang/lib/Serialization/ASTReader.cpp | 159 ++
clang/lib/Serialization/ASTReaderDecl.cpp | 12 +-
clang/lib/Serialization/ASTWriter.cpp | 7 +-
clang/lib/Serialization/ModuleFile.cpp| 3 +-
.../Modules/no-transitive-decls-change.cppm | 112
12 files changed, 283 insertions(+), 147 deletions(-)
create mode 100644 clang/test/Modules/no-transitive-decls-change.cppm
diff --git a/clang/include/clang/AST/DeclBase.h
b/clang/include/clang/AST/DeclBase.h
index e43e812cd9455..4bdf27aa99405 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -701,10 +701,7 @@ class alignas(8) Decl {
/// Set the owning module ID. This may only be called for
/// deserialized Decls.
- void setOwningModuleID(unsigned ID) {
-assert(isFromASTFile() && "Only works on a deserialized declaration");
-*((unsigned*)this - 2) = ID;
- }
+ void setOwningModuleID(unsigned ID);
public:
/// Determine the availability of the given declaration.
@@ -777,19 +774,11 @@ class alignas(8) Decl {
/// Retrieve the global declaration ID associated with this
/// declaration, which specifies where this Decl was loaded from.
- GlobalDeclID getGlobalID() const {
-if (isFromASTFile())
- return (*((const GlobalDeclID *)this - 1));
-return GlobalDeclID();
- }
+ GlobalDeclID getGlobalID() const;
/// Retrieve the global ID of the module that owns this particular
/// declaration.
- unsigned getOwningModuleID() const {
-if (isFromASTFile())
- return *((const unsigned*)this - 2);
-return 0;
- }
+ unsigned getOwningModuleID() const;
private:
Module *getOwningModuleSlow() const;
diff --git a/clang/include/clang/AST/DeclID.h b/clang/include/clang/AST/DeclID.h
index 614ba06b63860..32d2ed41a374a 100644
--- a/clang/include/clang/AST/DeclID.h
+++ b/clang/include/clang/AST/DeclID.h
@@ -19,6 +19,8 @@
#include "llvm/ADT/DenseMapInfo.h"
#include "llvm/ADT/iterator.h"
+#include
+
namespace clang {
/// Predefined declaration IDs.
@@ -107,12 +109,16 @@ class DeclIDBase {
///
/// DeclID should only be used directly in serialization. All other users
/// should use LocalDeclID or GlobalDeclID.
- using DeclID = uint32_t;
+ using DeclID = uint64_t;
protected:
DeclIDBase() : ID(PREDEF_DECL_NULL_ID) {}
explicit DeclIDBase(DeclID ID) : ID(ID) {}
+ explicit DeclIDBase(unsigned LocalID, unsigned ModuleFileIndex) {
+ID = (DeclID)LocalID | ((DeclID)ModuleFileIndex << 32);
+ }
+
public:
DeclID get() const { return ID; }
@@ -124,6 +130,10 @@ class DeclIDBase {
bool isInvalid() const { return ID == PREDEF_DECL_NULL_ID; }
+ unsigned getModuleFileIndex() const { return ID >> 32; }
+
+ unsigned getLocalDeclIndex() const;
+
friend bool operator==(const DeclIDBase , const DeclIDBase ) {
return LHS.ID == RHS.ID;
}
@@ -156,6 +166,9 @@ class LocalDeclID : public DeclIDBase {
LocalDeclID(PredefinedDeclIDs ID) : Base(ID) {}
explicit LocalDeclID(DeclID ID) : Base(ID) {}
+ explicit LocalDeclID(unsigned LocalID, unsigned ModuleFileIndex)
+ : Base(LocalID, ModuleFileIndex) {}
+
LocalDeclID ++() {
++ID;
return *this;
@@ -175,6 +188,9 @@ class GlobalDeclID : public DeclIDBase {
GlobalDeclID() : Base() {}
explicit GlobalDeclID(DeclID ID) : Base(ID) {}
+ explicit GlobalDeclID(unsigned LocalID, unsigned ModuleFileIndex)
+ : Base(LocalID, ModuleFileIndex) {}
+
// For DeclIDIterator to be able to convert a GlobalDeclID
// to a LocalDeclID.
explicit operator LocalDeclID() const { return LocalDeclID(this->ID); }
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h
b/clang/include/clang/Serialization/ASTBitCodes.h
index fe1bd47348be1..9e4b21baa7d20 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -255,6 +255,12 @@ class DeclOffset {
}
};
+// The unaligned decl ID used in the Blobs of bistreams.
+using unaligned_decl_id_t =
+llvm::support::detail::packed_endian_specific_integral<
+serialization::DeclID, llvm::endianness::native,
+llvm::support::unaligned>;
+
/// The number of predefined preprocessed entity IDs.
const unsigned int NUM_PREDEF_PP_ENTITY_IDS = 1;
[clang] b590ba7 - [NFC] Rename 'DependentModules' in ModuleFile to `TransitiveImports`
Author: Chuanqi Xu
Date: 2024-05-27T13:53:38+08:00
New Revision: b590ba73a76609bace9949ea8195d2ee8213cb3f
URL:
https://github.com/llvm/llvm-project/commit/b590ba73a76609bace9949ea8195d2ee8213cb3f
DIFF:
https://github.com/llvm/llvm-project/commit/b590ba73a76609bace9949ea8195d2ee8213cb3f.diff
LOG: [NFC] Rename 'DependentModules' in ModuleFile to `TransitiveImports`
Required in the review process of
https://github.com/llvm/llvm-project/pull/92083
Added:
Modified:
clang/include/clang/Serialization/ASTReader.h
clang/include/clang/Serialization/ModuleFile.h
clang/lib/Serialization/ASTReader.cpp
Removed:
diff --git a/clang/include/clang/Serialization/ASTReader.h
b/clang/include/clang/Serialization/ASTReader.h
index 75e41ea91715b..4ece4593f0738 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -2246,7 +2246,7 @@ class ASTReader
auto [Loc, ModuleFileIndex] = ReadUntranslatedSourceLocation(Raw, Seq);
ModuleFile *OwningModuleFile =
-ModuleFileIndex == 0 ? : MF.DependentModules[ModuleFileIndex - 1];
+ModuleFileIndex == 0 ? : MF.TransitiveImports[ModuleFileIndex - 1];
assert(!SourceMgr.isLoadedSourceLocation(Loc) &&
"Run out source location space");
diff --git a/clang/include/clang/Serialization/ModuleFile.h
b/clang/include/clang/Serialization/ModuleFile.h
index 7d8cbe3d40f56..992d26a8b88c1 100644
--- a/clang/include/clang/Serialization/ModuleFile.h
+++ b/clang/include/clang/Serialization/ModuleFile.h
@@ -513,11 +513,11 @@ class ModuleFile {
/// List of modules which this modules dependent on. Different
/// from `Imports`, this includes indirectly imported modules too.
- /// The order of DependentModules is significant. It should keep
+ /// The order of TransitiveImports is significant. It should keep
/// the same order with that module file manager when we write
/// the current module file. The value of the member will be initialized
/// in `ASTReader::ReadModuleOffsetMap`.
- llvm::SmallVector DependentModules;
+ llvm::SmallVector TransitiveImports;
/// Determine whether this module was directly imported at
/// any point during translation.
diff --git a/clang/lib/Serialization/ASTReader.cpp
b/clang/lib/Serialization/ASTReader.cpp
index 54414af3a646e..4a6e1d23161be 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -4059,7 +4059,7 @@ void ASTReader::ReadModuleOffsetMap(ModuleFile ) const {
RemapBuilder DeclRemap(F.DeclRemap);
RemapBuilder TypeRemap(F.TypeRemap);
- auto = F.DependentModules;
+ auto = F.TransitiveImports;
assert(ImportedModuleVector.empty());
while (Data < DataEnd) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] e73e495 - [NFC] Fix typo unalighed_decl_id_t
Author: Chuanqi Xu
Date: 2024-05-27T13:48:00+08:00
New Revision: e73e4951b20c70f24354e2a2820876c818dcaee3
URL:
https://github.com/llvm/llvm-project/commit/e73e4951b20c70f24354e2a2820876c818dcaee3
DIFF:
https://github.com/llvm/llvm-project/commit/e73e4951b20c70f24354e2a2820876c818dcaee3.diff
LOG: [NFC] Fix typo unalighed_decl_id_t
It should be unalignhed_decl_id_t
Added:
Modified:
clang/include/clang/Serialization/ASTReader.h
clang/lib/Serialization/ASTReader.cpp
Removed:
diff --git a/clang/include/clang/Serialization/ASTReader.h
b/clang/include/clang/Serialization/ASTReader.h
index 1bb5fa27a2419..75e41ea91715b 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -601,11 +601,11 @@ class ASTReader
/// An array of lexical contents of a declaration context, as a sequence of
/// Decl::Kind, DeclID pairs.
- using unalighed_decl_id_t =
+ using unaligned_decl_id_t =
llvm::support::detail::packed_endian_specific_integral<
serialization::DeclID, llvm::endianness::native,
llvm::support::unaligned>;
- using LexicalContents = ArrayRef;
+ using LexicalContents = ArrayRef;
/// Map from a DeclContext to its lexical contents.
llvm::DenseMap>
diff --git a/clang/lib/Serialization/ASTReader.cpp
b/clang/lib/Serialization/ASTReader.cpp
index d7fc6697eaf74..54414af3a646e 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -1264,7 +1264,7 @@ bool ASTReader::ReadLexicalDeclContextStorage(ModuleFile
,
if (!Lex.first) {
Lex = std::make_pair(
, llvm::ArrayRef(
-reinterpret_cast(Blob.data()),
+reinterpret_cast(Blob.data()),
Blob.size() / sizeof(DeclID)));
}
DC->setHasExternalLexicalStorage(true);
@@ -3401,7 +3401,7 @@ llvm::Error ASTReader::ReadASTBlock(ModuleFile ,
case TU_UPDATE_LEXICAL: {
DeclContext *TU = ContextObj->getTranslationUnitDecl();
LexicalContents Contents(
- reinterpret_cast(Blob.data()),
+ reinterpret_cast(Blob.data()),
static_cast(Blob.size() / sizeof(DeclID)));
TULexicalDecls.push_back(std::make_pair(, Contents));
TU->setHasExternalLexicalStorage(true);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
ChuanqiXu9 wrote: Oh, I don't know why I didn't get this in files page so I missed this. But since we can't get rid of writing/reading the modules actually in `ModulesBuilder` (Or it is pretty hard). Then it looks not so worthy to introduce the layer. https://github.com/llvm/llvm-project/pull/66462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,339 @@
+//===- ModulesBuilder.cpp *-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ModulesBuilder.h"
+#include "PrerequisiteModules.h"
+#include "support/Logger.h"
+
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/FrontendActions.h"
+
+namespace clang {
+namespace clangd {
+
+namespace {
+
+/// Get or create a path to store module files. Generally it should be:
+///
+/// project_root/.cache/clangd/module_files/{RequiredPrefixDir}/.
+///
+/// \param MainFile is used to get the root of the project from global
+/// compilation database. \param RequiredPrefixDir is used to get the user
+/// defined prefix for module files. This is useful when we want to seperate
+/// module files. e.g., we want to build module files for the same module unit
+/// `a.cppm` with 2 different users `b.cpp` and `c.cpp` and we don't want the
+/// module file for `b.cpp` be conflict with the module files for `c.cpp`. Then
+/// we can put the 2 module files into different dirs like:
+///
+/// project_root/.cache/clangd/module_files/b.cpp/a.pcm
+/// project_root/.cache/clangd/module_files/c.cpp/a.pcm
+llvm::SmallString<256> getModuleFilesPath(PathRef MainFile,
+ const GlobalCompilationDatabase ,
+ StringRef RequiredPrefixDir) {
+ std::optional PI = CDB.getProjectInfo(MainFile);
+ if (!PI)
+return {};
+
+ // FIXME: PI->SourceRoot may be empty, depending on the CDB strategy.
+ llvm::SmallString<256> Result(PI->SourceRoot);
+
+ llvm::sys::path::append(Result, ".cache");
+ llvm::sys::path::append(Result, "clangd");
+ llvm::sys::path::append(Result, "module_files");
+
+ llvm::sys::path::append(Result, RequiredPrefixDir);
+
+ llvm::sys::fs::create_directories(Result, /*IgnoreExisting=*/true);
+
+ return Result;
+}
+
+/// Get the absolute path for the filename from the compile command.
+llvm::SmallString<128> getAbsolutePath(const tooling::CompileCommand ) {
+ llvm::SmallString<128> AbsolutePath;
+ if (llvm::sys::path::is_absolute(Cmd.Filename)) {
+AbsolutePath = Cmd.Filename;
+ } else {
+AbsolutePath = Cmd.Directory;
+llvm::sys::path::append(AbsolutePath, Cmd.Filename);
+llvm::sys::path::remove_dots(AbsolutePath, true);
+ }
+ return AbsolutePath;
+}
+
+/// Get a unique module file path under \param ModuleFilesPrefix.
+std::string getUniqueModuleFilePath(StringRef ModuleName,
+PathRef ModuleFilesPrefix) {
+ llvm::SmallString<256> ModuleFilePattern(ModuleFilesPrefix);
+ auto [PrimaryModuleName, PartitionName] = ModuleName.split(':');
+ llvm::sys::path::append(ModuleFilePattern, PrimaryModuleName);
+ if (!PartitionName.empty()) {
+ModuleFilePattern.append("-");
+ModuleFilePattern.append(PartitionName);
+ }
+
+ ModuleFilePattern.append("-%%-%%-%%-%%-%%-%%");
+ ModuleFilePattern.append(".pcm");
+
+ llvm::SmallString<256> ModuleFilePath;
+ llvm::sys::fs::createUniquePath(ModuleFilePattern, ModuleFilePath,
+ /*MakeAbsolute=*/false);
+
+ return (std::string)ModuleFilePath;
+}
+} // namespace
+
+bool ModulesBuilder::buildModuleFile(StringRef ModuleName,
+ const ThreadsafeFS *TFS,
+ std::shared_ptr MDB,
+ PathRef ModuleFilesPrefix,
+ PrerequisiteModules ) {
+ if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName))
+return true;
+
+ PathRef ModuleUnitFileName = MDB->getSourceForModuleName(ModuleName);
+ /// It is possible that we're meeting third party modules (modules whose
+ /// source are not in the project. e.g, the std module may be a third-party
+ /// module for most project) or something wrong with the implementation of
+ /// ProjectModules.
+ /// FIXME: How should we treat third party modules here? If we want to ignore
+ /// third party modules, we should return true instead of false here.
+ /// Currently we simply bail out.
+ if (ModuleUnitFileName.empty())
+return false;
+
+ for (auto : MDB->getRequiredModules(ModuleUnitFileName))
{
+// Return early if there are errors building the module file.
+if (!buildModuleFile(RequiredModuleName, TFS, MDB, ModuleFilesPrefix,
+ BuiltModuleFiles)) {
+ log("Failed to build module {0}", RequiredModuleName);
+ return false;
+}
+ }
+
+ auto Cmd = CDB.getCompileCommand(ModuleUnitFileName);
+ if (!Cmd)
+return false;
+
+ std::string ModuleFileName =
+ getUniqueModuleFilePath(ModuleName, ModuleFilesPrefix);
+ Cmd->Output =
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,62 @@
+//===-- ProjectModules.h -*-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ProjectModules.h"
+
+namespace clang {
+namespace clangd {
+
+/// TODO: The existing `ScanningAllProjectModules` is not efficient. See the
+/// comments in ModuleDependencyScanner for detail.
+///
+/// In the future, we wish the build system can provide a well design
+/// compilation database for modules then we can query that new compilation
+/// database directly. Or we need to have a global long-live scanner to detect
+/// the state of each file.
+class ScanningAllProjectModules : public ProjectModules {
+public:
+ ScanningAllProjectModules(std::vector &,
+const GlobalCompilationDatabase ,
+const ThreadsafeFS )
+ : AllFiles(std::move(AllFiles)), Scanner(CDB, TFS) {}
+
+ ~ScanningAllProjectModules() override = default;
+
+ std::vector getRequiredModules(PathRef File) override {
+return Scanner.getRequiredModules(File);
+ }
+
+ /// RequiredSourceFile is not used intentionally. See the comments of
+ /// ModuleDependencyScanner for detail.
+ PathRef
+ getSourceForModuleName(StringRef ModuleName,
+ PathRef RequiredSourceFile = PathRef()) override {
+if (!Scanner.isGlobalScanned())
+ Scanner.globalScan(AllFiles);
+
+return Scanner.getSourceForModuleName(ModuleName);
+ }
+
+private:
+ std::vector AllFiles;
+
+ ModuleDependencyScanner Scanner;
ChuanqiXu9 wrote:
Done. Good idea. I like it.
https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,339 @@
+//===- ModulesBuilder.cpp *-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ModulesBuilder.h"
+#include "PrerequisiteModules.h"
+#include "support/Logger.h"
+
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/FrontendActions.h"
+
+namespace clang {
+namespace clangd {
+
+namespace {
+
+/// Get or create a path to store module files. Generally it should be:
+///
+/// project_root/.cache/clangd/module_files/{RequiredPrefixDir}/.
+///
+/// \param MainFile is used to get the root of the project from global
+/// compilation database. \param RequiredPrefixDir is used to get the user
+/// defined prefix for module files. This is useful when we want to seperate
+/// module files. e.g., we want to build module files for the same module unit
+/// `a.cppm` with 2 different users `b.cpp` and `c.cpp` and we don't want the
+/// module file for `b.cpp` be conflict with the module files for `c.cpp`. Then
+/// we can put the 2 module files into different dirs like:
+///
+/// project_root/.cache/clangd/module_files/b.cpp/a.pcm
+/// project_root/.cache/clangd/module_files/c.cpp/a.pcm
+llvm::SmallString<256> getModuleFilesPath(PathRef MainFile,
+ const GlobalCompilationDatabase ,
+ StringRef RequiredPrefixDir) {
+ std::optional PI = CDB.getProjectInfo(MainFile);
+ if (!PI)
+return {};
+
+ // FIXME: PI->SourceRoot may be empty, depending on the CDB strategy.
+ llvm::SmallString<256> Result(PI->SourceRoot);
+
+ llvm::sys::path::append(Result, ".cache");
+ llvm::sys::path::append(Result, "clangd");
+ llvm::sys::path::append(Result, "module_files");
+
+ llvm::sys::path::append(Result, RequiredPrefixDir);
+
+ llvm::sys::fs::create_directories(Result, /*IgnoreExisting=*/true);
+
+ return Result;
+}
+
+/// Get the absolute path for the filename from the compile command.
+llvm::SmallString<128> getAbsolutePath(const tooling::CompileCommand ) {
+ llvm::SmallString<128> AbsolutePath;
+ if (llvm::sys::path::is_absolute(Cmd.Filename)) {
+AbsolutePath = Cmd.Filename;
+ } else {
+AbsolutePath = Cmd.Directory;
+llvm::sys::path::append(AbsolutePath, Cmd.Filename);
+llvm::sys::path::remove_dots(AbsolutePath, true);
+ }
+ return AbsolutePath;
+}
+
+/// Get a unique module file path under \param ModuleFilesPrefix.
+std::string getUniqueModuleFilePath(StringRef ModuleName,
+PathRef ModuleFilesPrefix) {
+ llvm::SmallString<256> ModuleFilePattern(ModuleFilesPrefix);
+ auto [PrimaryModuleName, PartitionName] = ModuleName.split(':');
+ llvm::sys::path::append(ModuleFilePattern, PrimaryModuleName);
+ if (!PartitionName.empty()) {
+ModuleFilePattern.append("-");
+ModuleFilePattern.append(PartitionName);
+ }
+
+ ModuleFilePattern.append("-%%-%%-%%-%%-%%-%%");
+ ModuleFilePattern.append(".pcm");
+
+ llvm::SmallString<256> ModuleFilePath;
+ llvm::sys::fs::createUniquePath(ModuleFilePattern, ModuleFilePath,
+ /*MakeAbsolute=*/false);
+
+ return (std::string)ModuleFilePath;
+}
+} // namespace
+
+bool ModulesBuilder::buildModuleFile(StringRef ModuleName,
+ const ThreadsafeFS *TFS,
+ std::shared_ptr MDB,
+ PathRef ModuleFilesPrefix,
+ PrerequisiteModules ) {
+ if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName))
+return true;
+
+ PathRef ModuleUnitFileName = MDB->getSourceForModuleName(ModuleName);
+ /// It is possible that we're meeting third party modules (modules whose
+ /// source are not in the project. e.g, the std module may be a third-party
+ /// module for most project) or something wrong with the implementation of
+ /// ProjectModules.
+ /// FIXME: How should we treat third party modules here? If we want to ignore
+ /// third party modules, we should return true instead of false here.
+ /// Currently we simply bail out.
+ if (ModuleUnitFileName.empty())
+return false;
+
+ for (auto : MDB->getRequiredModules(ModuleUnitFileName))
{
+// Return early if there are errors building the module file.
+if (!buildModuleFile(RequiredModuleName, TFS, MDB, ModuleFilesPrefix,
+ BuiltModuleFiles)) {
+ log("Failed to build module {0}", RequiredModuleName);
+ return false;
+}
+ }
+
+ auto Cmd = CDB.getCompileCommand(ModuleUnitFileName);
+ if (!Cmd)
+return false;
+
+ std::string ModuleFileName =
+ getUniqueModuleFilePath(ModuleName, ModuleFilesPrefix);
+ Cmd->Output =
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,71 @@
+//===- ModulesBuilder.h --*-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// Experimental support for C++20 Modules.
+//
+// Currently we simplify the implementations by preventing reusing module files
+// across different versions and different source files. But this is clearly a
+// waste of time and space in the end of the day.
+//
+// FIXME: Supporting reusing module files across different versions and
+// different source files.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULES_BUILDER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULES_BUILDER_H
+
+#include "GlobalCompilationDatabase.h"
+#include "ProjectModules.h"
+
+#include "support/Path.h"
+#include "support/ThreadsafeFS.h"
+
+#include "llvm/ADT/SmallString.h"
+
+#include
+
+namespace clang {
+namespace clangd {
+
+class PrerequisiteModules;
+
+/// This class handles building module files for a given source file.
+///
+/// In the future, we want the class to manage the module files acorss
+/// different versions and different source files.
+class ModulesBuilder {
+public:
+ ModulesBuilder() = delete;
+
+ ModulesBuilder(const GlobalCompilationDatabase ) : CDB(CDB) {}
+
+ ModulesBuilder(const ModulesBuilder &) = delete;
+ ModulesBuilder(ModulesBuilder &&) = delete;
+
+ ModulesBuilder =(const ModulesBuilder &) = delete;
+ ModulesBuilder =(ModulesBuilder &&) = delete;
ChuanqiXu9 wrote:
Primarily a conservative design. Since this is owned by the ClangdLSPServer,
and it looks like we won't copy or move the server. Then it is safe to mark it
as non-copyable and non-movable. It may be easy to convert a non-moveable and
non-copyable to the opposite. But it is not the case vice versa.
https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,71 @@
+//===- ModulesBuilder.h --*-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// Experimental support for C++20 Modules.
+//
+// Currently we simplify the implementations by preventing reusing module files
+// across different versions and different source files. But this is clearly a
+// waste of time and space in the end of the day.
+//
+// FIXME: Supporting reusing module files across different versions and
+// different source files.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULES_BUILDER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULES_BUILDER_H
+
+#include "GlobalCompilationDatabase.h"
+#include "ProjectModules.h"
+
+#include "support/Path.h"
+#include "support/ThreadsafeFS.h"
+
+#include "llvm/ADT/SmallString.h"
+
+#include
+
+namespace clang {
+namespace clangd {
+
+class PrerequisiteModules;
+
+/// This class handles building module files for a given source file.
+///
+/// In the future, we want the class to manage the module files acorss
+/// different versions and different source files.
+class ModulesBuilder {
+public:
+ ModulesBuilder() = delete;
+
+ ModulesBuilder(const GlobalCompilationDatabase ) : CDB(CDB) {}
+
+ ModulesBuilder(const ModulesBuilder &) = delete;
+ ModulesBuilder(ModulesBuilder &&) = delete;
+
+ ModulesBuilder =(const ModulesBuilder &) = delete;
+ ModulesBuilder =(ModulesBuilder &&) = delete;
+
+ ~ModulesBuilder() = default;
+
+ std::unique_ptr
+ buildPrerequisiteModulesFor(PathRef File, const ThreadsafeFS *TFS);
+
+private:
+ bool buildModuleFile(StringRef ModuleName, const ThreadsafeFS *TFS,
ChuanqiXu9 wrote:
Done
https://github.com/llvm/llvm-project/pull/66462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
