Thanks Gabe, The performance benefit is difficult to measure as the macro is not currently used; it could provide negligible or no improvement at all depending on the compiler and on where it could be used. So the question is not whether it makes gem5 faster now, it is more about giving our users the chance to write *potentially* faster code.
As you were saying the debug::Flag check could be removed by a later change. I'd argue though that a user should be able to catch the unexpected prints. In some cases, those prints would be even useful, signalling some debug only logic has slipped within the non-tracing code. If we take human error into consideration I am more worried about the dangers of silently executing debug only code even when no debug flag is specified. To provide a concrete example from the before mentioned patch [1], it means executing the printQueue method without printing anything. This won't happen if we use a DPRINTF_UNCONDITIONAL Please let me know your thoughts, Kind Regards Giacomo [1]: https://gem5-review.googlesource.com/c/public/gem5/+/47199/7/src/mem/cache/prefetch/queued.cc > -----Original Message----- > From: Gabe Black via gem5-dev <gem5-dev@gem5.org> > Sent: 15 July 2021 23:30 > To: gem5 Developer List <gem5-dev@gem5.org> > Cc: Gabe Black <gabe.bl...@gmail.com> > Subject: [gem5-dev] Re: DPRINTF_UNCONDITIONAL deprecation > > Meant to reply to the list. > > On Thu, Jul 15, 2021 at 3:29 PM Gabe Black <gabe.bl...@gmail.com > <mailto:gabe.bl...@gmail.com> > wrote: > > > Hi Giacomo. The DPRINTF_UNCONDITIONAL macro was used in one > place, and that I converted that place to a normal DPRINTF. Please see here > for explanation for that change, and justification for this type of change in > general: > > commit 07074f40e5a7e6000262a5d37f0be836f6ac92a9 > Author: Gabe Black <gabe.bl...@gmail.com > <mailto:gabe.bl...@gmail.com> > > Date: Thu Apr 29 20:09:32 2021 -0700 > > There are several problems with DPRINTF_UNCONDITIONAL. First, > while macros don't take any run time overhead because they just turn into > c++, and of course you could have just typed that c++ to begin with, they do > have overhead because they make the code more complex and fragile, and > obfuscate what's actually going on. This isn't cost incurred (directly) by the > computer, but by the people who work on the code. This may eventually > translate into cost to the computer, because when code is harder to work > with, it's less likely to be completely correct or optimized. > > Second, the compiler is much better at figuring out and keeping track > of what code is reachable each time you build. Code paths change, new > callers are added, etc, and what may have been already guarded by a debug > variable may not any more. Also, the compiler can already merge together or > drop checks it knows are unnecessary, so I think we may frequently be > dropping something the compiler is already going to drop for us. I think > DPRINTF_UNCONDITIONAL may be a bit of premature optimization without > measurement to say that it's really necessary or even useful. > > I acknowledge that the lack of performance benefit is also an > assumption on my part though, and please let me know if you can measure a > difference in the scenarios you're thinking of. I still think the complexity > and > fragility argument apply, but if the difference is substantial it might be > still be > worth it. > > Gabe > > > > On Thu, Jul 15, 2021 at 6:08 AM Giacomo Travaglini > <giacomo.travagl...@arm.com <mailto:giacomo.travagl...@arm.com> > > wrote: > > > Hi Gabe, > > The DPRINTF_UNCONDITIONAL macro has been deprecated > in develop [1] and the deprecation is going to be official in v21.1. > As far as I understood, the reason why it has been > deprecated is because it was not used in gem5. > > I am proposing to remove the deprecation. IMO that macro is > useful when the Debug flag has already been checked outside of the > Debug print. We recently merged a patch [2] that would have > benefitted from the use of DPRINTF_UNCONDITIONAL. I am quite confident > I can find some other parts of gem5 where the > DPRINTF_UNCONDITIONAL would have been a better printing choice. > I believe speed should still be a prime concern even if we are > tracing, to make the debugging process faster and smoother for our users, > especially > As it comes with no cost (the definition of a macro) > > Let me know what you think about this, and if you don't have > any objection I will push a revert of the patch you posted, and I will convert > some DPRINT into a DPRINTF_UNCONDITIONAL > > Kind Regards > > Giacomo > > [1]: https://gem5- > review.googlesource.com/c/public/gem5/+/44987 > [2]: https://gem5- > review.googlesource.com/c/public/gem5/+/47199/7/src/mem/cache/prefet > ch/queued.cc#134 > IMPORTANT NOTICE: The contents of this email and any > attachments are confidential and may also be privileged. If you are not the > intended recipient, please notify the sender immediately and do not disclose > the contents to any other person, use it for any purpose, or store or copy the > information in any medium. Thank you. > IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. _______________________________________________ gem5-dev mailing list -- gem5-dev@gem5.org To unsubscribe send an email to gem5-dev-le...@gem5.org %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
