[clang][bytecode][NFC] Remove instance pointer from emitDestruction #157040
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) ChangesWe 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. Full diff: https://github.com/llvm/llvm-project/pull/157040.diff 5 Files Affected:
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());
|
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/162/builds/30455 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.