[gem5-dev] Re: deprecating warn_once
Hello, I like the idea of GEM5_ONCE, but as it would become part of the API users could definitely use in unforeseen ways (e.g., debugging). I am not saying this is a bad thing, but it is something to consider. Regards,Daniel ___ 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
[gem5-dev] Re: deprecating warn_once
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 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 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 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("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
>>> 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 <
gem5-dev@gem5.org> 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,
[gem5-dev] Re: deprecating warn_once
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 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 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("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 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 <
>>> gem5-dev@gem5.org> 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 -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
>>>
>>>
___
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
[gem5-dev] Re: deprecating warn_once
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 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("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 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 <
>> gem5-dev@gem5.org> 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 -- gem5-dev@gem5.org
>>> To unsubscribe send an email to gem5-dev-le...@gem5.org
>>> %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
>>
>>
___
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
[gem5-dev] Re: deprecating warn_once
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("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 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
> 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 -- gem5-dev@gem5.org
>> To unsubscribe send an email to gem5-dev-le...@gem5.org
>> %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
>
>
___
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
[gem5-dev] Re: deprecating warn_once
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
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 -- gem5-dev@gem5.org
> To unsubscribe send an email to gem5-dev-le...@gem5.org
> %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
___
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
