[clang] 904c53d - [clang][Interp] Use different inline descriptors for global variables
Timm Bäder via cfe-commits
cfe-commits at lists.llvm.org
Fri Jun 14 20:42:10 PDT 2024
Author: Timm Bäder
Date: 2024-06-15T05:41:59+02:00
New Revision: 904c53d53e7abc3916a679832ec4fa6e521e09b9
URL: https://github.com/llvm/llvm-project/commit/904c53d53e7abc3916a679832ec4fa6e521e09b9
DIFF: https://github.com/llvm/llvm-project/commit/904c53d53e7abc3916a679832ec4fa6e521e09b9.diff
LOG: [clang][Interp] Use different inline descriptors for global variables
Most of the InlineDescriptor fields were unused for global variables.
But more importantly, we need to differentiate between global variables
that are uninitialized because they didn't have an initializer when we
originally created them, and ones that are uninitialized because they
DID have an initializer, but evaluating it failed.
Added:
Modified:
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/ByteCodeExprGen.h
clang/lib/AST/Interp/Descriptor.h
clang/lib/AST/Interp/Pointer.cpp
clang/lib/AST/Interp/Pointer.h
clang/lib/AST/Interp/Program.cpp
clang/test/AST/Interp/cxx98.cpp
clang/unittests/AST/Interp/Descriptor.cpp
Removed:
################################################################################
diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
index b8e32452371a4..300c013d15da8 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -30,15 +30,21 @@ template <class Emitter> class DeclScope final : public VariableScope<Emitter> {
public:
DeclScope(ByteCodeExprGen<Emitter> *Ctx, const ValueDecl *VD)
: VariableScope<Emitter>(Ctx, nullptr), Scope(Ctx->P, VD),
- OldGlobalDecl(Ctx->GlobalDecl) {
+ OldGlobalDecl(Ctx->GlobalDecl),
+ OldInitializingDecl(Ctx->InitializingDecl) {
Ctx->GlobalDecl = Context::shouldBeGloballyIndexed(VD);
+ Ctx->InitializingDecl = VD;
}
- ~DeclScope() { this->Ctx->GlobalDecl = OldGlobalDecl; }
+ ~DeclScope() {
+ this->Ctx->GlobalDecl = OldGlobalDecl;
+ this->Ctx->InitializingDecl = OldInitializingDecl;
+ }
private:
Program::DeclScope Scope;
bool OldGlobalDecl;
+ const ValueDecl *OldInitializingDecl;
};
/// Scope used to handle initialization methods.
@@ -3125,11 +3131,16 @@ template <class Emitter>
bool ByteCodeExprGen<Emitter>::visitDecl(const VarDecl *VD) {
assert(!VD->isInvalidDecl() && "Trying to constant evaluate an invalid decl");
- // Global variable we've already seen but that's uninitialized means
- // evaluating the initializer failed. Just return failure.
- if (std::optional<unsigned> Index = P.getGlobal(VD);
- Index && !P.getPtrGlobal(*Index).isInitialized())
- return false;
+ // If we've seen the global variable already and the initializer failed,
+ // just return false immediately.
+ if (std::optional<unsigned> Index = P.getGlobal(VD)) {
+ const Pointer &Ptr = P.getPtrGlobal(*Index);
+ const GlobalInlineDescriptor &GD =
+ *reinterpret_cast<const GlobalInlineDescriptor *>(
+ Ptr.block()->rawData());
+ if (GD.InitState == GlobalInitState::InitializerFailed)
+ return false;
+ }
// Create and initialize the variable.
if (!this->visitVarDecl(VD))
@@ -3167,13 +3178,15 @@ bool ByteCodeExprGen<Emitter>::visitDecl(const VarDecl *VD) {
auto GlobalIndex = P.getGlobal(VD);
assert(GlobalIndex);
Block *GlobalBlock = P.getGlobal(*GlobalIndex);
- InlineDescriptor &ID =
- *reinterpret_cast<InlineDescriptor *>(GlobalBlock->rawData());
- ID.IsInitialized = false;
+ GlobalInlineDescriptor &GD =
+ *reinterpret_cast<GlobalInlineDescriptor *>(GlobalBlock->rawData());
+
+ GD.InitState = GlobalInitState::InitializerFailed;
GlobalBlock->invokeDtor();
}
return false;
}
+
return true;
}
@@ -3967,36 +3980,39 @@ bool ByteCodeExprGen<Emitter>::visitDeclRef(const ValueDecl *D, const Expr *E) {
}
}
- // Try to lazily visit (or emit dummy pointers for) declarations
- // we haven't seen yet.
- if (Ctx.getLangOpts().CPlusPlus) {
- if (const auto *VD = dyn_cast<VarDecl>(D)) {
- const auto typeShouldBeVisited = [&](QualType T) -> bool {
- if (T.isConstant(Ctx.getASTContext()))
- return true;
- if (const auto *RT = T->getAs<ReferenceType>())
- return RT->getPointeeType().isConstQualified();
- return false;
- };
+ if (D != InitializingDecl) {
+ // Try to lazily visit (or emit dummy pointers for) declarations
+ // we haven't seen yet.
+ if (Ctx.getLangOpts().CPlusPlus) {
+ if (const auto *VD = dyn_cast<VarDecl>(D)) {
+ const auto typeShouldBeVisited = [&](QualType T) -> bool {
+ if (T.isConstant(Ctx.getASTContext()))
+ return true;
+ if (const auto *RT = T->getAs<ReferenceType>())
+ return RT->getPointeeType().isConstQualified();
+ return false;
+ };
- // Visit local const variables like normal.
- if ((VD->isLocalVarDecl() || VD->isStaticDataMember()) &&
- typeShouldBeVisited(VD->getType())) {
+ // Visit local const variables like normal.
+ if ((VD->hasGlobalStorage() || VD->isLocalVarDecl() ||
+ VD->isStaticDataMember()) &&
+ typeShouldBeVisited(VD->getType())) {
+ if (!this->visitVarDecl(VD))
+ return false;
+ // Retry.
+ return this->visitDeclRef(VD, E);
+ }
+ }
+ } else {
+ if (const auto *VD = dyn_cast<VarDecl>(D);
+ VD && VD->getAnyInitializer() &&
+ VD->getType().isConstant(Ctx.getASTContext()) && !VD->isWeak()) {
if (!this->visitVarDecl(VD))
return false;
// Retry.
return this->visitDeclRef(VD, E);
}
}
- } else {
- if (const auto *VD = dyn_cast<VarDecl>(D);
- VD && VD->getAnyInitializer() &&
- VD->getType().isConstant(Ctx.getASTContext()) && !VD->isWeak()) {
- if (!this->visitVarDecl(VD))
- return false;
- // Retry.
- return this->visitDeclRef(VD, E);
- }
}
if (std::optional<unsigned> I = P.getOrCreateDummy(D)) {
diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.h b/clang/lib/AST/Interp/ByteCodeExprGen.h
index 295cfef0525cd..7bb5304cac71e 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.h
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -321,6 +321,7 @@ class ByteCodeExprGen : public ConstStmtVisitor<ByteCodeExprGen<Emitter>, bool>,
/// Flag inidicating if we're initializing an already created
/// variable. This is set in visitInitializer().
bool Initializing = false;
+ const ValueDecl *InitializingDecl = nullptr;
/// Flag indicating if we're initializing a global variable.
bool GlobalDecl = false;
diff --git a/clang/lib/AST/Interp/Descriptor.h b/clang/lib/AST/Interp/Descriptor.h
index fcb14e76e7eb0..f444b8a78e802 100644
--- a/clang/lib/AST/Interp/Descriptor.h
+++ b/clang/lib/AST/Interp/Descriptor.h
@@ -47,6 +47,18 @@ using BlockMoveFn = void (*)(Block *Storage, const std::byte *SrcFieldPtr,
std::byte *DstFieldPtr,
const Descriptor *FieldDesc);
+enum class GlobalInitState {
+ Initialized,
+ NoInitializer,
+ InitializerFailed,
+};
+
+/// Descriptor used for global variables.
+struct alignas(void *) GlobalInlineDescriptor {
+ GlobalInitState InitState = GlobalInitState::InitializerFailed;
+};
+static_assert(sizeof(GlobalInlineDescriptor) == sizeof(void *), "");
+
/// Inline descriptor embedded in structures and arrays.
///
/// Such descriptors precede all composite array elements and structure fields.
@@ -86,6 +98,7 @@ struct InlineDescriptor {
void dump() const { dump(llvm::errs()); }
void dump(llvm::raw_ostream &OS) const;
};
+static_assert(sizeof(GlobalInlineDescriptor) != sizeof(InlineDescriptor), "");
/// Describes a memory block created by an allocation site.
struct Descriptor final {
@@ -110,6 +123,7 @@ struct Descriptor final {
using MetadataSize = std::optional<unsigned>;
static constexpr MetadataSize InlineDescMD = sizeof(InlineDescriptor);
+ static constexpr MetadataSize GlobalMD = sizeof(GlobalInlineDescriptor);
/// Pointer to the record, if block contains records.
const Record *const ElemRecord = nullptr;
diff --git a/clang/lib/AST/Interp/Pointer.cpp b/clang/lib/AST/Interp/Pointer.cpp
index 85857d4ee1c88..d77cd8943c496 100644
--- a/clang/lib/AST/Interp/Pointer.cpp
+++ b/clang/lib/AST/Interp/Pointer.cpp
@@ -243,8 +243,17 @@ bool Pointer::isInitialized() const {
return IM->second->isElementInitialized(getIndex());
}
+ if (asBlockPointer().Base == 0)
+ return true;
+
+ if (isRoot() && PointeeStorage.BS.Base == sizeof(GlobalInlineDescriptor)) {
+ const GlobalInlineDescriptor &GD =
+ *reinterpret_cast<const GlobalInlineDescriptor *>(block()->rawData());
+ return GD.InitState == GlobalInitState::Initialized;
+ }
+
// Field has its bit in an inline descriptor.
- return PointeeStorage.BS.Base == 0 || getInlineDesc()->IsInitialized;
+ return getInlineDesc()->IsInitialized;
}
void Pointer::initialize() const {
@@ -282,6 +291,13 @@ void Pointer::initialize() const {
return;
}
+ if (isRoot() && PointeeStorage.BS.Base == sizeof(GlobalInlineDescriptor)) {
+ GlobalInlineDescriptor &GD = *reinterpret_cast<GlobalInlineDescriptor *>(
+ asBlockPointer().Pointee->rawData());
+ GD.InitState = GlobalInitState::Initialized;
+ return;
+ }
+
// Field has its bit in an inline descriptor.
assert(PointeeStorage.BS.Base != 0 &&
"Only composite fields can be initialised");
@@ -292,6 +308,10 @@ void Pointer::activate() const {
// Field has its bit in an inline descriptor.
assert(PointeeStorage.BS.Base != 0 &&
"Only composite fields can be initialised");
+
+ if (isRoot() && PointeeStorage.BS.Base == sizeof(GlobalInlineDescriptor))
+ return;
+
getInlineDesc()->IsActive = true;
}
diff --git a/clang/lib/AST/Interp/Pointer.h b/clang/lib/AST/Interp/Pointer.h
index c6e4f4d0b4abd..609367a462b00 100644
--- a/clang/lib/AST/Interp/Pointer.h
+++ b/clang/lib/AST/Interp/Pointer.h
@@ -256,9 +256,7 @@ class Pointer {
if (isIntegralPointer())
return false;
- unsigned Base = asBlockPointer().Base;
- return Base != 0 && Base != sizeof(InlineDescriptor) &&
- Base != RootPtrMark && getFieldDesc()->asDecl();
+ return !isRoot() && getFieldDesc()->asDecl();
}
/// Accessor for information about the declaration site.
@@ -462,9 +460,7 @@ class Pointer {
bool isMutable() const {
if (!isBlockPointer())
return false;
- return asBlockPointer().Base != 0 &&
- asBlockPointer().Base != sizeof(InlineDescriptor) &&
- getInlineDesc()->IsFieldMutable;
+ return !isRoot() && getInlineDesc()->IsFieldMutable;
}
bool isWeak() const {
@@ -627,6 +623,7 @@ class Pointer {
/// Returns the embedded descriptor preceding a field.
InlineDescriptor *getInlineDesc() const {
+ assert(asBlockPointer().Base != sizeof(GlobalInlineDescriptor));
return getDescriptor(asBlockPointer().Base);
}
diff --git a/clang/lib/AST/Interp/Program.cpp b/clang/lib/AST/Interp/Program.cpp
index e3d48d5a8ddb4..7b1f719779e90 100644
--- a/clang/lib/AST/Interp/Program.cpp
+++ b/clang/lib/AST/Interp/Program.cpp
@@ -54,11 +54,11 @@ unsigned Program::createGlobalString(const StringLiteral *S) {
}
// Create a descriptor for the string.
- Descriptor *Desc = allocateDescriptor(S, CharType, Descriptor::InlineDescMD,
- S->getLength() + 1,
- /*isConst=*/true,
- /*isTemporary=*/false,
- /*isMutable=*/false);
+ Descriptor *Desc =
+ allocateDescriptor(S, CharType, Descriptor::GlobalMD, S->getLength() + 1,
+ /*isConst=*/true,
+ /*isTemporary=*/false,
+ /*isMutable=*/false);
// Allocate storage for the string.
// The byte length does not include the null terminator.
@@ -207,11 +207,10 @@ std::optional<unsigned> Program::createGlobal(const DeclTy &D, QualType Ty,
const bool IsConst = Ty.isConstQualified();
const bool IsTemporary = D.dyn_cast<const Expr *>();
if (std::optional<PrimType> T = Ctx.classify(Ty))
- Desc =
- createDescriptor(D, *T, Descriptor::InlineDescMD, IsConst, IsTemporary);
+ Desc = createDescriptor(D, *T, Descriptor::GlobalMD, IsConst, IsTemporary);
else
- Desc = createDescriptor(D, Ty.getTypePtr(), Descriptor::InlineDescMD,
- IsConst, IsTemporary);
+ Desc = createDescriptor(D, Ty.getTypePtr(), Descriptor::GlobalMD, IsConst,
+ IsTemporary);
if (!Desc)
return std::nullopt;
@@ -224,7 +223,9 @@ std::optional<unsigned> Program::createGlobal(const DeclTy &D, QualType Ty,
G->block()->invokeCtor();
// Initialize InlineDescriptor fields.
- new (G->block()->rawData()) InlineDescriptor(Desc);
+ auto *GD = new (G->block()->rawData()) GlobalInlineDescriptor();
+ if (!Init)
+ GD->InitState = GlobalInitState::NoInitializer;
Globals.push_back(G);
return I;
diff --git a/clang/test/AST/Interp/cxx98.cpp b/clang/test/AST/Interp/cxx98.cpp
index e68e4dbc8d74b..471a58f8e0551 100644
--- a/clang/test/AST/Interp/cxx98.cpp
+++ b/clang/test/AST/Interp/cxx98.cpp
@@ -18,11 +18,11 @@ template struct C<cval>;
/// FIXME: This example does not get properly diagnosed in the new interpreter.
extern const int recurse1;
-const int recurse2 = recurse1; // ref-note {{declared here}}
+const int recurse2 = recurse1; // both-note {{declared here}}
const int recurse1 = 1;
int array1[recurse1];
int array2[recurse2]; // ref-warning 2{{variable length array}} \
- // ref-note {{initializer of 'recurse2' is not a constant expression}} \
+ // both-note {{initializer of 'recurse2' is not a constant expression}} \
// expected-warning {{variable length array}} \
// expected-error {{variable length array}}
diff --git a/clang/unittests/AST/Interp/Descriptor.cpp b/clang/unittests/AST/Interp/Descriptor.cpp
index 3157b4d401f98..abcf1bb0714cc 100644
--- a/clang/unittests/AST/Interp/Descriptor.cpp
+++ b/clang/unittests/AST/Interp/Descriptor.cpp
@@ -53,7 +53,7 @@ TEST(Descriptor, Primitives) {
ASSERT_FALSE(GlobalDesc->asRecordDecl());
// Still true because this is a global variable.
- ASSERT_TRUE(GlobalDesc->getMetadataSize() == sizeof(InlineDescriptor));
+ ASSERT_TRUE(GlobalDesc->getMetadataSize() == sizeof(GlobalInlineDescriptor));
ASSERT_FALSE(GlobalDesc->isPrimitiveArray());
ASSERT_FALSE(GlobalDesc->isCompositeArray());
ASSERT_FALSE(GlobalDesc->isZeroSizeArray());
More information about the cfe-commits
mailing list