strimo378 created this revision.
Herald added a project: All.
strimo378 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Clang codegen emits an unnecessary memcpy for C++ class without content. This 
patch fixes the issue.

See https://github.com/llvm/llvm-project/issues/59336

Please let me know if the patch is going into the right direction that it can 
be accepted. If yes, I'll finish it with test cases and also extend it to C. 
Currently, check-clang is not showing an error...


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158267

Files:
  clang/lib/CodeGen/CGExprAgg.cpp


Index: clang/lib/CodeGen/CGExprAgg.cpp
===================================================================
--- clang/lib/CodeGen/CGExprAgg.cpp
+++ clang/lib/CodeGen/CGExprAgg.cpp
@@ -2099,6 +2099,34 @@
   return AggValueSlot::MayOverlap;
 }
 
+static bool hasSomethingToCopy(const ASTContext &Ctx, QualType Ty) {
+  if (const RecordType *RT = Ty->getAs<RecordType>()) {
+    CXXRecordDecl *Record = cast<CXXRecordDecl>(RT->getDecl());
+
+    if (Record->isEmpty())
+      return false;
+    if (Record->isDynamicClass())
+      return true;
+
+    for (auto *Field : Record->fields()) {
+      if (Field->isZeroLengthBitField(Ctx))
+        continue;
+
+      if (hasSomethingToCopy(Ctx, Field->getType()))
+        return true;
+    }
+
+    for (auto &Base : Record->bases()) {
+      if (hasSomethingToCopy(Ctx, Base.getType()))
+        return true;
+    }
+
+    return false;
+  } else {
+    return true;
+  }
+}
+
 void CodeGenFunction::EmitAggregateCopy(LValue Dest, LValue Src, QualType Ty,
                                         AggValueSlot::Overlap_t MayOverlap,
                                         bool isVolatile) {
@@ -2118,7 +2146,7 @@
              "Trying to aggregate-copy a type without a trivial copy/move "
              "constructor or assignment operator");
       // Ignore empty classes in C++.
-      if (Record->isEmpty())
+      if (!hasSomethingToCopy(getContext(), Ty))
         return;
     }
   }


Index: clang/lib/CodeGen/CGExprAgg.cpp
===================================================================
--- clang/lib/CodeGen/CGExprAgg.cpp
+++ clang/lib/CodeGen/CGExprAgg.cpp
@@ -2099,6 +2099,34 @@
   return AggValueSlot::MayOverlap;
 }
 
+static bool hasSomethingToCopy(const ASTContext &Ctx, QualType Ty) {
+  if (const RecordType *RT = Ty->getAs<RecordType>()) {
+    CXXRecordDecl *Record = cast<CXXRecordDecl>(RT->getDecl());
+
+    if (Record->isEmpty())
+      return false;
+    if (Record->isDynamicClass())
+      return true;
+
+    for (auto *Field : Record->fields()) {
+      if (Field->isZeroLengthBitField(Ctx))
+        continue;
+
+      if (hasSomethingToCopy(Ctx, Field->getType()))
+        return true;
+    }
+
+    for (auto &Base : Record->bases()) {
+      if (hasSomethingToCopy(Ctx, Base.getType()))
+        return true;
+    }
+
+    return false;
+  } else {
+    return true;
+  }
+}
+
 void CodeGenFunction::EmitAggregateCopy(LValue Dest, LValue Src, QualType Ty,
                                         AggValueSlot::Overlap_t MayOverlap,
                                         bool isVolatile) {
@@ -2118,7 +2146,7 @@
              "Trying to aggregate-copy a type without a trivial copy/move "
              "constructor or assignment operator");
       // Ignore empty classes in C++.
-      if (Record->isEmpty())
+      if (!hasSomethingToCopy(getContext(), Ty))
         return;
     }
   }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D158267: [clang][Code... Timo Stripf via Phabricator via cfe-commits

Reply via email to