This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] timevar TLC


On 8/14/12, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
> On Aug 14, 2012 Diego Novillo <dnovillo@google.com> wrote:
> > On 12-08-14 14:26 , Steven Bosscher wrote:
> > > Many unused timevars, many timevars that measure completely
> > > different passes, passes with the wrong timevar, etc.
> > >
> > > Time for a bit of maintenance / janitorial.
> > >
> > > Bootstrapped&tested on powerpc64-unknown-linux-gnu. OK
> > > for trunk?
> > >
> > >    * timevar.def (TV_VARPOOL, TV_WHOPR_WPA_LTRANS_EXEC, TV_LIFE,
> > >    TV_LIFE_UPDATE, TV_DF_UREC, TV_INLINE_HEURISTICS,
> > >    TV_TREE_LINEAR_TRANSFORM, TV_TREE_LOOP_INIT, TV_TREE_LOOP_FINI,
> > >    TV_VPT, TV_LOCAL_ALLOC, TV_GLOBAL_ALLOC, TV_SEQABSTR): Remove.
> > >    (TV_IPA_INLINING, TV_FLATTEN_INLINING, TV_EARLY_INLINING,
> > >    TV_INLINE_PARAMETERS, TV_LOOP_INIT, TV_LOOP_FINI): New.
> > >    * timevar.c (timevar_print): Make printing width of timevar
> > > names
> > >    more flexible, but enforce maximum length.
> > >    * ipa-inline.c (pass_early_inline): Use TV_EARLY_INLINING.
> > >    (pass_ipa_inline): Use TV_IPA_INLINING.
> > >    * ipa-inline-analysis.c (pass_inline_parameters): Use
> > >    TV_INLINE_HEURISTICS.
> > >    * tree-ssa-loop.c (pass_tree_loop_init): No timevar for wrapper
> > > pass.
> > >    (pass_tree_loop_done): Likewise.
> > >    * final.c (pass_shorten_branches): Use TV_SHORTEN_BRANCH.
> > >    * loop-init.c (loop_optimizer_init): Push/pop TV_LOOP_INIT.
> > >    (loop_optimizer_finalize): Push/pop TV_LOOP_FINI.
> >
> > Looks fine, except:
> >
> > > @@ -505,6 +507,16 @@ timevar_print (FILE *fp)
> > >    TIMEVAR.  */
> > >    start_time = now;
> > >
> > > +#ifdef ENABLE_CHECKING
> > > +  /* Pester those who add timevars with too long names.  */
> > > +  for (id = 0; id < (unsigned int) TIMEVAR_LAST; ++id)
> > > +    {
> > > +      struct timevar_def *tv = &timevars[(timevar_id_t) id];
> > > +      if ((timevar_id_t) id != TV_TOTAL && tv->used)
> > > +       gcc_assert (strlen (tv->name) <= name_width);
> > > +    }
> > > +#endif
> >
> > I'm not liking this too much.  I would rather do truncation
> > or wrapping.  Not ICEing.  And we'd do this all the time,
> > not just with checking enabled.
>
> Wrapping would be bad, it'd break scripts that parse the
> -ftime-report output.  Truncation is a bit silly, too: If the name
> is always truncated, why have the long name in the first place?
>
> I chose for this gcc_assert solution to make absolutely sure
> nobody can add a timevar with an overlong name.
>
> > I suppose this works with -ftime-report right? (I'm thinking
> > about the new code that emits phase-level timers).
>
> Yes, I've been using -ftime-report a lot for PR54146, and the
> output format was itching...

You can check the error statically.  Something like

% cat limitstring.c
#define LIMIT 32

struct def {
  int x;
  char name[LIMIT+1];
};

struct def var[] = {
  { 3, "hello" },
  { 4, "name is much too too long for a reasonable name" },
};

% gcc -c limitstring.c -Werror
cc1: warnings being treated as errors
limitstring.c:10: error: initializer-string for array of chars is too long
limitstring.c:10: error: (near initialization for 'timevars[1].name')

But of course the variable definition would look more like

#define DEFTIMEVAR(identifier__, name__) \
    { ...., name__, ... },
struct def var[] = {
#include "timevar.def"
};

-- 
Lawrence Crowl


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]