On Wed, 2016-02-10 at 15:15 +0100, Michael Matz wrote: > Hi, > > On Wed, 10 Feb 2016, Richard Biener wrote: > > > > The problem is that TV_PHASE_DBGINFO is now nested within > > > TV_PHASE_OPT_GEN, which violates the above mutual exclusivity > > > requirement. Therefore the attached patch simply gets rid of > > > TV_PHASE_DBGINFO (as well as of the sibling > > > TV_PHASE_CHECK_DBGINFO > > > which was already unused). > > > > > > Tested on x86_64-suse-linux, OK for the mainline? > > > > Ok. > > I had this in my tree for a while, asserting that such nesting > doesn't > happen (it asserts that we're always in some phase, and that phases > don't > nest). Might be a good addition for gcc 7.
> Ciao, > Michael. > > Index: timevar.c > =================================================================== > --- timevar.c (revision 232927) > +++ timevar.c (working copy) > @@ -325,6 +325,8 @@ timer::push (timevar_id_t timevar) > push_internal (tv); > } > > +static timevar_id_t global_phase; FWIW I like the idea, but could this be a private field within class timer, rather than a global? I moved the global state relating to timevars into a "class timer" in r223092 (the jit uses this, via a followup: r226530: in theory different threads can have different timer instances [1]). I see that all of the accesses are within timer:: methods. Maybe "m_current_phase" or somesuch? [1] https://gcc.gnu.org/onlinedocs/jit/topics/performance.html#the-timi ng-api and there's a TV_JIT_ACQUIRING_MUTEX which a thread uses for time spent waiting to acquire the big jit mutex (which guards e.g. the g_timer singleton pointer. Within toplev and below, g_timer is the single "live" instance of class timer, but there can be multiple threads each with timer instances waiting to call into toplev via gcc_jit_context_co mpile, if that makes sense). > /* Push TV onto the timing stack, either one of the builtin ones > for a timevar_id_t, or one provided by client code to libgccjit. > */ > > @@ -350,6 +352,8 @@ timer::push_internal (struct timevar_def > if (m_stack) > timevar_accumulate (&m_stack->timevar->elapsed, &m_start_time, > &now); > > + gcc_assert (global_phase >= TV_PHASE_SETUP > + && global_phase <= TV_PHASE_FINALIZE); > /* Reset the start time; from now on, time is attributed to > TIMEVAR. */ > m_start_time = now; > @@ -432,6 +436,9 @@ timer::start (timevar_id_t timevar) > { > struct timevar_def *tv = &m_timevars[timevar]; > > + gcc_assert (global_phase == TV_NONE || global_phase == TV_TOTAL); > + global_phase = timevar; > + > /* Mark this timing variable as used. */ > tv->used = 1; > > @@ -463,6 +470,12 @@ timer::stop (timevar_id_t timevar) > struct timevar_def *tv = &m_timevars[timevar]; > struct timevar_time_def now; > > + gcc_assert (global_phase == timevar); > + if (timevar == TV_TOTAL) > + global_phase = TV_NONE; > + else > + global_phase = TV_TOTAL; > + > /* TIMEVAR must have been started via timevar_start. */ > gcc_assert (tv->standalone); > tv->standalone = 0; /* Enable a restart. */
