Sorry for not responding to this for a while. I think this change is probably a good idea. There is some cost to it, but even when students are adding new debug flags, if they do that successfully (aka it builds), they aren't likely to need to do that very many times. It's likely not something they would need to tweak often, since they're likely to come up with a few categories of DPRINTFs over time and add them once each. It *is* a cost and it would be nice to avoid it, but I think in practice it's probably relatively minor, and it would simplify the code and make it easier to include all the headers you should directly without setting up accidental transitive dependencies.
Gabe On Mon, Jan 18, 2021 at 9:16 AM Jason Lowe-Power via gem5-dev < [email protected]> wrote: > Thanks for bringing this up, Daniel! > > I don't have a strong opinion, personally. I see how this makes things a > little easier for developers. No longer would we have to teach people that > to add a new debug flag you both declare it in the SConscript file and > include the named header file. Given that I've worked with gem5 for a long > time, I don't see that as much overhead, though. > > This would increase the compile time (though, hopefully only rarely). And, > we recently found from the gem5 survey that people don't care that much > about the compile time (more info coming soon). > > I think the biggest downside would be for those that use gem5 in teaching. > Students in architecture classes are the most likely to have underpowered > computers building gem5, and they are also likely to be adding debug flags. > > Cheers, > Jason > > On Fri, Jan 15, 2021 at 5:47 PM Daniel Carvalho via gem5-dev < > [email protected]> wrote: > >> Hello all, >> >> I've uploaded a commit that re-joins the debug header files into a single >> one: https://gem5-review.googlesource.com/c/public/gem5/+/39255. This >> means that instead of having to include a different header for each and >> every debug flag used in a file, only one is needed. For example, in >> src/arch/arm/kvm/arm_cpu.cc >> >> #include "debug/Kvm.hh" >> #include "debug/KvmContext.hh" >> #include "debug/KvmInt.hh" >> >> would be replaced by >> >> #include "debug/flags.hh" >> >> Finally, since most of the code using these flags will use DPRINTF or a >> similar macro, this include could be added to base/trace.hh, so that most >> of the times it would be included transitively. >> >> The advantage of this change is that debugging becomes easier/faster: >> this file would behave as a library of supported flags and very few >> header files would suffice for all debugging needs. The disadvantage is >> that every time a commit introduces or removes a debug flag debug/flags.hh >> will be recompiled, which will cause an almost complete build. Luckily, >> debug flags are not frequently modified. >> >> Performance-wise both approaches are similar. >> >> What are your opinions in this matter? >> >> Cheers, >> Daniel >> _______________________________________________ >> gem5-dev mailing list -- [email protected] >> To unsubscribe send an email to [email protected] >> %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s > > _______________________________________________ > gem5-dev mailing list -- [email protected] > To unsubscribe send an email to [email protected] > %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
_______________________________________________ gem5-dev mailing list -- [email protected] To unsubscribe send an email to [email protected] %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
