This revision was automatically updated to reflect the committed changes.
Closed by commit rG83f4c3af021c: [modules] Do not cache invalid state for
modules that we attempted to load. (authored by vsapsai).
Changed prior to commit:
https://reviews.llvm.org/D72860?vs=238562&id=238668#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72860/new/
https://reviews.llvm.org/D72860
Files:
clang/include/clang/Serialization/InMemoryModuleCache.h
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/lib/Serialization/InMemoryModuleCache.cpp
clang/lib/Serialization/ModuleManager.cpp
clang/test/Modules/Inputs/implicit-invalidate-chain/A.h
clang/test/Modules/Inputs/implicit-invalidate-chain/B.h
clang/test/Modules/Inputs/implicit-invalidate-chain/C.h
clang/test/Modules/Inputs/implicit-invalidate-chain/module.modulemap
clang/test/Modules/implicit-invalidate-chain.c
clang/unittests/Frontend/FrontendActionTest.cpp
clang/unittests/Serialization/InMemoryModuleCacheTest.cpp
Index: clang/unittests/Serialization/InMemoryModuleCacheTest.cpp
===================================================================
--- clang/unittests/Serialization/InMemoryModuleCacheTest.cpp
+++ clang/unittests/Serialization/InMemoryModuleCacheTest.cpp
@@ -24,12 +24,11 @@
TEST(InMemoryModuleCacheTest, initialState) {
InMemoryModuleCache Cache;
- EXPECT_EQ(InMemoryModuleCache::Unknown, Cache.getPCMState("B"));
+ EXPECT_EQ(nullptr, Cache.lookupPCM("B"));
EXPECT_FALSE(Cache.isPCMFinal("B"));
- EXPECT_FALSE(Cache.shouldBuildPCM("B"));
#if !defined(NDEBUG) && GTEST_HAS_DEATH_TEST
- EXPECT_DEATH(Cache.tryToDropPCM("B"), "PCM to remove is unknown");
+ EXPECT_DEATH(Cache.tryToRemovePCM("B"), "PCM to remove is unknown");
EXPECT_DEATH(Cache.finalizePCM("B"), "PCM to finalize is unknown");
#endif
}
@@ -40,37 +39,33 @@
InMemoryModuleCache Cache;
EXPECT_EQ(RawB, &Cache.addPCM("B", std::move(B)));
- EXPECT_EQ(InMemoryModuleCache::Tentative, Cache.getPCMState("B"));
EXPECT_EQ(RawB, Cache.lookupPCM("B"));
EXPECT_FALSE(Cache.isPCMFinal("B"));
- EXPECT_FALSE(Cache.shouldBuildPCM("B"));
#if !defined(NDEBUG) && GTEST_HAS_DEATH_TEST
EXPECT_DEATH(Cache.addPCM("B", getBuffer(2)), "Already has a PCM");
- EXPECT_DEATH(Cache.addBuiltPCM("B", getBuffer(2)),
- "Trying to override tentative PCM");
+ EXPECT_DEATH(Cache.addFinalPCM("B", getBuffer(2)),
+ "Already has a non-final PCM");
#endif
}
-TEST(InMemoryModuleCacheTest, addBuiltPCM) {
+TEST(InMemoryModuleCacheTest, addFinalPCM) {
auto B = getBuffer(1);
auto *RawB = B.get();
InMemoryModuleCache Cache;
- EXPECT_EQ(RawB, &Cache.addBuiltPCM("B", std::move(B)));
- EXPECT_EQ(InMemoryModuleCache::Final, Cache.getPCMState("B"));
+ EXPECT_EQ(RawB, &Cache.addFinalPCM("B", std::move(B)));
EXPECT_EQ(RawB, Cache.lookupPCM("B"));
EXPECT_TRUE(Cache.isPCMFinal("B"));
- EXPECT_FALSE(Cache.shouldBuildPCM("B"));
#if !defined(NDEBUG) && GTEST_HAS_DEATH_TEST
EXPECT_DEATH(Cache.addPCM("B", getBuffer(2)), "Already has a PCM");
- EXPECT_DEATH(Cache.addBuiltPCM("B", getBuffer(2)),
+ EXPECT_DEATH(Cache.addFinalPCM("B", getBuffer(2)),
"Trying to override finalized PCM");
#endif
}
-TEST(InMemoryModuleCacheTest, tryToDropPCM) {
+TEST(InMemoryModuleCacheTest, tryToRemovePCM) {
auto B1 = getBuffer(1);
auto B2 = getBuffer(2);
auto *RawB1 = B1.get();
@@ -78,27 +73,22 @@
ASSERT_NE(RawB1, RawB2);
InMemoryModuleCache Cache;
- EXPECT_EQ(InMemoryModuleCache::Unknown, Cache.getPCMState("B"));
EXPECT_EQ(RawB1, &Cache.addPCM("B", std::move(B1)));
- EXPECT_FALSE(Cache.tryToDropPCM("B"));
+ EXPECT_FALSE(Cache.tryToRemovePCM("B"));
EXPECT_EQ(nullptr, Cache.lookupPCM("B"));
- EXPECT_EQ(InMemoryModuleCache::ToBuild, Cache.getPCMState("B"));
EXPECT_FALSE(Cache.isPCMFinal("B"));
- EXPECT_TRUE(Cache.shouldBuildPCM("B"));
#if !defined(NDEBUG) && GTEST_HAS_DEATH_TEST
- EXPECT_DEATH(Cache.addPCM("B", getBuffer(2)), "Already has a PCM");
- EXPECT_DEATH(Cache.tryToDropPCM("B"),
- "PCM to remove is scheduled to be built");
- EXPECT_DEATH(Cache.finalizePCM("B"), "Trying to finalize a dropped PCM");
+ EXPECT_DEATH(Cache.tryToRemovePCM("B"), "PCM to remove is unknown");
+ EXPECT_DEATH(Cache.finalizePCM("B"), "PCM to finalize is unknown");
#endif
// Add a new one.
- EXPECT_EQ(RawB2, &Cache.addBuiltPCM("B", std::move(B2)));
+ EXPECT_EQ(RawB2, &Cache.addFinalPCM("B", std::move(B2)));
EXPECT_TRUE(Cache.isPCMFinal("B"));
// Can try to drop again, but this should error and do nothing.
- EXPECT_TRUE(Cache.tryToDropPCM("B"));
+ EXPECT_TRUE(Cache.tryToRemovePCM("B"));
EXPECT_EQ(RawB2, Cache.lookupPCM("B"));
}
@@ -107,12 +97,10 @@
auto *RawB = B.get();
InMemoryModuleCache Cache;
- EXPECT_EQ(InMemoryModuleCache::Unknown, Cache.getPCMState("B"));
EXPECT_EQ(RawB, &Cache.addPCM("B", std::move(B)));
// Call finalize.
Cache.finalizePCM("B");
- EXPECT_EQ(InMemoryModuleCache::Final, Cache.getPCMState("B"));
EXPECT_TRUE(Cache.isPCMFinal("B"));
}
Index: clang/unittests/Frontend/FrontendActionTest.cpp
===================================================================
--- clang/unittests/Frontend/FrontendActionTest.cpp
+++ clang/unittests/Frontend/FrontendActionTest.cpp
@@ -284,11 +284,9 @@
// Check whether the PCH was cached.
if (ShouldCache)
- EXPECT_EQ(InMemoryModuleCache::Final,
- Compiler.getModuleCache().getPCMState(PCHFilename));
+ EXPECT_TRUE(Compiler.getModuleCache().isPCMFinal(PCHFilename));
else
- EXPECT_EQ(InMemoryModuleCache::Unknown,
- Compiler.getModuleCache().getPCMState(PCHFilename));
+ EXPECT_EQ(nullptr, Compiler.getModuleCache().lookupPCM(PCHFilename));
}
}
Index: clang/test/Modules/implicit-invalidate-chain.c
===================================================================
--- clang/test/Modules/implicit-invalidate-chain.c
+++ /dev/null
@@ -1,67 +0,0 @@
-// RUN: rm -rf %t1 %t2 %t-include
-// RUN: mkdir %t-include
-// RUN: echo 'module D { header "D.h" }' >> %t-include/module.modulemap
-
-// Run with -verify, which onliy gets remarks from the main TU.
-//
-// RUN: echo '#define D 0' > %t-include/D.h
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t1 \
-// RUN: -fdisable-module-hash -fsyntax-only \
-// RUN: -I%S/Inputs/implicit-invalidate-chain -I%t-include \
-// RUN: -Rmodule-build -Rmodule-import %s
-// RUN: echo '#define D 11' > %t-include/D.h
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t1 \
-// RUN: -fdisable-module-hash -fsyntax-only \
-// RUN: -I%S/Inputs/implicit-invalidate-chain -I%t-include \
-// RUN: -Rmodule-build -Rmodule-import -verify %s
-
-// Run again, using FileCheck to check remarks from the module builds. This is
-// the key test: after the first attempt to import an out-of-date 'D', all the
-// modules have been invalidated and are not imported again until they are
-// rebuilt.
-//
-// RUN: echo '#define D 0' > %t-include/D.h
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t2 \
-// RUN: -fdisable-module-hash -fsyntax-only \
-// RUN: -I%S/Inputs/implicit-invalidate-chain -I%t-include \
-// RUN: -Rmodule-build -Rmodule-import %s
-// RUN: echo '#define D 11' > %t-include/D.h
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t2 \
-// RUN: -fdisable-module-hash -fsyntax-only \
-// RUN: -I%S/Inputs/implicit-invalidate-chain -I%t-include \
-// RUN: -Rmodule-build -Rmodule-import %s 2>&1 |\
-// RUN: FileCheck %s -implicit-check-not "remark:"
-
-#include "A.h" // \
- expected-remark-re{{importing module 'A' from '{{.*[/\\]}}A.pcm'}} \
- expected-remark-re{{importing module 'B' into 'A' from '{{.*[/\\]}}B.pcm'}} \
- expected-remark-re{{importing module 'C' into 'B' from '{{.*[/\\]}}C.pcm'}} \
- expected-remark-re{{importing module 'D' into 'C' from '{{.*[/\\]}}D.pcm'}} \
- expected-remark-re{{building module 'A' as '{{.*[/\\]}}A.pcm'}} \
- expected-remark{{finished building module 'A'}} \
- expected-remark-re{{importing module 'A' from '{{.*[/\\]}}A.pcm'}} \
- expected-remark-re{{importing module 'B' into 'A' from '{{.*[/\\]}}B.pcm'}} \
- expected-remark-re{{importing module 'C' into 'B' from '{{.*[/\\]}}C.pcm'}} \
- expected-remark-re{{importing module 'D' into 'C' from '{{.*[/\\]}}D.pcm'}}
-// CHECK: remark: importing module 'A' from '{{.*[/\\]}}A.pcm'
-// CHECK: remark: importing module 'B' into 'A' from '{{.*[/\\]}}B.pcm'
-// CHECK: remark: importing module 'C' into 'B' from '{{.*[/\\]}}C.pcm'
-// CHECK: remark: importing module 'D' into 'C' from '{{.*[/\\]}}D.pcm'
-// CHECK: remark: building module 'A'
-// CHECK: remark: building module 'B'
-// CHECK: remark: building module 'C'
-// CHECK: remark: building module 'D'
-// CHECK: remark: finished building module 'D'
-// CHECK: remark: importing module 'D' from '{{.*[/\\]}}D.pcm'
-// CHECK: remark: finished building module 'C'
-// CHECK: remark: importing module 'C' from '{{.*[/\\]}}C.pcm'
-// CHECK: remark: importing module 'D' into 'C' from '{{.*[/\\]}}D.pcm'
-// CHECK: remark: finished building module 'B'
-// CHECK: remark: importing module 'B' from '{{.*[/\\]}}B.pcm'
-// CHECK: remark: importing module 'C' into 'B' from '{{.*[/\\]}}C.pcm'
-// CHECK: remark: importing module 'D' into 'C' from '{{.*[/\\]}}D.pcm'
-// CHECK: remark: finished building module 'A'
-// CHECK: remark: importing module 'A' from '{{.*[/\\]}}A.pcm'
-// CHECK: remark: importing module 'B' into 'A' from '{{.*[/\\]}}B.pcm'
-// CHECK: remark: importing module 'C' into 'B' from '{{.*[/\\]}}C.pcm'
-// CHECK: remark: importing module 'D' into 'C' from '{{.*[/\\]}}D.pcm'
Index: clang/test/Modules/Inputs/implicit-invalidate-chain/module.modulemap
===================================================================
--- clang/test/Modules/Inputs/implicit-invalidate-chain/module.modulemap
+++ /dev/null
@@ -1,3 +0,0 @@
-module A { header "A.h" }
-module B { header "B.h" }
-module C { header "C.h" }
Index: clang/test/Modules/Inputs/implicit-invalidate-chain/C.h
===================================================================
--- clang/test/Modules/Inputs/implicit-invalidate-chain/C.h
+++ /dev/null
@@ -1,2 +0,0 @@
-// C
-#include "D.h"
Index: clang/test/Modules/Inputs/implicit-invalidate-chain/B.h
===================================================================
--- clang/test/Modules/Inputs/implicit-invalidate-chain/B.h
+++ /dev/null
@@ -1,2 +0,0 @@
-// B
-#include "C.h"
Index: clang/test/Modules/Inputs/implicit-invalidate-chain/A.h
===================================================================
--- clang/test/Modules/Inputs/implicit-invalidate-chain/A.h
+++ /dev/null
@@ -1,2 +0,0 @@
-// A
-#include "B.h"
Index: clang/lib/Serialization/ModuleManager.cpp
===================================================================
--- clang/lib/Serialization/ModuleManager.cpp
+++ clang/lib/Serialization/ModuleManager.cpp
@@ -163,7 +163,7 @@
// Load the contents of the module
if (std::unique_ptr<llvm::MemoryBuffer> Buffer = lookupBuffer(FileName)) {
// The buffer was already provided for us.
- NewModule->Buffer = &ModuleCache->addBuiltPCM(FileName, std::move(Buffer));
+ NewModule->Buffer = &ModuleCache->addFinalPCM(FileName, std::move(Buffer));
// Since the cached buffer is reused, it is safe to close the file
// descriptor that was opened while stat()ing the PCM in
// lookupModuleFile() above, it won't be needed any longer.
@@ -173,11 +173,6 @@
NewModule->Buffer = Buffer;
// As above, the file descriptor is no longer needed.
Entry->closeFile();
- } else if (getModuleCache().shouldBuildPCM(FileName)) {
- // Report that the module is out of date, since we tried (and failed) to
- // import it earlier.
- Entry->closeFile();
- return OutOfDate;
} else {
// Open the AST file.
llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Buf((std::error_code()));
@@ -185,7 +180,9 @@
Buf = llvm::MemoryBuffer::getSTDIN();
} else {
// Get a buffer of the file and close the file descriptor when done.
- Buf = FileMgr.getBufferForFile(NewModule->File, /*isVolatile=*/false);
+ // The file is volatile because in a parallel build we expect multiple
+ // compiler processes to use the same module file rebuilding it if needed.
+ Buf = FileMgr.getBufferForFile(NewModule->File, /*isVolatile=*/true);
}
if (!Buf) {
Index: clang/lib/Serialization/InMemoryModuleCache.cpp
===================================================================
--- clang/lib/Serialization/InMemoryModuleCache.cpp
+++ clang/lib/Serialization/InMemoryModuleCache.cpp
@@ -11,16 +11,6 @@
using namespace clang;
-InMemoryModuleCache::State
-InMemoryModuleCache::getPCMState(llvm::StringRef Filename) const {
- auto I = PCMs.find(Filename);
- if (I == PCMs.end())
- return Unknown;
- if (I->second.IsFinal)
- return Final;
- return I->second.Buffer ? Tentative : ToBuild;
-}
-
llvm::MemoryBuffer &
InMemoryModuleCache::addPCM(llvm::StringRef Filename,
std::unique_ptr<llvm::MemoryBuffer> Buffer) {
@@ -30,11 +20,11 @@
}
llvm::MemoryBuffer &
-InMemoryModuleCache::addBuiltPCM(llvm::StringRef Filename,
+InMemoryModuleCache::addFinalPCM(llvm::StringRef Filename,
std::unique_ptr<llvm::MemoryBuffer> Buffer) {
auto &PCM = PCMs[Filename];
assert(!PCM.IsFinal && "Trying to override finalized PCM?");
- assert(!PCM.Buffer && "Trying to override tentative PCM?");
+ assert(!PCM.Buffer && "Already has a non-final PCM");
PCM.Buffer = std::move(Buffer);
PCM.IsFinal = true;
return *PCM.Buffer;
@@ -49,24 +39,21 @@
}
bool InMemoryModuleCache::isPCMFinal(llvm::StringRef Filename) const {
- return getPCMState(Filename) == Final;
-}
-
-bool InMemoryModuleCache::shouldBuildPCM(llvm::StringRef Filename) const {
- return getPCMState(Filename) == ToBuild;
+ auto I = PCMs.find(Filename);
+ if (I == PCMs.end())
+ return false;
+ return I->second.IsFinal;
}
-bool InMemoryModuleCache::tryToDropPCM(llvm::StringRef Filename) {
+bool InMemoryModuleCache::tryToRemovePCM(llvm::StringRef Filename) {
auto I = PCMs.find(Filename);
assert(I != PCMs.end() && "PCM to remove is unknown...");
auto &PCM = I->second;
- assert(PCM.Buffer && "PCM to remove is scheduled to be built...");
-
if (PCM.IsFinal)
return true;
- PCM.Buffer.reset();
+ PCMs.erase(I);
return false;
}
Index: clang/lib/Serialization/ASTWriter.cpp
===================================================================
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -4304,7 +4304,7 @@
WritingAST = false;
if (ShouldCacheASTInMemory) {
// Construct MemoryBuffer and update buffer manager.
- ModuleCache.addBuiltPCM(OutputFile,
+ ModuleCache.addFinalPCM(OutputFile,
llvm::MemoryBuffer::getMemBufferCopy(
StringRef(Buffer.begin(), Buffer.size())));
}
Index: clang/lib/Serialization/ASTReader.cpp
===================================================================
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -4503,7 +4503,7 @@
if (ShouldFinalizePCM)
MC.finalizePCM(FileName);
else
- MC.tryToDropPCM(FileName);
+ MC.tryToRemovePCM(FileName);
});
ModuleFile &F = *M;
BitstreamCursor &Stream = F.Stream;
Index: clang/include/clang/Serialization/InMemoryModuleCache.h
===================================================================
--- clang/include/clang/Serialization/InMemoryModuleCache.h
+++ clang/include/clang/Serialization/InMemoryModuleCache.h
@@ -45,61 +45,35 @@
llvm::StringMap<PCM> PCMs;
public:
- /// There are four states for a PCM. It must monotonically increase.
- ///
- /// 1. Unknown: the PCM has neither been read from disk nor built.
- /// 2. Tentative: the PCM has been read from disk but not yet imported or
- /// built. It might work.
- /// 3. ToBuild: the PCM read from disk did not work but a new one has not
- /// been built yet.
- /// 4. Final: indicating that the current PCM was either built in this
- /// process or has been successfully imported.
- enum State { Unknown, Tentative, ToBuild, Final };
-
- /// Get the state of the PCM.
- State getPCMState(llvm::StringRef Filename) const;
-
/// Store the PCM under the Filename.
///
- /// \pre state is Unknown
- /// \post state is Tentative
+ /// \pre PCM for the same Filename shouldn't be in cache already.
/// \return a reference to the buffer as a convenience.
llvm::MemoryBuffer &addPCM(llvm::StringRef Filename,
std::unique_ptr<llvm::MemoryBuffer> Buffer);
- /// Store a just-built PCM under the Filename.
+ /// Store a final PCM under the Filename.
///
- /// \pre state is Unknown or ToBuild.
- /// \pre state is not Tentative.
+ /// \pre PCM for the same Filename shouldn't be in cache already.
/// \return a reference to the buffer as a convenience.
- llvm::MemoryBuffer &addBuiltPCM(llvm::StringRef Filename,
+ llvm::MemoryBuffer &addFinalPCM(llvm::StringRef Filename,
std::unique_ptr<llvm::MemoryBuffer> Buffer);
- /// Try to remove a buffer from the cache. No effect if state is Final.
+ /// Try to remove a PCM from the cache. No effect if it is Final.
///
- /// \pre state is Tentative/Final.
- /// \post Tentative => ToBuild or Final => Final.
- /// \return false on success, i.e. if Tentative => ToBuild.
- bool tryToDropPCM(llvm::StringRef Filename);
+ /// \return false on success.
+ bool tryToRemovePCM(llvm::StringRef Filename);
/// Mark a PCM as final.
- ///
- /// \pre state is Tentative or Final.
- /// \post state is Final.
void finalizePCM(llvm::StringRef Filename);
- /// Get a pointer to the pCM if it exists; else nullptr.
+ /// Get a pointer to the PCM if it exists; else nullptr.
llvm::MemoryBuffer *lookupPCM(llvm::StringRef Filename) const;
/// Check whether the PCM is final and has been shown to work.
///
/// \return true iff state is Final.
bool isPCMFinal(llvm::StringRef Filename) const;
-
- /// Check whether the PCM is waiting to be built.
- ///
- /// \return true iff state is ToBuild.
- bool shouldBuildPCM(llvm::StringRef Filename) const;
};
} // end namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
