[Mesa-dev] [PATCH] aubinator: implement a rolling window of programs

[Eric Engestrom]

On Friday, 2017-09-01 15:45:37 +0100, Lionel Landwerlin wrote:
> On 01/09/17 15:37, Eric Engestrom wrote:
> > On Friday, 2017-09-01 10:38:33 +0100, Lionel Landwerlin wrote:
> > > If we have more programs than what we can store,
> > > aubinator_error_decode will assert. Instead let's have a rolling
> > > window of programs.
> > > 
> > > Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> > > ---
> > >   src/intel/tools/aubinator_error_decode.c | 16 ++++++++--------
> > >   1 file changed, 8 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/src/intel/tools/aubinator_error_decode.c b/src/intel/tools/aubinator_error_decode.c
> > > index 636f56a3365..42cc6994353 100644
> > > --- a/src/intel/tools/aubinator_error_decode.c
> > > +++ b/src/intel/tools/aubinator_error_decode.c
> > > @@ -47,6 +47,8 @@
> > >   #define GREEN_HEADER CSI "1;42m"
> > >   #define NORMAL       CSI "0m"
> > > +#define MIN(a, b) ((a) < (b) ? (a) : (b))
> > > +
> > >   /* options */
> > >   static bool option_full_decode = true;
> > > @@ -300,7 +302,7 @@ static void decode(struct gen_spec *spec,
> > >                                  enabled[1] ? "SIMD16 fragment shader" :
> > >                                  enabled[2] ? "SIMD32 fragment shader" : NULL;
> > > -            programs[num_programs++] = (struct program) {
> > > +            programs[num_programs++ % ARRAY_SIZE(programs)] = (struct program) {
> > num_programs++ will eventually overflow, you should wrap it around on
> > your own term. How about:
> > 
> > ----8<----
> > diff --git a/src/intel/tools/aubinator_error_decode.c b/src/intel/tools/aubinator_error_decode.c
> > index 636f56a336..173e0e97fe 100644
> > --- a/src/intel/tools/aubinator_error_decode.c
> > +++ b/src/intel/tools/aubinator_error_decode.c
> > @@ -222,6 +222,11 @@ struct program {
> >   static struct program programs[MAX_NUM_PROGRAMS];
> >   static int num_programs = 0;
> > +static int next_program()
> > +{
> > +   return num_programs = (num_programs + 1) % ARRAY_SIZE(programs);
> > +}
> 
> I guess we need to take some care with the programs[0] then, as this might
> leave it empty/uninitialized.

Yes right, I just wrote this code to explain my idea, I didn't
double-check it.

  static int next_program()
  {
     int ret = num_programs;
     num_programs = (num_programs + 1) % ARRAY_SIZE(programs);
     return ret;
  }

That said, I don't think it makes any difference anywhere that we
effectively start counting at 1, does it?
If that's right, I think the simpler code (ie. my first suggestion)
might be better.