Author: Chuanqi Xu Date: 2023-05-24T15:45:16+08:00 New Revision: 1c9a8004ed88c9a5e42e5b247cb489456559b845
URL: https://github.com/llvm/llvm-project/commit/1c9a8004ed88c9a5e42e5b247cb489456559b845 DIFF: https://github.com/llvm/llvm-project/commit/1c9a8004ed88c9a5e42e5b247cb489456559b845.diff LOG: Recommit [C++20] [Modules] Serialize the evaluated constant values for VarDecl Close https://github.com/llvm/llvm-project/issues/62796. Previously, we didn't serialize the evaluated result for VarDecl. This caused the compilation of template metaprogramming become slower than expect. This patch fixes the issue. This is a recommit tested with asan built clang. Added: clang/test/Modules/pr62796.cppm clang/unittests/Serialization/VarDeclConstantInitTest.cpp Modified: clang/lib/Serialization/ASTReaderDecl.cpp clang/lib/Serialization/ASTWriter.cpp clang/unittests/Serialization/CMakeLists.txt Removed: ################################################################################ diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index 5ce7007d8acef..fb8677769d09b 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -1659,6 +1659,13 @@ void ASTDeclReader::ReadVarDeclInit(VarDecl *VD) { EvaluatedStmt *Eval = VD->ensureEvaluatedStmt(); Eval->HasConstantInitialization = (Val & 2) != 0; Eval->HasConstantDestruction = (Val & 4) != 0; + Eval->WasEvaluated = (Val & 8) != 0; + if (Eval->WasEvaluated) { + Eval->Evaluated = Record.readAPValue(); + if (Eval->Evaluated.needsCleanup()) + Reader.getContext().addDestruction(&Eval->Evaluated); + } + // Store the offset of the initializer. Don't deserialize it yet: it might // not be needed, and might refer back to the variable, for example if it // contains a lambda. diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 749aaa4cd6e19..4efc27b3d2434 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -5987,13 +5987,20 @@ void ASTRecordWriter::AddVarDeclInit(const VarDecl *VD) { return; } - unsigned Val = 1; + uint64_t Val = 1; if (EvaluatedStmt *ES = VD->getEvaluatedStmt()) { Val |= (ES->HasConstantInitialization ? 2 : 0); Val |= (ES->HasConstantDestruction ? 4 : 0); - // FIXME: Also emit the constant initializer value. + APValue *Evaluated = VD->getEvaluatedValue(); + // If the evaluted result is constant, emit it. + if (Evaluated && (Evaluated->isInt() || Evaluated->isFloat())) + Val |= 8; } push_back(Val); + if (Val & 8) { + AddAPValue(*VD->getEvaluatedValue()); + } + writeStmtRef(Init); } diff --git a/clang/test/Modules/pr62796.cppm b/clang/test/Modules/pr62796.cppm new file mode 100644 index 0000000000000..f96e54bc6aded --- /dev/null +++ b/clang/test/Modules/pr62796.cppm @@ -0,0 +1,51 @@ +// RUN: rm -rf %t +// RUN: mkdir -p %t +// RUN: split-file %s %t +// +// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/Cache.cppm -o %t/Cache.pcm +// RUN: %clang_cc1 -std=c++20 %t/Use.cpp -fmodule-file=Fibonacci.Cache=%t/Cache.pcm \ +// RUN: -fsyntax-only -verify + +//--- Cache.cppm +export module Fibonacci.Cache; + +export namespace Fibonacci +{ + constexpr unsigned long Recursive(unsigned long n) + { + if (n == 0) + return 0; + if (n == 1) + return 1; + return Recursive(n - 2) + Recursive(n - 1); + } + + template<unsigned long N> + struct Number{}; + + struct DefaultStrategy + { + constexpr unsigned long operator()(unsigned long n, auto... other) const + { + return (n + ... + other); + } + }; + + constexpr unsigned long Compute(Number<10ul>, auto strategy) + { + return strategy(Recursive(10ul)); + } + + template<unsigned long N, typename Strategy = DefaultStrategy> + constexpr unsigned long Cache = Compute(Number<N>{}, Strategy{}); + + template constexpr unsigned long Cache<10ul>; +} + +//--- Use.cpp +// expected-no-diagnostics +import Fibonacci.Cache; + +constexpr bool value = Fibonacci::Cache<10ul> == 55; + +static_assert(value == true, ""); diff --git a/clang/unittests/Serialization/CMakeLists.txt b/clang/unittests/Serialization/CMakeLists.txt index cfb4089167aad..88aca2e135200 100644 --- a/clang/unittests/Serialization/CMakeLists.txt +++ b/clang/unittests/Serialization/CMakeLists.txt @@ -8,6 +8,7 @@ add_clang_unittest(SerializationTests InMemoryModuleCacheTest.cpp ModuleCacheTest.cpp SourceLocationEncodingTest.cpp + VarDeclConstantInitTest.cpp ) clang_target_link_libraries(SerializationTests @@ -18,4 +19,5 @@ clang_target_link_libraries(SerializationTests clangLex clangSema clangSerialization + clangTooling ) diff --git a/clang/unittests/Serialization/VarDeclConstantInitTest.cpp b/clang/unittests/Serialization/VarDeclConstantInitTest.cpp new file mode 100644 index 0000000000000..33fc82b9adc75 --- /dev/null +++ b/clang/unittests/Serialization/VarDeclConstantInitTest.cpp @@ -0,0 +1,135 @@ +//===- unittests/Serialization/VarDeclConstantInitTest.cpp - CI tests -----===// +// +// 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/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/FileManager.h" +#include "clang/Frontend/CompilerInstance.h" +#include "clang/Frontend/CompilerInvocation.h" +#include "clang/Frontend/FrontendActions.h" +#include "clang/Frontend/Utils.h" +#include "clang/Lex/HeaderSearch.h" +#include "clang/Tooling/Tooling.h" +#include "llvm/ADT/SmallString.h" +#include "llvm/Support/FileSystem.h" +#include "llvm/Support/raw_ostream.h" + +#include "gtest/gtest.h" + +using namespace llvm; +using namespace clang; + +namespace { + +class VarDeclConstantInitTest : public ::testing::Test { + void SetUp() override { + ASSERT_FALSE(sys::fs::createUniqueDirectory("modules-test", TestDir)); + } + + void TearDown() override { sys::fs::remove_directories(TestDir); } + +public: + SmallString<256> TestDir; + + void addFile(StringRef Path, StringRef Contents) { + ASSERT_FALSE(sys::path::is_absolute(Path)); + + SmallString<256> AbsPath(TestDir); + sys::path::append(AbsPath, Path); + + ASSERT_FALSE( + sys::fs::create_directories(llvm::sys::path::parent_path(AbsPath))); + + std::error_code EC; + llvm::raw_fd_ostream OS(AbsPath, EC); + ASSERT_FALSE(EC); + OS << Contents; + } +}; + +TEST_F(VarDeclConstantInitTest, CachedConstantInit) { + addFile("Cached.cppm", R"cpp( +export module Fibonacci.Cache; + +export namespace Fibonacci +{ + constexpr unsigned long Recursive(unsigned long n) + { + if (n == 0) + return 0; + if (n == 1) + return 1; + return Recursive(n - 2) + Recursive(n - 1); + } + + template<unsigned long N> + struct Number{}; + + struct DefaultStrategy + { + constexpr unsigned long operator()(unsigned long n, auto... other) const + { + return (n + ... + other); + } + }; + + constexpr unsigned long Compute(Number<10ul>, auto strategy) + { + return strategy(Recursive(10ul)); + } + + template<unsigned long N, typename Strategy = DefaultStrategy> + constexpr unsigned long Cache = Compute(Number<N>{}, Strategy{}); + + template constexpr unsigned long Cache<10ul>; +} + )cpp"); + + IntrusiveRefCntPtr<DiagnosticsEngine> Diags = + CompilerInstance::createDiagnostics(new DiagnosticOptions()); + CreateInvocationOptions CIOpts; + CIOpts.Diags = Diags; + + std::string CacheBMIPath = llvm::Twine(TestDir + "/Cached.pcm").str(); + const char *Args[] = { + "clang++", "-std=c++20", "--precompile", "-working-directory", + TestDir.c_str(), "Cached.cppm", "-o", CacheBMIPath.c_str()}; + std::shared_ptr<CompilerInvocation> Invocation = + createInvocation(Args, CIOpts); + ASSERT_TRUE(Invocation); + + CompilerInstance Instance; + Instance.setDiagnostics(Diags.get()); + Instance.setInvocation(Invocation); + GenerateModuleInterfaceAction Action; + ASSERT_TRUE(Instance.ExecuteAction(Action)); + ASSERT_FALSE(Diags->hasErrorOccurred()); + + std::string DepArg = + llvm::Twine("-fmodule-file=Fibonacci.Cache=" + CacheBMIPath).str(); + std::unique_ptr<ASTUnit> AST = tooling::buildASTFromCodeWithArgs( + R"cpp( +import Fibonacci.Cache; + )cpp", + /*Args=*/{"-std=c++20", DepArg.c_str()}); + + using namespace clang::ast_matchers; + ASTContext &Ctx = AST->getASTContext(); + const auto *cached = selectFirst<VarDecl>( + "Cache", + match(varDecl(isTemplateInstantiation(), hasName("Cache")).bind("Cache"), + Ctx)); + EXPECT_TRUE(cached); + EXPECT_TRUE(cached->getEvaluatedStmt()); + EXPECT_TRUE(cached->getEvaluatedStmt()->WasEvaluated); + EXPECT_TRUE(cached->getEvaluatedValue()); + EXPECT_TRUE(cached->getEvaluatedValue()->isInt()); + EXPECT_EQ(cached->getEvaluatedValue()->getInt(), 55); +} + +} // anonymous namespace _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
