[Bug c++/62306] [4.9/5 Regression?] Change in the comdat used for constructors
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62306 --- Comment #11 from Jason Merrill jason at gcc dot gnu.org --- I'm not too concerned about the change.(In reply to Jakub Jelinek from comment #9) 4.5+4.6+4.9 is more released compilers than 4.7/4.8 though, and 4.9 is already widely deployed too, IMHO it is worse to change this again mid-release. Fair enough. I'm not too concerned about the mismatch either way; even in 4.7/4.8 any TU that emits D1/2 also emits D0, so we aren't going to have link failures.
[Bug c++/62306] [4.9/5 Regression?] Change in the comdat used for constructors
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62306 Jason Merrill jason at gcc dot gnu.org changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |INVALID Assignee|unassigned at gcc dot gnu.org |jason at gcc dot gnu.org --- Comment #12 from Jason Merrill jason at gcc dot gnu.org --- So, intentional. And the test added for 62302 covers this as well.
[Bug c++/62306] [4.9/5 Regression?] Change in the comdat used for constructors
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62306 --- Comment #10 from Rafael Avila de Espindola rafael.espindola at gmail dot com --- (In reply to Jakub Jelinek from comment #9) (In reply to Jason Merrill from comment #8) I think I'm sympathetic to Rafael's argument that we should stick with the 4.7 behavior since that's what most deployed GCCs currently do. 4.5+4.6+4.9 is more released compilers than 4.7/4.8 though, and 4.9 is already widely deployed too, IMHO it is worse to change this again mid-release. Another option to start using a D6 COMDAT that is defined to always contain D1 and D2 and never contain D0. This would mean a small bloat when mixing some .o files compiled with 5 and some with previous releases, but would also mean better codegen and a clean state for 5 and newer.
[Bug c++/62306] [4.9/5 Regression?] Change in the comdat used for constructors
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62306 --- Comment #8 from Jason Merrill jason at gcc dot gnu.org --- So, yeah. When we were originally developing the ABI, we wanted to allow implementations to make all of the symbols entry points to the same function. But this didn't end up being reflected in the ABI document, which doesn't currently allow for sharing a COMDAT group at all: Implicitly-defined or inline user-defined constructors and destructors are emitted where referenced, each in its own COMDAT group identified by the constructor or destructor name. I started a discussion about this on the ABI list in 2009 but it didn't conclude. It sounds like most other compilers still put the destructors in different groups. clang avoids creating two identical clones by just omitting the complete-object clone (and putting the base clone in the vtable), which also seems nonconformant. Apparently HP puts all the constructors in a C3 group but uses separate groups for the destructors. So we're already incompatible with everyone else on this, though it's pretty harmless because the worst that can happen is a bit of extra bloat. So we don't need to consider compatibility with other vendors in this decision and can just do what's right for us. I think I'm sympathetic to Rafael's argument that we should stick with the 4.7 behavior since that's what most deployed GCCs currently do.
[Bug c++/62306] [4.9/5 Regression?] Change in the comdat used for constructors
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62306 --- Comment #9 from Jakub Jelinek jakub at gcc dot gnu.org --- (In reply to Jason Merrill from comment #8) I think I'm sympathetic to Rafael's argument that we should stick with the 4.7 behavior since that's what most deployed GCCs currently do. 4.5+4.6+4.9 is more released compilers than 4.7/4.8 though, and 4.9 is already widely deployed too, IMHO it is worse to change this again mid-release.
[Bug c++/62306] [4.9/5 Regression?] Change in the comdat used for constructors
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62306 --- Comment #3 from Jakub Jelinek jakub at gcc dot gnu.org --- I don't remember, but IRC log does something: IRC #gcc on oftc Dec 1 2009 jakub jason: so, I have a patch to handle the implicit cdtors in extern template class by exporting the cdtors at the explicit instantiation side and never emit them at the side with just extern template class jakub jason: which cures the dtor either D[012], or none of them check for virtual dtors jason I would only do that for virtual dtors jakub jason: unfortunately it is probably an incompatible ABI change, given that we weren't previously exporting them all at the point of explicit instantiation jason mm jason true jakub jason: with the patch a few libstdc++ tests fail, because _ZNSt15basic_stringbufIcSt11char_traitsIcESaIcEED2Ev is not exported from libstdc++.so.6 jason that's with an old libstdc++? jakub jason: while we could export it now, similar issue could be with other C++ shared libraries - compiled with older g++ they wouldn't export some dtors that new g++ compiled code would now expect in it jason right jakub jason: that is with new libstdc++, which has now those dtors emitted, but hidden by the linker script jason so, I don't think that's worth doing jakub jason: as I wrote, in the libstdc++ case we could export them at GLIBCXX_3.4.14 jakub jason: so, can I change the patch not to emit *D0* deleting dtor in the *D5* group then? jakub jason: or do you see other options? jason what's the problem with just emitting them as needed like we do now? jakub jason: if the *D5* comdat group is emitted sometimes with all of *D[012]* and sometimes with just *D[12]* (and perhaps sometimes with just *D0* symbols in it richi emit them as needed outside of any comdat group? jakub jason: then the linker will pick just one of the *D5* comdat groups, and depending on which symbols that group has, it will either allow stuff to be linked or not jason the D5 comdat group should always be emitted with all of D[012]. jason that's separate from what we export from libstdc++ jason (D[012] or D[12] depending on whether or not D0 exists) jakub jason: if it is a requirement that we always emit all of D[012] if dtor is virtual, then we'll just emit bloat jason why? jakub jason: because we don't need the D0 anywhere jason I see jakub jason: say we have libfoo.so which contains extern template class S5; template class S5; where S has trivial virtual dtor jakub jason: that library will likely have D[012] emitted jakub jason: then in some other DSO we just use extern template class S5;, current g++ (unfortunately) will emit D[12] when those are referenced by the code jakub jason: if we need D[012] in D5 comdat group, it would mean we have to also emit *D0* in that other DSO jason but D0 is just two calls jason and we won't emit anything if inlining is on jason oddly, the ABI doesn't seem to say anything about explicit instantiation jakub jason: true, D0 is just two calls jakub jason: it is not true that we don't emit anything if inlining is on, the inliner apparently doesn't inline in many cases when it thinks the caller is cold jason ah jakub jason: plus, I have no idea how to teach cgraph etc. that it should either emit all of D[012], or none of them jakub jason: this is quite different case than the same body aliases, in this case it is two different functions jakub jason: that's going to be quite non-trivial especially on the lto side :( jason My thinking was that the ABI change should also support implementations that implement D0 as another entry point into the destructor jakub jason: I understand that reason (though I think the 2 calls solution is far more likely, as D0 is really D1 plus delete) jakub jason: I'm just saying that it has a lot of issues jakub jason: perhaps we could avoid doing the same body optimization (== use D5 group at all) when 1) DECL_ONE_ONLY 2) the virtual dtor is trivial 3) extern template class was seen jakub jason: at least as a short term solution, to at least optimize all the other cases jason jakub: I suppose that's ok as a short term solution jason s/trivial/implicitly defined/ jason what do we do for user-defined inline [cd]tors? jakub jason: those are either inlined successfully, or external symbol references jason so we handle synthesized methods differently from user inlines? oops jason they ought to work the same jakub jason: the problem is that do_type_instantiation ignores FUNCTION_DECLs with !DECL_TEMPLATE_INSTANTIATION and that's the case for artificial methods jakub jason: let me post the patch jakub jason: pt.c patch with description posted jakub jason: is there a way to check for the extern template class seen? jason DECL_EXPLICIT_INSTANTIATION DECL_REALLY_EXTERN jason or CLASSTYPE_ jakub jason: these synthetized methods never have any DECL_TEMPLATE_USE non-zero jason right, would need to check CLASSTYPE_ jakub jason: CLASSTYPE_EXPLICIT_INSTANTIATION (DECL_CONTEXT ()) will also trigger
[Bug c++/62306] [4.9/5 Regression?] Change in the comdat used for constructors
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62306 --- Comment #4 from Rafael Avila de Espindola rafael.espindola at gmail dot com --- So it looks like the reason was jason My thinking was that the ABI change should also support implementations that implement D0 as another entry point into the destructor jakub jason: leaving D0 out of D5 would be easiest, but would perhaps be a problem for other implementations - But I am not sure what the extra entry point would look like. D0 has a call to delete *after* the call to the other destructor, so there is no tail that could be used as an alternative entry point.
[Bug c++/62306] [4.9/5 Regression?] Change in the comdat used for constructors
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62306 --- Comment #5 from Jakub Jelinek jakub at gcc dot gnu.org --- You could e.g. implement the dtors as: _Z...D0...: set some hard reg to 0 tail call some function _Z...D1...: set some hard reg to 1 tail call some function _Z...D2...: set some hard reg to 2 tail call some function and that function would just have a magic extra argument, if the same in between all levels would not use it at all, otherwise could e.g. have if (extra == 0) delete ...; at the end, etc. My memory is fuzzy on it after all the years, but as the comdat names are part of the ABI, I think the idea was to allow implementations to choose how to implement it while staying interoperable. So, if g++ 4.[569]/5 emit it in D5, I think that is how it was meant, and what should be done.
[Bug c++/62306] [4.9/5 Regression?] Change in the comdat used for constructors
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62306 --- Comment #6 from Rafael Avila de Espindola rafael.espindola at gmail dot com --- OK, so should we declare r206182 an unintentional bug fix and mark this bug wontfix? To be clear, the ABI then is For any class an implementation has the option of using one comdat per constructor/destructor or using a C5/D5 comdat. I may make that decision based on any profitability criterion. If using a C5/D5 comdat the rules are * A C5 comdat must have C1 and C2. * If a class has a virtual destructor, the D5 comdat must have D0, D1 and D2 * If a class has a non-virtual destructor, the D5 comdat must have only the D1 and D2 destructors. That is true even if the implementation uses D0 instead of a call to D1 + _ZdlPv to implement delete *x Should this be documented in https://refspecs.linuxbase.org/cxxabi-1.86.html ?
[Bug c++/62306] [4.9/5 Regression?] Change in the comdat used for constructors
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62306 --- Comment #7 from Rafael Avila de Espindola rafael.espindola at gmail dot com --- (In reply to Rafael Avila de Espindola from comment #6) OK, so should we declare r206182 an unintentional bug fix and mark this bug wontfix? One thing to keep in mind. If r206182 was the bug fix, r176071 was the revision that introduced the bug. That was when trunk was 4.7.0. A quick test with the current branches shows that for the example in the bug description 4.6: Puts D0 in D5 4.7: Puts D0 in D0 4.8: Puts D0 in D0 4.9: Puts D0 in D5 trunk: Puts D0 in D5 in the case of the 4.9 branch, Given that most linux distros are still with 4.8, the safest thing to do might be to say that 4.7 changed the ABI and patch 4.9 and trunk to put D0 in its own comdat.
[Bug c++/62306] [4.9/5 Regression?] Change in the comdat used for constructors
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62306 --- Comment #2 from Rafael Avila de Espindola rafael.espindola at gmail dot com --- This is again visible on trunk now that pr62302 has been fixed (thanks!). It doesn't seem profitable to ever put D0 in the D5 comdat. It cannot be equal to D1 or D2, so putting it there just constrains the linker unnecessarily.
[Bug c++/62306] [4.9/5 Regression?] Change in the comdat used for constructors
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62306 Jason Merrill jason at gcc dot gnu.org changed: What|Removed |Added Status|UNCONFIRMED |NEW Last reconfirmed||2014-08-29 CC|jason at redhat dot com|jakub at gcc dot gnu.org, ||jason at gcc dot gnu.org Ever confirmed|0 |1 --- Comment #1 from Jason Merrill jason at gcc dot gnu.org --- So, the change to use D5 for the deleting dtor was deliberate when we started using D5 for the base/complete dtor aliases in 4.5. Then 4.7 started using D0 again for some reason, and then 4.9 went back to D5. Jakub, do you remember why you wanted to put the deleting dtor in D5 in the first place? The gcc-patches threads don't seem to mention why. https://gcc.gnu.org/ml/gcc-patches/2009-11/threads.html#00700 https://gcc.gnu.org/ml/gcc-patches/2009-11/threads.html#01768 https://gcc.gnu.org/ml/gcc-patches/2009-12/threads.html#00023
