[RFA,jit,2/2] introduce scoped_timevar

This introduces a new scoped_timevar class.  It pushes a given timevar
in its constructor, and pops it in the destructor, giving a much
simpler way to use timevars in the typical case where they can be
scoped.
---
 gcc/ChangeLog.jit      |  4 ++++
 gcc/jit/ChangeLog.jit  |  4 ++++
 gcc/jit/internal-api.c | 16 +++++-----------
 gcc/timevar.h          | 24 +++++++++++++++++++++++-
 4 files changed, 36 insertions(+), 12 deletions(-)

On Tue, 2014-03-18 at 08:57 -0600, Tom Tromey wrote:
> This introduces a new scoped_timevar class.  It pushes a given timevar
> in its constructor, and pops it in the destructor, giving a much
> simpler way to use timevars in the typical case where they can be
> scoped.
> ---
>  gcc/ChangeLog.jit      |  4 ++++
>  gcc/jit/ChangeLog.jit  |  4 ++++
>  gcc/jit/internal-api.c | 16 +++++-----------
>  gcc/timevar.h          | 24 +++++++++++++++++++++++-
>  4 files changed, 36 insertions(+), 12 deletions(-)

Thanks.  Looks good to me, but, like the other patch, are the
ChangeLog.jit entries available somewhere?

On Tue, Mar 18, 2014 at 08:57:44AM -0600, Tom Tromey wrote:
> This introduces a new scoped_timevar class.  It pushes a given timevar
> in its constructor, and pops it in the destructor, giving a much
> simpler way to use timevars in the typical case where they can be
> scoped.

thanks for doing this.  I wonder about naming, we already have auto_vec
and while I don't really care wether we use auto_ or scoped_ it seems like
being consistant would be nice.

Trev

> ---
>  gcc/ChangeLog.jit      |  4 ++++
>  gcc/jit/ChangeLog.jit  |  4 ++++
>  gcc/jit/internal-api.c | 16 +++++-----------
>  gcc/timevar.h          | 24 +++++++++++++++++++++++-
>  4 files changed, 36 insertions(+), 12 deletions(-)
> 
> diff --git a/gcc/jit/internal-api.c b/gcc/jit/internal-api.c
> index 6a4d2ae..8285c64 100644
> --- a/gcc/jit/internal-api.c
> +++ b/gcc/jit/internal-api.c
> @@ -3737,8 +3737,6 @@ compile ()
>    if (get_bool_option (GCC_JIT_BOOL_OPTION_DUMP_GENERATED_CODE))
>     dump_generated_code ();
>  
> -  timevar_push (TV_ASSEMBLE);
> -
>    /* Gross hacks follow:
>       We have a .s file; we want a .so file.
>       We could reuse parts of gcc/gcc.c to do this.
> @@ -3746,6 +3744,8 @@ compile ()
>     */
>    /* FIXME: totally faking it for now, not even using pex */
>    {
> +    scoped_timevar assemble_timevar (TV_ASSEMBLE);
> +
>      char cmd[1024];
>      snprintf (cmd, 1024, "gcc -shared %s -o %s",
>                m_path_s_file, m_path_so_file);
> @@ -3753,20 +3753,16 @@ compile ()
>        printf ("cmd: %s\n", cmd);
>      int ret = system (cmd);
>      if (ret)
> -      {
> -	timevar_pop (TV_ASSEMBLE);
> -	return NULL;
> -      }
> +      return NULL;
>    }
> -  timevar_pop (TV_ASSEMBLE);
>  
>    // TODO: split out assembles vs linker
>  
>    /* dlopen the .so file. */
>    {
> -    const char *error;
> +    scoped_timevar load_timevar (TV_LOAD);
>  
> -    timevar_push (TV_LOAD);
> +    const char *error;
>  
>      /* Clear any existing error.  */
>      dlerror ();
> @@ -3779,8 +3775,6 @@ compile ()
>        result_obj = new result (handle);
>      else
>        result_obj = NULL;
> -
> -    timevar_pop (TV_LOAD);
>    }
>  
>    return result_obj;
> diff --git a/gcc/timevar.h b/gcc/timevar.h
> index dc2a8bc..eb8bf0d 100644
> --- a/gcc/timevar.h
> +++ b/gcc/timevar.h
> @@ -1,5 +1,5 @@
>  /* Timing variables for measuring compiler performance.
> -   Copyright (C) 2000-2013 Free Software Foundation, Inc.
> +   Copyright (C) 2000-2014 Free Software Foundation, Inc.
>     Contributed by Alex Samuel <samuel@codesourcery.com>
>  
>     This file is part of GCC.
> @@ -110,6 +110,28 @@ timevar_pop (timevar_id_t tv)
>      timevar_pop_1 (tv);
>  }
>  
> +class scoped_timevar
> +{
> + public:
> +  scoped_timevar (timevar_id_t tv)
> +    : m_tv (tv)
> +  {
> +    timevar_push (m_tv);
> +  }
> +
> +  ~scoped_timevar ()
> +  {
> +    timevar_push (m_tv);
> +  }
> +
> + private:
> +
> +  // Private to disallow copies.
> +  scoped_timevar (const scoped_timevar &);
> +
> +  timevar_id_t m_tv;
> +};
> +
>  extern void print_time (const char *, long);
>  
>  #endif /* ! GCC_TIMEVAR_H */
> -- 
> 1.8.5.3
>

>>>>> "Trevor" == Trevor Saunders <tsaunders@mozilla.com> writes:

Trevor> thanks for doing this.  I wonder about naming, we already have
Trevor> auto_vec and while I don't really care wether we use auto_ or
Trevor> scoped_ it seems like being consistant would be nice.

Sounds reasonable to me, I've made this change for v2.

Tom