Skip to content

Commit 1c9a800

Browse files
ChuanqiXu9ChuanqiXu9
committed
Recommit [C++20] [Modules] Serialize the evaluated constant values for VarDecl
Close #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.
1 parent 0bf120a commit 1c9a800

File tree

5 files changed

+204
-2
lines changed

5 files changed

+204
-2
lines changed

‎clang/lib/Serialization/ASTReaderDecl.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1659,6 +1659,13 @@ void ASTDeclReader::ReadVarDeclInit(VarDecl *VD) {
16591659
EvaluatedStmt *Eval = VD->ensureEvaluatedStmt();
16601660
Eval->HasConstantInitialization = (Val & 2) != 0;
16611661
Eval->HasConstantDestruction = (Val & 4) != 0;
1662+
Eval->WasEvaluated = (Val & 8) != 0;
1663+
if (Eval->WasEvaluated) {
1664+
Eval->Evaluated = Record.readAPValue();
1665+
if (Eval->Evaluated.needsCleanup())
1666+
Reader.getContext().addDestruction(&Eval->Evaluated);
1667+
}
1668+
16621669
// Store the offset of the initializer. Don't deserialize it yet: it might
16631670
// not be needed, and might refer back to the variable, for example if it
16641671
// contains a lambda.

‎clang/lib/Serialization/ASTWriter.cpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5987,13 +5987,20 @@ void ASTRecordWriter::AddVarDeclInit(const VarDecl *VD) {
59875987
return;
59885988
}
59895989

5990-
unsigned Val = 1;
5990+
uint64_t Val = 1;
59915991
if (EvaluatedStmt *ES = VD->getEvaluatedStmt()) {
59925992
Val |= (ES->HasConstantInitialization ? 2 : 0);
59935993
Val |= (ES->HasConstantDestruction ? 4 : 0);
5994-
// FIXME: Also emit the constant initializer value.
5994+
APValue *Evaluated = VD->getEvaluatedValue();
5995+
// If the evaluted result is constant, emit it.
5996+
if (Evaluated && (Evaluated->isInt() || Evaluated->isFloat()))
5997+
Val |= 8;
59955998
}
59965999
push_back(Val);
6000+
if (Val & 8) {
6001+
AddAPValue(*VD->getEvaluatedValue());
6002+
}
6003+
59976004
writeStmtRef(Init);
59986005
}
59996006

‎clang/test/Modules/pr62796.cppm

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
// RUN: rm -rf %t
2+
// RUN: mkdir -p %t
3+
// RUN: split-file %s %t
4+
//
5+
// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/Cache.cppm -o %t/Cache.pcm
6+
// RUN: %clang_cc1 -std=c++20 %t/Use.cpp -fmodule-file=Fibonacci.Cache=%t/Cache.pcm \
7+
// RUN: -fsyntax-only -verify
8+
9+
//--- Cache.cppm
10+
export module Fibonacci.Cache;
11+
12+
export namespace Fibonacci
13+
{
14+
constexpr unsigned long Recursive(unsigned long n)
15+
{
16+
if (n == 0)
17+
return 0;
18+
if (n == 1)
19+
return 1;
20+
return Recursive(n - 2) + Recursive(n - 1);
21+
}
22+
23+
template<unsigned long N>
24+
struct Number{};
25+
26+
struct DefaultStrategy
27+
{
28+
constexpr unsigned long operator()(unsigned long n, auto... other) const
29+
{
30+
return (n + ... + other);
31+
}
32+
};
33+
34+
constexpr unsigned long Compute(Number<10ul>, auto strategy)
35+
{
36+
return strategy(Recursive(10ul));
37+
}
38+
39+
template<unsigned long N, typename Strategy = DefaultStrategy>
40+
constexpr unsigned long Cache = Compute(Number<N>{}, Strategy{});
41+
42+
template constexpr unsigned long Cache<10ul>;
43+
}
44+
45+
//--- Use.cpp
46+
// expected-no-diagnostics
47+
import Fibonacci.Cache;
48+
49+
constexpr bool value = Fibonacci::Cache<10ul> == 55;
50+
51+
static_assert(value == true, "");

‎clang/unittests/Serialization/CMakeLists.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ add_clang_unittest(SerializationTests
88
InMemoryModuleCacheTest.cpp
99
ModuleCacheTest.cpp
1010
SourceLocationEncodingTest.cpp
11+
VarDeclConstantInitTest.cpp
1112
)
1213

