On 8/14/12, Steven Bosscher <stevenb....@gmail.com> wrote:
> On Aug 14, 2012 Diego Novillo <dnovi...@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
