[clang] cbf6e93 - [clang codegen] Delete unnecessary GEP cleanup code. (#90303)

[via cfe-commits]

Author: Eli Friedman
Date: 2024-05-28T20:47:49-07:00
New Revision: cbf6e93ceee7b9de2b7c3e7e8cea3a972eda0e75

URL: https://github.com/llvm/llvm-project/commit/cbf6e93ceee7b9de2b7c3e7e8cea3a972eda0e75
DIFF: https://github.com/llvm/llvm-project/commit/cbf6e93ceee7b9de2b7c3e7e8cea3a972eda0e75.diff

LOG: [clang codegen] Delete unnecessary GEP cleanup code. (#90303)

There's some code in AggExprEmitter::VisitCXXParenListOrInitListExpr to
try to do early cleanup for GEPs for fields that aren't accessed. But
it's unlikely to actually save significant compile-time, and it's subtly
wrong in cases where EmitLValueForFieldInitialization() doesn't create a
GEP. So just delete the code.

Fixes #88077. Fixes #89547.

Added: 
    

Modified: 
    clang/lib/CodeGen/CGExprAgg.cpp
    clang/test/CodeGenCXX/no-unique-address.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/CGExprAgg.cpp b/clang/lib/CodeGen/CGExprAgg.cpp
index bba00257fd4f0..7a92fc3dfb4a4 100644
--- a/clang/lib/CodeGen/CGExprAgg.cpp
+++ b/clang/lib/CodeGen/CGExprAgg.cpp
@@ -1789,7 +1789,6 @@ void AggExprEmitter::VisitCXXParenListOrInitListExpr(
     // Push a destructor if necessary.
     // FIXME: if we have an array of structures, all explicitly
     // initialized, we can end up pushing a linear number of cleanups.
-    bool pushedCleanup = false;
     if (QualType::DestructionKind dtorKind
           = field->getType().isDestructedType()) {
       assert(LV.isSimple());
@@ -1797,17 +1796,8 @@ void AggExprEmitter::VisitCXXParenListOrInitListExpr(
         CGF.pushDestroyAndDeferDeactivation(NormalAndEHCleanup, LV.getAddress(),
                                             field->getType(),
                                             CGF.getDestroyer(dtorKind), false);
-        pushedCleanup = true;
       }
     }
-
-    // If the GEP didn't get used because of a dead zero init or something
-    // else, clean it up for -O0 builds and general tidiness.
-    if (!pushedCleanup && LV.isSimple())
-      if (llvm::GetElementPtrInst *GEP =
-              dyn_cast<llvm::GetElementPtrInst>(LV.emitRawPointer(CGF)))
-        if (GEP->use_empty())
-          GEP->eraseFromParent();
   }
 }
 

diff  --git a/clang/test/CodeGenCXX/no-unique-address.cpp b/clang/test/CodeGenCXX/no-unique-address.cpp
index 7b4bbbf2a05d5..82532c5e1be82 100644
--- a/clang/test/CodeGenCXX/no-unique-address.cpp
+++ b/clang/test/CodeGenCXX/no-unique-address.cpp
@@ -101,3 +101,28 @@ struct HasZeroSizedFieldWithNonTrivialInit {
 HasZeroSizedFieldWithNonTrivialInit testHasZeroSizedFieldWithNonTrivialInit = {.a = 1};
 // CHECK-LABEL: define {{.*}}cxx_global_var_init
 // CHECK: call {{.*}}@_ZN14NonTrivialInitC1Ev({{.*}}@testHasZeroSizedFieldWithNonTrivialInit
+
+void *operator new(unsigned long, void *);
+template <class Ty>
+struct _box {
+  [[no_unique_address]] Ty _value;
+};
+// Make sure this doesn't crash.
+// CHECK-LABEL: define {{.*}}placement_new_struct
+void placement_new_struct() {
+  struct set_value_t {};
+
+  // GH88077
+  struct _tuple : _box<set_value_t>, _box<int> {};
+
+  int _storage[1];
+  new (_storage) _tuple{};
+
+  // GH89547
+  struct _tuple2 {
+    _box<set_value_t> a;
+  };
+
+  int _storage2[1];
+  new (_storage2) _tuple2{};
+}