[PATCH] D154658: Optimize emission of `dynamic_cast` to final classes.

2023-07-25 Thread Matthias Braun via Phabricator via cfe-commits
MatzeB added a comment. In D154658#4533476 , @MatzeB wrote: > This change results in some of our builds (distributed Thin-LTO in case that > matters) to fail with missing symbols. At a first glance this seems to emit > VTables in some files where it

[PATCH] D154658: Optimize emission of `dynamic_cast` to final classes.

2023-07-25 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In D154658#4533476 , @MatzeB wrote: > This change results in some of our builds (distributed Thin-LTO in case that > matters) to fail with missing symbols. At a first glance this seems to emit > VTables in some files where it

[PATCH] D154658: Optimize emission of `dynamic_cast` to final classes.

2023-07-25 Thread Matthias Braun via Phabricator via cfe-commits
MatzeB added a comment. This change results in some of our builds (distributed Thin-LTO in case that matters) to fail with missing symbols. At a first glance this seems to emit VTables in some files where it didn't do this before and then fails to resolve some members of that vtable. I'm in

[PATCH] D154658: Optimize emission of `dynamic_cast` to final classes.

2023-07-24 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment. This caused https://github.com/llvm/llvm-project/issues/64088 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154658/new/ https://reviews.llvm.org/D154658 ___ cfe-commits mailing

[PATCH] D154658: Optimize emission of `dynamic_cast` to final classes.

2023-07-21 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG9d525bf94b25: Optimize emission of `dynamic_cast` to final classes. (authored by rsmith). Changed prior to commit: https://reviews.llvm.org/D154658?vs=542693=543133#toc Repository: rG LLVM Github

[PATCH] D154658: Optimize emission of `dynamic_cast` to final classes.

2023-07-21 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Thanks, LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154658/new/ https://reviews.llvm.org/D154658

[PATCH] D154658: Optimize emission of `dynamic_cast` to final classes.

2023-07-20 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In D154658#4516554 , @rjmccall wrote: > LGTM, except, should we have a way to turn this optimization off specifically? Sure. I suppose even for an internal linkage vtable there could be a reason to want to turn this off (eg, if

[PATCH] D154658: Optimize emission of `dynamic_cast` to final classes.

2023-07-20 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith updated this revision to Diff 542693. rsmith added a comment. Herald added a subscriber: MaskRay. - Add option to disable vptr-based dynamic_cast optimization. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154658/new/

[PATCH] D154658: Optimize emission of `dynamic_cast` to final classes.

2023-07-19 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. LGTM, except, should we have a way to turn this optimization off specifically? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154658/new/ https://reviews.llvm.org/D154658 ___

[PATCH] D154658: Optimize emission of `dynamic_cast` to final classes.

2023-07-19 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154658/new/ https://reviews.llvm.org/D154658 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D154658: Optimize emission of `dynamic_cast` to final classes.

2023-07-12 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In D154658#4490468 , @rjmccall wrote: > Oh, this does matter on platforms using pointer authentication, because each > vptr field is signed with a constant discriminator derived from the name of > the class that introduced it.

[PATCH] D154658: Optimize emission of `dynamic_cast` to final classes.

2023-07-12 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith updated this revision to Diff 539776. rsmith marked an inline comment as done. rsmith added a comment. Herald added a reviewer: jdoerfert. Herald added subscribers: jplehr, sstefan1. - Avoid emitting the type_info when detecting whether it would be null. - Bring back class type in

[PATCH] D154658: Optimize emission of `dynamic_cast` to final classes.

2023-07-11 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In D154658#4482202 , @rsmith wrote: > In D154658#4481225 , @rjmccall > wrote: > >> In D154658#4479213 , @rsmith wrote: >> >>> I think (hope?)

[PATCH] D154658: Optimize emission of `dynamic_cast` to final classes.

2023-07-07 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith updated this revision to Diff 538283. rsmith added a comment. - Mark gep as inbounds. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154658/new/ https://reviews.llvm.org/D154658 Files: clang/lib/AST/ExprCXX.cpp

[PATCH] D154658: Optimize emission of `dynamic_cast` to final classes.

2023-07-07 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith marked an inline comment as done. rsmith added a comment. In D154658#4481225 , @rjmccall wrote: > In D154658#4479213 , @rsmith wrote: > >> I think (hope?) we should be able to apply this to a much larger

[PATCH] D154658: Optimize emission of `dynamic_cast` to final classes.

2023-07-07 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith updated this revision to Diff 538281. rsmith added a comment. - Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154658/new/ https://reviews.llvm.org/D154658 Files: clang/lib/AST/ExprCXX.cpp

[PATCH] D154658: Optimize emission of `dynamic_cast` to final classes.

2023-07-07 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In D154658#4479213 , @rsmith wrote: > In D154658#4479170 , @rjmccall > wrote: > >> I don't think it's an intended guarantee of the Itanium ABI that the v-table >> will be unique, and

[PATCH] D154658: Optimize emission of `dynamic_cast` to final classes.

2023-07-06 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In D154658#4479170 , @rjmccall wrote: > I don't think it's an intended guarantee of the Itanium ABI that the v-table > will be unique, and v-tables are frequently not unique in the presence of > shared libraries.

[PATCH] D154658: Optimize emission of `dynamic_cast` to final classes.

2023-07-06 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. I don't think it's an intended guarantee of the Itanium ABI that the v-table will be unique, and v-tables are frequently not unique in the presence of shared libraries. They should be unique for classes with internal linkage, but of course that's a vastly reduced

[PATCH] D154658: Optimize emission of `dynamic_cast` to final classes.

2023-07-06 Thread Nuno Lopes via Phabricator via cfe-commits
nlopes added inline comments. Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:1541 +CGF.Builder.CreateBr(CastFail); +return llvm::UndefValue::get(CGF.VoidPtrTy); + } Please use PoisonValue as a placeholder whenever possible. We are trying to get rid of

[PATCH] D154658: Optimize emission of `dynamic_cast` to final classes.

2023-07-06 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith created this revision. rsmith added a reviewer: rjmccall. Herald added subscribers: nlopes, mgrang. Herald added a project: All. rsmith requested review of this revision. Herald added subscribers: cfe-commits, wangpc. Herald added a project: clang. - When the destination is a final class