On Wed, 2014-08-13 at 14:34 -0600, Jeff Law wrote: > On 08/13/14 14:28, David Malcolm wrote: > > Thanks. Although this function gets converted back to a macro in patch > > 191, I just realized that in the meantime that it's not inlined, as is > > the case for some of the other macro->function conversions in patches > > 13-16. > > > > Do I need to convert them to inline functions with the appropriate > > headers, and is that regarded as a sufficiently trivial fix to the stuff > > you've already reviewed to not need re-review? (I will bootstrap&test). > I'd just make it a follow-up. #237 ;-)
Right, but these would be modifications to stuff that only exists between phases 1-3 of the kit, before going away in phase 4, so although it might be #237, it would need to be applied when the individual changes go in. > > Or is it OK to suffer the performance hit as the patchkit lands, before > > they all become macros again in phase 4 of the patchkit? > I think so. This is a transient state, and my goal is to have this > stuff reviewed and get off the critical path before I go on PTO next week. (FWIW I'm on PTO on Friday) Transient, yes, but given the amount of time for me to simply bootstrap each candidate patch, the non-inlined functions could last in trunk for a couple of weeks (there are only 168 hours in a week, and a bootstrap +regrtest takes about 3 hours on my box, so for 236 patches we're talking ~4 weeks of compute time just for that). I guess the underlying point here is that this is a big change and I'm trying to be fastidious here. Murphy's Law suggests I'm going to break at least *something* :( > > Note also that Jakub expressed concern about the effect of all these > > inline functions on the debugging experience, and there's this patch > > (awaiting review) which I believe addresses that: > > https://gcc.gnu.org/ml/gcc-patches/2014-08/msg00743.html > Make it #238. Right. I probably should move the link to the docs from the commit message to a comment in the gdbinit.in itself. Similar ordering considerations apply: I don't want to make debugging painful on trunk for a few weeks - so this kind of thing would need to go in as the inline functions go in. > > Presumably similar changes to gdbinit.in should occur for the relevant > > headers (e.g. df.h in this case, though possibly targeted to just the > > new function - there are already quite a few inline functions in df.h) > Yea, probably. I guess I'll try to get you patches for this by end of tomorrow. Thanks for all your reviewing Dave
