https://github.com/efriedma-quic created https://github.com/llvm/llvm-project/pull/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. >From 4a3612bcf0e6dd3a68e2b648bb662b4faf154b26 Mon Sep 17 00:00:00 2001 From: Eli Friedman <efrie...@quicinc.com> Date: Fri, 26 Apr 2024 16:58:57 -0700 Subject: [PATCH] [clang codegen] Delete unnecessary GEP cleanup code. 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 355fec42be4489..61db6293ed6a1e 100644 --- a/clang/lib/CodeGen/CGExprAgg.cpp +++ b/clang/lib/CodeGen/CGExprAgg.cpp @@ -1811,7 +1811,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()); @@ -1819,17 +1818,8 @@ void AggExprEmitter::VisitCXXParenListOrInitListExpr( CGF.pushDestroy(EHCleanup, LV.getAddress(CGF), field->getType(), CGF.getDestroyer(dtorKind), false); addCleanup(CGF.EHStack.stable_begin()); - 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(); } // Deactivate all the partial cleanups in reverse order, which diff --git a/clang/test/CodeGenCXX/no-unique-address.cpp b/clang/test/CodeGenCXX/no-unique-address.cpp index 7b4bbbf2a05d51..82532c5e1be82a 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{}; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits