Re: [PATCH 3/5] timevar.h: Add an auto_timevar class

[Richard Biener <richard.guenther@gmail.com>]

On Tue, Oct 14, 2014 at 5:51 PM, David Malcolm <dmalcolm@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 <dmalcolm@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
>> >
>
>