[clang] [clang][bytecode][NFC] Remove instance pointer from emitDestruction (PR #157040)
Timm Baeder via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 5 00:54:47 PDT 2025
https://github.com/tbaederr created https://github.com/llvm/llvm-project/pull/157040
We only call this when we just pushed a new pointer to the stack, so try to save the folling PopPtr op by removing the pointer inside emitDestruction directly, e.g. by letting the Call op just remove it.
>From 4fd1b8e0defdf00ab0095b59e976f561402cbce2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Fri, 5 Sep 2025 09:21:03 +0200
Subject: [PATCH] [clang][bytecode][NFC] Remove instance pointer from
emitDestruction
We only call this when we just pushed a new pointer to the stack, so try
to save the folling PopPtr op by removing the pointer inside
emitDestruction directly, e.g. by letting the Call op just remove it.
---
clang/lib/AST/ByteCode/Compiler.cpp | 84 ++++++++++-----------------
clang/lib/AST/ByteCode/Compiler.h | 8 +--
clang/lib/AST/ByteCode/Descriptor.cpp | 3 +-
clang/lib/AST/ByteCode/Record.cpp | 7 +++
clang/lib/AST/ByteCode/Record.h | 4 ++
5 files changed, 47 insertions(+), 59 deletions(-)
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index 3f30e524ab179..d4e10b32c470c 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -6284,26 +6284,21 @@ bool Compiler<Emitter>::compileDestructor(const CXXDestructorDecl *Dtor) {
// First, destroy all fields.
for (const Record::Field &Field : llvm::reverse(R->fields())) {
const Descriptor *D = Field.Desc;
- if (!D->isPrimitive() && !D->isPrimitiveArray()) {
- if (!this->emitGetPtrField(Field.Offset, SourceInfo{}))
- return false;
- if (!this->emitDestruction(D, SourceInfo{}))
- return false;
- if (!this->emitPopPtr(SourceInfo{}))
- return false;
- }
+ if (D->hasTrivialDtor())
+ continue;
+ if (!this->emitGetPtrField(Field.Offset, SourceInfo{}))
+ return false;
+ if (!this->emitDestructionPop(D, SourceInfo{}))
+ return false;
}
}
for (const Record::Base &Base : llvm::reverse(R->bases())) {
- if (Base.R->isAnonymousUnion())
+ if (Base.R->hasTrivialDtor())
continue;
-
if (!this->emitGetPtrBase(Base.Offset, SourceInfo{}))
return false;
- if (!this->emitRecordDestruction(Base.R, {}))
- return false;
- if (!this->emitPopPtr(SourceInfo{}))
+ if (!this->emitRecordDestructionPop(Base.R, {}))
return false;
}
@@ -7200,71 +7195,56 @@ bool Compiler<Emitter>::emitComplexComparison(const Expr *LHS, const Expr *RHS,
/// on the stack.
/// Emit destruction of record types (or arrays of record types).
template <class Emitter>
-bool Compiler<Emitter>::emitRecordDestruction(const Record *R, SourceInfo Loc) {
+bool Compiler<Emitter>::emitRecordDestructionPop(const Record *R,
+ SourceInfo Loc) {
assert(R);
- assert(!R->isAnonymousUnion());
+ assert(!R->hasTrivialDtor());
const CXXDestructorDecl *Dtor = R->getDestructor();
- if (!Dtor || Dtor->isTrivial())
- return true;
-
assert(Dtor);
const Function *DtorFunc = getFunction(Dtor);
if (!DtorFunc)
return false;
assert(DtorFunc->hasThisPointer());
assert(DtorFunc->getNumParams() == 1);
- if (!this->emitDupPtr(Loc))
- return false;
return this->emitCall(DtorFunc, 0, Loc);
}
/// When calling this, we have a pointer of the local-to-destroy
/// on the stack.
/// Emit destruction of record types (or arrays of record types).
template <class Emitter>
-bool Compiler<Emitter>::emitDestruction(const Descriptor *Desc,
- SourceInfo Loc) {
+bool Compiler<Emitter>::emitDestructionPop(const Descriptor *Desc,
+ SourceInfo Loc) {
assert(Desc);
- assert(!Desc->isPrimitive());
- assert(!Desc->isPrimitiveArray());
+ assert(!Desc->hasTrivialDtor());
// Arrays.
if (Desc->isArray()) {
const Descriptor *ElemDesc = Desc->ElemDesc;
assert(ElemDesc);
- // Don't need to do anything for these.
- if (ElemDesc->isPrimitiveArray())
- return true;
+ unsigned N = Desc->getNumElems();
+ if (N == 0)
+ return this->emitPopPtr(Loc);
- // If this is an array of record types, check if we need
- // to call the element destructors at all. If not, try
- // to save the work.
- if (const Record *ElemRecord = ElemDesc->ElemRecord) {
- if (const CXXDestructorDecl *Dtor = ElemRecord->getDestructor();
- !Dtor || Dtor->isTrivial())
- return true;
- }
-
- if (unsigned N = Desc->getNumElems()) {
- for (ssize_t I = N - 1; I >= 0; --I) {
- if (!this->emitConstUint64(I, Loc))
- return false;
- if (!this->emitArrayElemPtrUint64(Loc))
- return false;
- if (!this->emitDestruction(ElemDesc, Loc))
- return false;
- if (!this->emitPopPtr(Loc))
- return false;
- }
+ for (ssize_t I = N - 1; I >= 1; --I) {
+ if (!this->emitConstUint64(I, Loc))
+ return false;
+ if (!this->emitArrayElemPtrUint64(Loc))
+ return false;
+ if (!this->emitDestructionPop(ElemDesc, Loc))
+ return false;
}
- return true;
+ // Last iteration, removes the instance pointer from the stack.
+ if (!this->emitConstUint64(0, Loc))
+ return false;
+ if (!this->emitArrayElemPtrPopUint64(Loc))
+ return false;
+ return this->emitDestructionPop(ElemDesc, Loc);
}
assert(Desc->ElemRecord);
- if (Desc->ElemRecord->isAnonymousUnion())
- return true;
-
- return this->emitRecordDestruction(Desc->ElemRecord, Loc);
+ assert(!Desc->ElemRecord->hasTrivialDtor());
+ return this->emitRecordDestructionPop(Desc->ElemRecord, Loc);
}
/// Create a dummy pointer for the given decl (or expr) and
diff --git a/clang/lib/AST/ByteCode/Compiler.h b/clang/lib/AST/ByteCode/Compiler.h
index 475faee4d3fde..bb8c660427310 100644
--- a/clang/lib/AST/ByteCode/Compiler.h
+++ b/clang/lib/AST/ByteCode/Compiler.h
@@ -391,8 +391,8 @@ class Compiler : public ConstStmtVisitor<Compiler<Emitter>, bool>,
bool emitComplexBoolCast(const Expr *E);
bool emitComplexComparison(const Expr *LHS, const Expr *RHS,
const BinaryOperator *E);
- bool emitRecordDestruction(const Record *R, SourceInfo Loc);
- bool emitDestruction(const Descriptor *Desc, SourceInfo Loc);
+ bool emitRecordDestructionPop(const Record *R, SourceInfo Loc);
+ bool emitDestructionPop(const Descriptor *Desc, SourceInfo Loc);
bool emitDummyPtr(const DeclTy &D, const Expr *E);
bool emitFloat(const APFloat &F, const Expr *E);
unsigned collectBaseOffset(const QualType BaseType,
@@ -587,11 +587,9 @@ template <class Emitter> class LocalScope : public VariableScope<Emitter> {
if (!this->Ctx->emitGetPtrLocal(Local.Offset, E))
return false;
- if (!this->Ctx->emitDestruction(Local.Desc, Local.Desc->getLoc()))
+ if (!this->Ctx->emitDestructionPop(Local.Desc, Local.Desc->getLoc()))
return false;
- if (!this->Ctx->emitPopPtr(E))
- return false;
removeIfStoredOpaqueValue(Local);
}
return true;
diff --git a/clang/lib/AST/ByteCode/Descriptor.cpp b/clang/lib/AST/ByteCode/Descriptor.cpp
index 647de56b28013..0a819599287ee 100644
--- a/clang/lib/AST/ByteCode/Descriptor.cpp
+++ b/clang/lib/AST/ByteCode/Descriptor.cpp
@@ -457,8 +457,7 @@ bool Descriptor::hasTrivialDtor() const {
if (isRecord()) {
assert(ElemRecord);
- const CXXDestructorDecl *Dtor = ElemRecord->getDestructor();
- return !Dtor || Dtor->isTrivial();
+ return ElemRecord->hasTrivialDtor();
}
if (!ElemDesc)
diff --git a/clang/lib/AST/ByteCode/Record.cpp b/clang/lib/AST/ByteCode/Record.cpp
index c20ec184f34f4..ff5c82d244574 100644
--- a/clang/lib/AST/ByteCode/Record.cpp
+++ b/clang/lib/AST/ByteCode/Record.cpp
@@ -37,6 +37,13 @@ std::string Record::getName() const {
return Ret;
}
+bool Record::hasTrivialDtor() const {
+ if (isAnonymousUnion())
+ return true;
+ const CXXDestructorDecl *Dtor = getDestructor();
+ return !Dtor || Dtor->isTrivial();
+}
+
const Record::Field *Record::getField(const FieldDecl *FD) const {
auto It = FieldMap.find(FD->getFirstDecl());
assert(It != FieldMap.end() && "Missing field");
diff --git a/clang/lib/AST/ByteCode/Record.h b/clang/lib/AST/ByteCode/Record.h
index 93ca43046180a..8245eeff2f20d 100644
--- a/clang/lib/AST/ByteCode/Record.h
+++ b/clang/lib/AST/ByteCode/Record.h
@@ -76,6 +76,10 @@ class Record final {
return nullptr;
}
+ /// Returns true for anonymous unions and records
+ /// with no destructor or for those with a trivial destructor.
+ bool hasTrivialDtor() const;
+
using const_field_iter = FieldList::const_iterator;
llvm::iterator_range<const_field_iter> fields() const {
return llvm::make_range(Fields.begin(), Fields.end());
More information about the cfe-commits
mailing list