dblaikie added a comment. In D119051#3888667 <https://reviews.llvm.org/D119051#3888667>, @rnk wrote:
> I realized I forgot some things I should've mentioned: > > - This probably deserves a release note, if it doesn't have one already that > I missed or forgot about e4ec6ce8a75c208b49b163c81cda90dc7373c791 <https://reviews.llvm.org/rGe4ec6ce8a75c208b49b163c81cda90dc7373c791> > - We should tag #clang-vendors <https://reviews.llvm.org/tag/clang-vendors/>, > since this does change ABI. You've already done the work of guarding it with > -fclang-abi-compat, so that should be OK, we just need to use the > notification mechansim. Yep yep - thanks @aaron.ballman for adding that. In D119051#3889416 <https://reviews.llvm.org/D119051#3889416>, @rnk wrote: > In D119051#3888906 <https://reviews.llvm.org/D119051#3888906>, @erichkeane > wrote: > >> I noticed in a downstream that this changes how we emit structs to IR >> sometimes (see https://godbolt.org/z/74xeq9rTj for an example, but the >> 'no-opaque-pointers' isn't necessary to cause this, just anything that >> causes emission of the struct). >> >> Are we OK with that? Is the 'padding' arrays important in any way? > > The actual layout is different, it now agrees with GCC, see: > https://godbolt.org/z/EK7Tb6YTT > > This is actually a quite significant ABI change, and there are various > mechanisms to stay on the old ABI. Yep, +1 to that. (another dimension to look at, in terms of "is this the right LLVM IR" - basically the GCC Itanium interpretation is that defaulted special members shouldn't, themselves, change the ABI - so comparing Clang with/without the defaulted special member does (now, with the fix applied) have the same IR: https://godbolt.org/z/88WKxP6G9 ) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119051/new/ https://reviews.llvm.org/D119051 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
