On Tue, Oct 14, 2014 at 5:51 PM, David Malcolm <dmalc...@redhat.com> wrote:
> On Tue, 2014-10-14 at 11:03 +0200, Richard Biener wrote:
>> On Mon, Oct 13, 2014 at 7:45 PM, David Malcolm <dmalc...@redhat.com> wrote:
>> > This is used in a couple of places in jit/jit-playback.c to ensure
>> > that we pop the timevar on every exit path from a function.
>> >
>> > I could rewrite them if need be, but it does simplify things.
>>
>> Sorry to be bikeshedding but auto_timevar sounds odd - this is
>> just a one-element timevar stack.
>
> Sorry that the usage examples didn't make it through in my original
> email; these are in patch 06/10 in gcc/jit/jit-playback.c and look like
> this:
>
> playback::context::
> compile ()
> {
> ... lots of code...
>
> {
> auto_timevar assemble_timevar (TV_ASSEMBLE);
>
> ... lots of code, with multiple return paths...
>
> }
>
> }
>
> the idea being that the timevar_pop happens implicitly at the exit from
> the scope (e.g. via one of the error-handling returns).
>
> FWIW I rather like the current name: I think of it as an RAII-style way
> of not having to manually call timevar_pop.
>
> The "auto_" prefix to me evokes both such RAII types as "auto_ptr" and
> "auto_vec", and the fact that it's intended to be on the stack i.e. have
> "auto" storage class.
Yeah, I got that. Anyway, objection withdrawn (ok, it really wasn't an
objection).
The patch is ok.
Thanks,
Richard.
>> Don't have a real better name though :/ Maybe timevar_pushpop ?
>>
>> Otherwise this looks ok.
>>
>> Thanks,
>> Richard.
>>
>> > Written by Tom Tromey.
>> >
>> > gcc/ChangeLog:
>> > * timevar.h (class auto_timevar): New class.
>> > ---
>> > gcc/timevar.h | 24 ++++++++++++++++++++++++
>> > 1 file changed, 24 insertions(+)
>> >
>> > diff --git a/gcc/timevar.h b/gcc/timevar.h
>> > index 6703cc9..f018e39 100644
>> > --- a/gcc/timevar.h
>> > +++ b/gcc/timevar.h
>> > @@ -110,6 +110,30 @@ timevar_pop (timevar_id_t tv)
>> > timevar_pop_1 (tv);
>> > }
>> >
>> > +// This is a simple timevar wrapper class that pushes a timevar in its
>> > +// constructor and pops the timevar in its destructor.
>> > +class auto_timevar
>> > +{
>> > + public:
>> > + auto_timevar (timevar_id_t tv)
>> > + : m_tv (tv)
>> > + {
>> > + timevar_push (m_tv);
>> > + }
>> > +
>> > + ~auto_timevar ()
>> > + {
>> > + timevar_pop (m_tv);
>> > + }
>> > +
>> > + private:
>> > +
>> > + // Private to disallow copies.
>> > + auto_timevar (const auto_timevar &);
>> > +
>> > + timevar_id_t m_tv;
>> > +};
>> > +
>> > extern void print_time (const char *, long);
>> >
>> > #endif /* ! GCC_TIMEVAR_H */
>> > --
>> > 1.8.5.3
>> >
>
>