1314
clang_target_link_libraries(SerializationTests
@@ -18,4 +19,5 @@ clang_target_link_libraries(SerializationTests
1819
clangLex
1920
clangSema
2021
clangSerialization
22+
clangTooling
2123
)

‎clang/unittests/Serialization/VarDeclConstantInitTest.cpp

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
//===- unittests/Serialization/VarDeclConstantInitTest.cpp - CI tests -----===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include "clang/ASTMatchers/ASTMatchFinder.h"
10+
#include "clang/ASTMatchers/ASTMatchers.h"
11+
#include "clang/Basic/FileManager.h"
12+
#include "clang/Frontend/CompilerInstance.h"
13+
#include "clang/Frontend/CompilerInvocation.h"
14+
#include "clang/Frontend/FrontendActions.h"
15+
#include "clang/Frontend/Utils.h"
16+
#include "clang/Lex/HeaderSearch.h"
17+
#include "clang/Tooling/Tooling.h"
18+
#include "llvm/ADT/SmallString.h"
19+
#include "llvm/Support/FileSystem.h"
20+
#include "llvm/Support/raw_ostream.h"
21+
22+
#include "gtest/gtest.h"
23+
24+
using namespace llvm;
25+
using namespace clang;
26+
27+
namespace {
28+
29+
class VarDeclConstantInitTest : public ::testing::Test {
30+
void SetUp() override {
31+
ASSERT_FALSE(sys::fs::createUniqueDirectory("modules-test", TestDir));
32+
}
33+
34+
void TearDown() override { sys::fs::remove_directories(TestDir); }
35+
36+
public:
37+
SmallString<256> TestDir;
38+
39+
void addFile(StringRef Path, StringRef Contents) {
40+
ASSERT_FALSE(sys::path::is_absolute(Path));
41+
42+
SmallString<256> AbsPath(TestDir);
43+
sys::path::append(AbsPath, Path);
44+
45+
ASSERT_FALSE(
46+
sys::fs::create_directories(llvm::sys::path::parent_path(AbsPath)));
47+
48+
std::error_code EC;
49+
llvm::raw_fd_ostream OS(AbsPath, EC);
50+
ASSERT_FALSE(EC);
51+
OS << Contents;
52+
}
53+
};
54+
55+
TEST_F(VarDeclConstantInitTest, CachedConstantInit) {
56+
addFile("Cached.cppm", R"cpp(
57+
export module Fibonacci.Cache;
58+
59+
export namespace Fibonacci
60+
{
61+
constexpr unsigned long Recursive(unsigned long n)
62+
{
63+
if (n == 0)
64+
return 0;
65+
if (n == 1)
66+
return 1;
67+
return Recursive(n - 2) + Recursive(n - 1);
68+
}
69+
70+
template<unsigned long N>
71+
struct Number{};
72+
73+
struct DefaultStrategy
74+
{
75+
constexpr unsigned long operator()(unsigned long n, auto... other) const
76+
{
77+
return (n + ... + other);
78+
}
79+
};
80+
81+
constexpr unsigned long Compute(Number<10ul>, auto strategy)
82+
{
83+
return strategy(Recursive(10ul));
84+
}
85+
86+
template<unsigned long N, typename Strategy = DefaultStrategy>
87+
constexpr unsigned long Cache = Compute(Number<N>{}, Strategy{});
88+
89+
template constexpr unsigned long Cache<10ul>;
90+
}
91+
)cpp");
92+
93+
IntrusiveRefCntPtr<DiagnosticsEngine> Diags =
94+
CompilerInstance::createDiagnostics(new DiagnosticOptions());
95+
CreateInvocationOptions CIOpts;
96+
CIOpts.Diags = Diags;
97+
98+
std::string CacheBMIPath = llvm::Twine(TestDir + "/Cached.pcm").str();
99+
const char *Args[] = {
100+
"clang++", "-std=c++20", "--precompile", "-working-directory",
101+
TestDir.c_str(), "Cached.cppm", "-o", CacheBMIPath.c_str()};
102+
std::shared_ptr<CompilerInvocation> Invocation =
103+
createInvocation(Args, CIOpts);
104+
ASSERT_TRUE(Invocation);
105+
106+
CompilerInstance Instance;
107+
Instance.setDiagnostics(Diags.get());
108+
Instance.setInvocation(Invocation);
109+
GenerateModuleInterfaceAction Action;
110+
ASSERT_TRUE(Instance.ExecuteAction(Action));
111+
ASSERT_FALSE(Diags->hasErrorOccurred());
112+
113+
std::string DepArg =
114+
llvm::Twine("-fmodule-file=Fibonacci.Cache=" + CacheBMIPath).str();
115+
std::unique_ptr<ASTUnit> AST = tooling::buildASTFromCodeWithArgs(
116+
R"cpp(
117+
import Fibonacci.Cache;
118+
)cpp",
119+
/*Args=*/{"-std=c++20", DepArg.c_str()});
120+
121+
using namespace clang::ast_matchers;
122+
ASTContext &Ctx = AST->getASTContext();
123+
const auto *cached = selectFirst<VarDecl>(
124+
"Cache",
125+
match(varDecl(isTemplateInstantiation(), hasName("Cache")).bind("Cache"),
126+
Ctx));
127+
EXPECT_TRUE(cached);
128+
EXPECT_TRUE(cached->getEvaluatedStmt());
129+
EXPECT_TRUE(cached->getEvaluatedStmt()->WasEvaluated);
130+
EXPECT_TRUE(cached->getEvaluatedValue());
131+
EXPECT_TRUE(cached->getEvaluatedValue()->isInt());
132+
EXPECT_EQ(cached->getEvaluatedValue()->getInt(), 55);
133+
}
134+
135+
} // anonymous namespace

0 commit comments

Comments
 (0)