From cbf6e93ceee7b9de2b7c3e7e8cea3a972eda0e75 Mon Sep 17 00:00:00 2001 From: Eli Friedman Date: Tue, 28 May 2024 20:47:49 -0700 Subject: [PATCH] [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. --- clang/lib/CodeGen/CGExprAgg.cpp | 10 --------- clang/test/CodeGenCXX/no-unique-address.cpp | 25 +++++++++++++++++++++ 2 files changed, 25 insertions(+), 10 deletions(-) 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(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 +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, _box {}; + + int _storage[1]; + new (_storage) _tuple{}; + + // GH89547 + struct _tuple2 { + _box a; + }; + + int _storage2[1]; + new (_storage2) _tuple2{}; +}