dblaikie added a comment.
In D118511#3373181 <https://reviews.llvm.org/D118511#3373181>, @dblaikie wrote: > In D118511#3373173 <https://reviews.llvm.org/D118511#3373173>, @tstellar > wrote: > >> In D118511#3373082 <https://reviews.llvm.org/D118511#3373082>, @dblaikie >> wrote: >> >>> In D118511#3372728 <https://reviews.llvm.org/D118511#3372728>, @jyknight >>> wrote: >>> >>>> In D118511#3371432 <https://reviews.llvm.org/D118511#3371432>, @tstellar >>>> wrote: >>>> >>>>> I'm fine with reverting if you think this is the best solution. I just >>>>> would like to conclude soon so I can make the final release candidate. >>>> >>>> ISTM that reverting the ABI change in the 14.x branch makes sense, to >>>> avoid ping-ponging the ABI for packed structs which would become >>>> non-packed (breaking ABI) in 14.x and packed again (breaking ABI) in >>>> https://reviews.llvm.org/D119051. >>> >>> Yeah - I think it'd be a pretty niche amount of code that'd churn like >>> that, but doesn't seem super important to rush this either. >>> >>> @tstellar - can/do you want to revert this on the release branch yourself? >>> Is that something I should do? Should I revert this on trunk (would be a >>> bit awkward/more churny for users - maybe not a full revert, but one that >>> leaves the new ABI version flag available as a no-op so users opting out >>> don't need to remove the flag only to add it back in later) so it can be >>> integrated to the release? >> >> I can revert in the release branch. Is this the commit: >> https://reviews.llvm.org/rG277123376ce08c98b07c154bf83e4092a5d4d3c6? > > Yep, that's the one! > >> It doesn't seem necessary to me to revert in the main branch, but I think >> that can be your call. > > Yeah, I'm leaning towards not reverting on main & hoping we can sort out the > POD ABI issue. @tstellar seems we didn't sort out the POD ABI issue in time for the 15 release branch - though we're making some progress on it now (see discussion here: D119051 <https://reviews.llvm.org/D119051> ) - reckon we should again revert the packed ABI layout in 15, to try to put this off until the 3 issues (ignoring packed when packing non-pod (277123376ce08c98b07c154bf83e4092a5d4d3c6 <https://reviews.llvm.org/rG277123376ce08c98b07c154bf83e4092a5d4d3c6>), pod with defaulted special members (D119051 <https://reviews.llvm.org/D119051>), warning for ignoring packed due to non-pod (this patch, D118511 <https://reviews.llvm.org/D118511>)) are put together in one release? Sorry for the churn/slow-movement on these. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118511/new/ https://reviews.llvm.org/D118511 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
