[clang][bytecode][NFC] Remove instance pointer from emitDestruction (#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.

Author: tbaederr

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

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;

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)

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");

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());