hans added a comment.
In D145369#4181296 <https://reviews.llvm.org/D145369#4181296>, @ilya-biryukov
wrote:
> I don't have a lot of experience in codegen, so will let Aaron and Richard do
> the review.
Thanks for checking! I don't have a lot of experience here either, so any
review is much appreciated.
> However, still wanted to share one observation. The actual check that avoids
> emitting the destructors for variables seems more involved than just checking
> if the destructor is `constexpr`.
> Is it safe to rely solely on the type for codegen? E.g. I would have expected
> checks that look at `HasConstantDestruction` for variables with non-trivial
> constructors.
Right, codegen is also factoring in the check about emitting destructors when
deciding about marking the LLVM value constant or not:
in CodeGenModule::EmitGlobalVarDefinition()
bool NeedsGlobalDtor =
!IsDefinitionAvailableExternally &&
D->needsDestruction(getContext()) == QualType::DK_cxx_destructor;
...
// If it is safe to mark the global 'constant', do so now.
GV->setConstant(!NeedsGlobalCtor && !NeedsGlobalDtor &&
isTypeConstant(D->getType(), true));
So I *think* this is correct, and that the `hasConstexprDestructor()` case was
just missing from `isTypeConstant()` because constexpr destructors were added
later. But again, not super familiar with this stuff :)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145369/new/
https://reviews.llvm.org/D145369
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
