Author: Chuanqi Xu Date: 2024-04-30T16:33:34+08:00 New Revision: b2b463bd8f6b21f040b80c4493682cf74f8dced5
URL: https://github.com/llvm/llvm-project/commit/b2b463bd8f6b21f040b80c4493682cf74f8dced5 DIFF: https://github.com/llvm/llvm-project/commit/b2b463bd8f6b21f040b80c4493682cf74f8dced5.diff LOG: [C++20] [Modules] Add signature to the BMI recording export imported modules After https://github.com/llvm/llvm-project/pull/86912, for the following example, ``` export module A; export import B; ``` The generated BMI of `A` won't change if the source location in `A` changes. Further, we plan avoid more such changes. However, it is slightly problematic since `export import` should propagate all the changes. So this patch adds a signature to the BMI of C++20 modules so that we can propagate the changes correctly. Added: clang/test/Modules/force-transitive-changes.cppm Modified: clang/include/clang/Serialization/ASTWriter.h clang/lib/Serialization/ASTWriter.cpp clang/test/Modules/no-transitive-source-location-change.cppm Removed: ################################################################################ diff --git a/clang/include/clang/Serialization/ASTWriter.h b/clang/include/clang/Serialization/ASTWriter.h index 428bf6a5a791b3..921678d278d6e2 100644 --- a/clang/include/clang/Serialization/ASTWriter.h +++ b/clang/include/clang/Serialization/ASTWriter.h @@ -525,6 +525,7 @@ class ASTWriter : public ASTDeserializationListener, /// Calculate hash of the pcm content. std::pair<ASTFileSignature, ASTFileSignature> createSignature() const; + ASTFileSignature createSignatureForNamedModule() const; void WriteInputFiles(SourceManager &SourceMgr, HeaderSearchOptions &HSOpts); void WriteSourceManagerBlock(SourceManager &SourceMgr, diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 4d85f6eb10d232..c3fcd1a4df2368 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -1174,26 +1174,47 @@ ASTWriter::createSignature() const { return std::make_pair(ASTBlockHash, Signature); } +ASTFileSignature ASTWriter::createSignatureForNamedModule() const { + llvm::SHA1 Hasher; + Hasher.update(StringRef(Buffer.data(), Buffer.size())); + + assert(WritingModule); + assert(WritingModule->isNamedModule()); + + // We need to combine all the export imported modules no matter + // we used it or not. + for (auto [ExportImported, _] : WritingModule->Exports) + Hasher.update(ExportImported->Signature); + + return ASTFileSignature::create(Hasher.result()); +} + +static void BackpatchSignatureAt(llvm::BitstreamWriter &Stream, + const ASTFileSignature &S, uint64_t BitNo) { + for (uint8_t Byte : S) { + Stream.BackpatchByte(BitNo, Byte); + BitNo += 8; + } +} + ASTFileSignature ASTWriter::backpatchSignature() { + if (isWritingStdCXXNamedModules()) { + ASTFileSignature Signature = createSignatureForNamedModule(); + BackpatchSignatureAt(Stream, Signature, SignatureOffset); + return Signature; + } + if (!WritingModule || !PP->getHeaderSearchInfo().getHeaderSearchOpts().ModulesHashContent) return {}; // For implicit modules, write the hash of the PCM as its signature. - - auto BackpatchSignatureAt = [&](const ASTFileSignature &S, uint64_t BitNo) { - for (uint8_t Byte : S) { - Stream.BackpatchByte(BitNo, Byte); - BitNo += 8; - } - }; - ASTFileSignature ASTBlockHash; ASTFileSignature Signature; std::tie(ASTBlockHash, Signature) = createSignature(); - BackpatchSignatureAt(ASTBlockHash, ASTBlockHashOffset); - BackpatchSignatureAt(Signature, SignatureOffset); + BackpatchSignatureAt(Stream, ASTBlockHash, ASTBlockHashOffset); + BackpatchSignatureAt(Stream, Signature, SignatureOffset); return Signature; } @@ -1210,9 +1231,11 @@ void ASTWriter::writeUnhashedControlBlock(Preprocessor &PP, RecordData Record; Stream.EnterSubblock(UNHASHED_CONTROL_BLOCK_ID, 5); - // For implicit modules, write the hash of the PCM as its signature. - if (WritingModule && - PP.getHeaderSearchInfo().getHeaderSearchOpts().ModulesHashContent) { + // For implicit modules and C++20 named modules, write the hash of the PCM as + // its signature. + if (isWritingStdCXXNamedModules() || + (WritingModule && + PP.getHeaderSearchInfo().getHeaderSearchOpts().ModulesHashContent)) { // At this point, we don't know the actual signature of the file or the AST // block - we're only able to compute those at the end of the serialization // process. Let's store dummy signatures for now, and replace them with the @@ -1223,21 +1246,24 @@ void ASTWriter::writeUnhashedControlBlock(Preprocessor &PP, auto Dummy = ASTFileSignature::createDummy(); SmallString<128> Blob{Dummy.begin(), Dummy.end()}; - auto Abbrev = std::make_shared<BitCodeAbbrev>(); - Abbrev->Add(BitCodeAbbrevOp(AST_BLOCK_HASH)); - Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); - unsigned ASTBlockHashAbbrev = Stream.EmitAbbrev(std::move(Abbrev)); + // We don't need AST Block hash in named modules. + if (!isWritingStdCXXNamedModules()) { + auto Abbrev = std::make_shared<BitCodeAbbrev>(); + Abbrev->Add(BitCodeAbbrevOp(AST_BLOCK_HASH)); + Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); + unsigned ASTBlockHashAbbrev = Stream.EmitAbbrev(std::move(Abbrev)); - Abbrev = std::make_shared<BitCodeAbbrev>(); + Record.push_back(AST_BLOCK_HASH); + Stream.EmitRecordWithBlob(ASTBlockHashAbbrev, Record, Blob); + ASTBlockHashOffset = Stream.GetCurrentBitNo() - Blob.size() * 8; + Record.clear(); + } + + auto Abbrev = std::make_shared<BitCodeAbbrev>(); Abbrev->Add(BitCodeAbbrevOp(SIGNATURE)); Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); unsigned SignatureAbbrev = Stream.EmitAbbrev(std::move(Abbrev)); - Record.push_back(AST_BLOCK_HASH); - Stream.EmitRecordWithBlob(ASTBlockHashAbbrev, Record, Blob); - ASTBlockHashOffset = Stream.GetCurrentBitNo() - Blob.size() * 8; - Record.clear(); - Record.push_back(SIGNATURE); Stream.EmitRecordWithBlob(SignatureAbbrev, Record, Blob); SignatureOffset = Stream.GetCurrentBitNo() - Blob.size() * 8; diff --git a/clang/test/Modules/force-transitive-changes.cppm b/clang/test/Modules/force-transitive-changes.cppm new file mode 100644 index 00000000000000..5732d264d1d552 --- /dev/null +++ b/clang/test/Modules/force-transitive-changes.cppm @@ -0,0 +1,38 @@ +// Test that the changes from export imported modules and touched +// modules can be popullated as expected. +// +// RUN: rm -rf %t +// RUN: split-file %s %t +// RUN: cd %t +// +// RUN: %clang_cc1 -std=c++20 %t/A.cppm -emit-reduced-module-interface -o %t/A.pcm +// RUN: %clang_cc1 -std=c++20 %t/A.v1.cppm -emit-reduced-module-interface -o %t/A.v1.pcm + +// The BMI of B should change it export imports A, so all the change to A should be popullated +// to B. +// RUN: %clang_cc1 -std=c++20 %t/B.cppm -emit-reduced-module-interface -fmodule-file=A=%t/A.pcm \ +// RUN: -o %t/B.pcm +// RUN: %clang_cc1 -std=c++20 %t/B.cppm -emit-reduced-module-interface -fmodule-file=A=%t/A.v1.pcm \ +// RUN: -o %t/B.v1.pcm +// RUN: not diff %t/B.v1.pcm %t/B.pcm &> /dev/null + +//--- A.cppm +export module A; +export int funcA() { + return 43; +} + +//--- A.v1.cppm +export module A; + +export int funcA() { + return 43; +} + +//--- B.cppm +export module B; +export import A; + +export int funcB() { + return funcA(); +} diff --git a/clang/test/Modules/no-transitive-source-location-change.cppm b/clang/test/Modules/no-transitive-source-location-change.cppm index b876fbb6d9f0cf..83cf6fb4f684d0 100644 --- a/clang/test/Modules/no-transitive-source-location-change.cppm +++ b/clang/test/Modules/no-transitive-source-location-change.cppm @@ -6,8 +6,6 @@ // RUN: cd %t // // RUN: %clang_cc1 -std=c++20 %t/A.cppm -emit-module-interface -o %t/A.pcm - -// // RUN: %clang_cc1 -std=c++20 %t/A.v1.cppm -emit-module-interface -o %t/A.v1.pcm // // The BMI may not be the same since the source location diff ers. @@ -26,6 +24,31 @@ // RUN: %clang_cc1 -std=c++20 %t/C.cppm -emit-module-interface -fmodule-file=A=%t/A.v1.pcm \ // RUN: -o %t/C.v1.pcm // RUN: not diff %t/C.v1.pcm %t/C.pcm &> /dev/null +// +// RUN: rm -rf %t +// RUN: split-file %s %t +// RUN: cd %t +// +// Test again with reduced BMI. +// RUN: %clang_cc1 -std=c++20 %t/A.cppm -emit-reduced-module-interface -o %t/A.pcm +// RUN: %clang_cc1 -std=c++20 %t/A.v1.cppm -emit-reduced-module-interface -o %t/A.v1.pcm +// +// The BMI may not be the same since the source location diff ers. +// RUN: not diff %t/A.pcm %t/A.v1.pcm &> /dev/null +// +// The BMI of B shouldn't change since all the locations remain the same. +// RUN: %clang_cc1 -std=c++20 %t/B.cppm -emit-reduced-module-interface -fmodule-file=A=%t/A.pcm \ +// RUN: -o %t/B.pcm +// RUN: %clang_cc1 -std=c++20 %t/B.cppm -emit-reduced-module-interface -fmodule-file=A=%t/A.v1.pcm \ +// RUN: -o %t/B.v1.pcm +// RUN: diff %t/B.v1.pcm %t/B.pcm &> /dev/null +// +// The BMI of C may change since the locations for instantiations changes. +// RUN: %clang_cc1 -std=c++20 %t/C.cppm -emit-reduced-module-interface -fmodule-file=A=%t/A.pcm \ +// RUN: -o %t/C.pcm +// RUN: %clang_cc1 -std=c++20 %t/C.cppm -emit-reduced-module-interface -fmodule-file=A=%t/A.v1.pcm \ +// RUN: -o %t/C.v1.pcm +// RUN: not diff %t/C.v1.pcm %t/C.pcm &> /dev/null //--- A.cppm export module A; @@ -63,7 +86,7 @@ export int funcB() { //--- C.cppm export module C; import A; -export void testD() { +export inline void testD() { C<int> c; c.func(); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
