[Mesa-dev] [PATCH 2/2] intel/compiler: Add brw_get_compiler_config_value for disk cache

[Jordan Justen]

On 2018-08-02 11:27:57, Dylan Baker wrote:
> Quoting Jordan Justen (2018-07-25 17:12:20)
> > During code review, Jason pointed out that:
> > 
> > 2b3064c0731 "i965, anv: Use INTEL_DEBUG for disk_cache driver flags"
> > 
> > Didn't account for INTEL_SCALER_* environment variables.
> > 
> > To fix this, let the compiler return the disk_cache driver flags.
> > 
> > Another possible fix would be to pull the INTEL_SCALER_* into
> > INTEL_DEBUG bits, but as we are currently using 41 of 64 bits, I
> > didn't think it was a good use of 4 more of these bits. (5 since
> > INTEL_PRECISE_TRIG needs to be accounted for as well.)
> > 
> > Cc: Jason Ekstrand <jason at jlekstrand.net>
> > Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
> > ---
> >  src/intel/compiler/brw_compiler.c          | 26 ++++++++++++++++++++++
> >  src/intel/compiler/brw_compiler.h          | 11 +++++++++
> >  src/intel/vulkan/anv_device.c              |  3 ++-
> >  src/mesa/drivers/dri/i965/brw_disk_cache.c |  3 ++-
> >  4 files changed, 41 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/intel/compiler/brw_compiler.c b/src/intel/compiler/brw_compiler.c
> > index 6480dbefbf6..ba6b8e0cbbe 100644
> > --- a/src/intel/compiler/brw_compiler.c
> > +++ b/src/intel/compiler/brw_compiler.c
> > @@ -180,6 +180,32 @@ brw_compiler_create(void *mem_ctx, const struct gen_device_info *devinfo)
> >  
> >     return compiler;
> >  }
> > +static void
> > +insert_u64_bit(uint64_t *val, bool add)
> > +{
> > +   *val = (*val << 1) | !!add;
> > +}
> > +
> > +const uint64_t
> > +brw_get_compiler_config_value(const struct brw_compiler *compiler)
> > +{
> > +   uint64_t config = 0;
> > +   insert_u64_bit(&config, compiler->precise_trig);
> > +   if (compiler->devinfo->gen >= 8 && compiler->devinfo->gen < 10) {
> > +      insert_u64_bit(&config, compiler->scalar_stage[MESA_SHADER_VERTEX]);
> > +      insert_u64_bit(&config, compiler->scalar_stage[MESA_SHADER_TESS_CTRL]);
> > +      insert_u64_bit(&config, compiler->scalar_stage[MESA_SHADER_TESS_EVAL]);
> > +      insert_u64_bit(&config, compiler->scalar_stage[MESA_SHADER_GEOMETRY]);
> > +   }
> > +   uint64_t debug_bits = INTEL_DEBUG;
> > +   uint64_t mask = DEBUG_DISK_CACHE_MASK;
> > +   while (mask != 0) {
> > +      const uint64_t bit = 1ULL << (ffsll(mask) - 1);
> > +      insert_u64_bit(&config, (debug_bits & bit) != 0);
> > +      mask &= ~bit;
> > +   }
> > +   return config;
> > +}
> >  
> >  unsigned
> >  brw_prog_data_size(gl_shader_stage stage)
> > diff --git a/src/intel/compiler/brw_compiler.h b/src/intel/compiler/brw_compiler.h
> > index 9dfcfcc0115..efefbbca64b 100644
> > --- a/src/intel/compiler/brw_compiler.h
> > +++ b/src/intel/compiler/brw_compiler.h
> > @@ -1212,6 +1212,17 @@ DEFINE_PROG_DATA_DOWNCAST(sf)
> >  
> >  struct brw_compiler *
> >  brw_compiler_create(void *mem_ctx, const struct gen_device_info *devinfo);
> > +/**
> > + * Returns a compiler configuration for use with shader_config
> > + *
> > + * This value only needs to change for settings that can cause different
> > + * program generation between two runs on the same hardware.
> > + *
> > + * For example, it doesn't need to be different for gen 8 and gen 9 hardware,
> > + * but it does need to be different if INTEL_DEBUG=nocompact is or isn't used.
> > + */
> > +const uint64_t
> > +brw_get_compiler_config_value(const struct brw_compiler *compiler);
> >  
> >  unsigned
> >  brw_prog_data_size(gl_shader_stage stage);
> > diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
> > index 6b72a79a914..c40b94d69f3 100644
> > --- a/src/intel/vulkan/anv_device.c
> > +++ b/src/intel/vulkan/anv_device.c
> > @@ -286,7 +286,8 @@ anv_physical_device_init_disk_cache(struct anv_physical_device *device)
> >     char timestamp[41];
> >     _mesa_sha1_format(timestamp, device->driver_build_sha1);
> >  
> > -   const uint64_t driver_flags = INTEL_DEBUG & DEBUG_DISK_CACHE_MASK;
> > +   const uint64_t driver_flags =
> > +      brw_get_compiler_config_value(device->compiler);
> >     device->disk_cache = disk_cache_create(renderer, timestamp, driver_flags);
> >  #else
> >     device->disk_cache = NULL;
> > diff --git a/src/mesa/drivers/dri/i965/brw_disk_cache.c b/src/mesa/drivers/dri/i965/brw_disk_cache.c
> > index 0797e6eac44..9a6f2ff570c 100644
> > --- a/src/mesa/drivers/dri/i965/brw_disk_cache.c
> > +++ b/src/mesa/drivers/dri/i965/brw_disk_cache.c
> > @@ -396,7 +396,8 @@ brw_disk_cache_init(struct intel_screen *screen)
> >     char timestamp[41];
> >     _mesa_sha1_format(timestamp, id_sha1);
> >  
> > -   const uint64_t driver_flags = INTEL_DEBUG & DEBUG_DISK_CACHE_MASK;
> > +   const uint64_t driver_flags =
> > +      brw_get_compiler_config_value(screen->compiler);
> >     screen->disk_cache = disk_cache_create(renderer, timestamp, driver_flags);
> >  #endif
> >  }
> > -- 
> > 2.18.0
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> Hi Jordan,
> 
> I've pulled this (and another patch it relies on) into the 18.1 branch. There
> were some conflicts when pulling them over, can you take a look at the
> staging/18.1 branch and make sure that the two commits (currently tip) look
> okay?

Yeah, it looks good.

If you decide to cherry-pick 8fcdb71d8c9 as well, and just ignore the
change in Anvil. (Anvil didn't have the transparent pipeline cache
enabled in 18.1.)

-Jordan