Yeah, that's not a bad option either. Between the two, I suggested GEM5_ONCE so that the _once version wouldn't be as inconsistent with the normal version of warn, and even though the GEM5_ONCE macro probably wouldn't be useful outside of warn, it's a pretty generic mechanism and could be. For instance, I think there's exactly one instance of a *_once macro being used in gem5 (I forget if it's inform_once, or hack_once), but if we decide those are worth bringing forward, GEM5_ONCE could be used with those too.
Keeping in mind the inconsistent format, GEM5_WARN_ONCE is fairly verbose, but given its infrequent usage that's not a big deal. Turning warn into GEM5_WARN would be a bigger deal since I think that's used quite a lot, and that name is twice as long. Renaming warn would also have major impact on existing code, and would be unnecessary if warn turned into a normal function. So, I don't think renaming warn_once would be the *wrong* approach, but between the two I do have a minor preference for splitting out the "once"-ness into a macro separately. This is also mostly an academic conversation right now, since until we move our minimum clang version up to 9, and/or move to c++20, I can't turn even the non _once version of those macros into normal functions. Gabe On Tue, Jul 27, 2021 at 10:58 AM Steve Reinhardt <[email protected]> wrote: > Wouldn't we get the same net result by just renaming warn_once() to > GEM5_WARN_ONCE()? Seems simpler 😊. > > Steve > > On Mon, Jul 26, 2021 at 11:03 PM Gabe Black <[email protected]> wrote: > >> Or I should say without un-namespaced macros (GEM5_ prefixed), since >> GEM5_ONCE itself would be a macro. >> >> Gabe >> >> On Mon, Jul 26, 2021 at 11:01 PM Gabe Black <[email protected]> wrote: >> >>> No, not that I'm aware of. It would just be to make it feasible to >>> implement the warn_once functionality without using macros. With c++20, I >>> can more or less get it to work with some minor template syntax, >>> warn<once>("xyz"), but that relies on the source location (file, line, >>> column which may be iffy) to be unique, which is defeated by, for instance, >>> putting multiple warn_once-s in a macro which then all look like they came >>> from the location of the macro in the source. >>> >>> Gabe >>> >>> On Mon, Jul 26, 2021 at 9:01 PM Steve Reinhardt <[email protected]> >>> wrote: >>> >>>> Hi Gabe, >>>> >>>> Is there a use case for GEM5_ONCE() other than warn_once()? >>>> >>>> Thanks, >>>> >>>> Steve >>>> >>>> >>>> On Mon, Jul 26, 2021 at 6:06 PM Gabe Black via gem5-dev < >>>> [email protected]> wrote: >>>> >>>>> Hi folks. I'm continuing to research how to turn warn, panic, etc, >>>>> into regular functions instead of macros, and one particularly sticky >>>>> problem is how to ensure something like warn_once only happens once. >>>>> >>>>> Right now, the macro warn_once expands to something like this: >>>>> >>>>> do { >>>>> static bool once = false; >>>>> if (!once) { >>>>> warn(...); >>>>> once = true; >>>>> } >>>>> } while (0) >>>>> >>>>> So, a static bool is declared which guards the warn. The problem with >>>>> this is that it requires declaring a static bool at the call sight, which >>>>> as you can imagine is hard to do without macros. >>>>> >>>>> As far as how it *might* be done, if we can extract the location of >>>>> the call (file name, source line, column offset), then we could possibly >>>>> use a template holding a static bool. >>>>> >>>>> template <${source location info}> >>>>> warn_once(...) >>>>> { >>>>> static bool once = false; >>>>> .... >>>>> } >>>>> >>>>> There are a few problems with this approach. First, the source >>>>> location would have to be broken down into individual primitive types, >>>>> like >>>>> an int for the line number, and individual characters for the file name >>>>> string, since you can't use a struct as a non-type template parameter >>>>> until >>>>> c++20. This *might* be possible using fancy template tricks, but it would >>>>> be a bit ugly and may gum up the compiler, slowing builds. >>>>> >>>>> Second, if the column information is not unique (I think the standard >>>>> is not very specific about what it maps to), then the "once" will apply to >>>>> more than one thing. This would be particularly true if a macro whose >>>>> contents all share the same source location had multiple warn_once-s in >>>>> it. >>>>> >>>>> I did a check with grep, and warn_once shows up in all of gem5 about >>>>> 80 times, so while it's used, it's not used extensively. >>>>> >>>>> What I would like to propose is that instead of having warn_once(...), >>>>> we add a new macro called GEM5_ONCE which would be defined something like >>>>> the following: >>>>> >>>>> #define GEM5_ONCE(statement) do { \ >>>>> static [[maybe_unused]] bool _once = ([](){ statement; }(), true); >>>>> \ >>>>> while (0) >>>>> >>>>> Then when you want to warn once (or anything else once), you'd write >>>>> it like this: >>>>> >>>>> GEM5_ONCE(warn("blah blah")); >>>>> >>>>> This is *slightly* more verbose, but warn_once is only used 80 times >>>>> in the whole code base. Also the macro is namespaced now, which is a nice >>>>> improvement. >>>>> >>>>> Gabe >>>>> _______________________________________________ >>>>> 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
