[Mesa-dev] [PATCH 1/8] radeonsi: add si_debug_options for convenient adding/removing of options
Marek Olšák
maraeo at gmail.com
Thu Apr 25 02:45:58 UTC 2019
On Wed, Apr 24, 2019 at 9:14 AM Nicolai Hähnle <nhaehnle at gmail.com> wrote:
> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>
> Move the definition of radeonsi_clear_db_cache_before_clear there,
> as well as radeonsi_enable_nir.
>
> This removes the AMD_DEBUG=nir option.
>
> We currently still have two places for options: the driconf machinery
> and AMD_DEBUG/R600_DEBUG. If we are to have a single place for options,
> then the driconf machinery should be preferred since it's more flexible.
>
> The only downside of the driconf machinery was that adding new options
> was quite inconvenient. With this change, a simple boolean option can
> be added with a single line of code, same as for AMD_DEBUG.
>
> One technical limitation of this particular implementation is that while
> almost all driconf features are available, the translation machinery
> doesn't
> pick up the description strings for options added in si_debvug_options. In
> practice, translations haven't been provided anyway, and this is intended
> for developer options, so I'm not too worried. It could always be added
> later if anybody really cares.
> ---
> .../drivers/radeonsi/driinfo_radeonsi.h | 12 +++-
> src/gallium/drivers/radeonsi/si_clear.c | 2 +-
> .../drivers/radeonsi/si_debug_options.inc | 4 ++
> src/gallium/drivers/radeonsi/si_get.c | 6 +-
> src/gallium/drivers/radeonsi/si_pipe.c | 22 ++++---
> src/gallium/drivers/radeonsi/si_pipe.h | 7 ++-
> src/util/merge_driinfo.py | 58 +++++++++++++++++--
> src/util/xmlpool/t_options.h | 9 ---
> 8 files changed, 89 insertions(+), 31 deletions(-)
>
> diff --git a/src/gallium/drivers/radeonsi/driinfo_radeonsi.h
> b/src/gallium/drivers/radeonsi/driinfo_radeonsi.h
> index edf8edba035..d92883b9c38 100644
> --- a/src/gallium/drivers/radeonsi/driinfo_radeonsi.h
> +++ b/src/gallium/drivers/radeonsi/driinfo_radeonsi.h
> @@ -4,13 +4,21 @@ DRI_CONF_SECTION_QUALITY
> DRI_CONF_SECTION_END
>
> DRI_CONF_SECTION_PERFORMANCE
> DRI_CONF_RADEONSI_ENABLE_SISCHED("false")
> DRI_CONF_RADEONSI_ASSUME_NO_Z_FIGHTS("false")
> DRI_CONF_RADEONSI_COMMUTATIVE_BLEND_ADD("false")
> DRI_CONF_RADEONSI_ZERO_ALL_VRAM_ALLOCS("false")
> DRI_CONF_SECTION_END
>
> DRI_CONF_SECTION_DEBUG
> - DRI_CONF_RADEONSI_CLEAR_DB_CACHE_BEFORE_CLEAR("false")
> - DRI_CONF_RADEONSI_ENABLE_NIR("false")
> +
> +//= BEGIN VERBATIM
> +#define OPT_BOOL(name, dflt, description) \
> + DRI_CONF_OPT_BEGIN_B(radeonsi_##name, #dflt) \
> + DRI_CONF_DESC(en, description) \
> + DRI_CONF_OPT_END
> +
> +#include "radeonsi/si_debug_options.inc"
> +//= END VERBATIM
> +
> DRI_CONF_SECTION_END
> diff --git a/src/gallium/drivers/radeonsi/si_clear.c
> b/src/gallium/drivers/radeonsi/si_clear.c
> index e1805f2a1c9..a4ebd5cf2a5 100644
> --- a/src/gallium/drivers/radeonsi/si_clear.c
> +++ b/src/gallium/drivers/radeonsi/si_clear.c
> @@ -631,21 +631,21 @@ static void si_clear(struct pipe_context *ctx,
> unsigned buffers,
> * a coincidence and the root cause is elsewhere.
> *
> * The corruption can be fixed by putting the DB flush
> before
> * or after the depth clear. (surprisingly)
> *
> * https://bugs.freedesktop.org/show_bug.cgi?id=102955
> (apitrace)
> *
> * This hack decreases back-to-back ClearDepth performance.
> */
> if ((sctx->db_depth_clear || sctx->db_stencil_clear) &&
> - sctx->screen->clear_db_cache_before_clear)
> + sctx->screen->options.clear_db_cache_before_clear)
> sctx->flags |= SI_CONTEXT_FLUSH_AND_INV_DB;
> }
>
> si_blitter_begin(sctx, SI_CLEAR);
> util_blitter_clear(sctx->blitter, fb->width, fb->height,
> util_framebuffer_get_num_layers(fb),
> buffers, color, depth, stencil);
> si_blitter_end(sctx);
>
> if (sctx->db_depth_clear) {
> diff --git a/src/gallium/drivers/radeonsi/si_debug_options.inc
> b/src/gallium/drivers/radeonsi/si_debug_options.inc
> new file mode 100644
> index 00000000000..165dba8baf5
> --- /dev/null
> +++ b/src/gallium/drivers/radeonsi/si_debug_options.inc
> @@ -0,0 +1,4 @@
> +OPT_BOOL(clear_db_cache_before_clear, false, "Clear DB cache before fast
> depth clear")
> +OPT_BOOL(enable_nir, false, "Enable NIR")
> +
> +#undef OPT_BOOL
> diff --git a/src/gallium/drivers/radeonsi/si_get.c
> b/src/gallium/drivers/radeonsi/si_get.c
> index 67fbc50998b..fda71da16e6 100644
> --- a/src/gallium/drivers/radeonsi/si_get.c
> +++ b/src/gallium/drivers/radeonsi/si_get.c
> @@ -203,21 +203,21 @@ static int si_get_param(struct pipe_screen *pscreen,
> enum pipe_cap param)
> case PIPE_CAP_VERTEX_BUFFER_OFFSET_4BYTE_ALIGNED_ONLY:
> case PIPE_CAP_VERTEX_BUFFER_STRIDE_4BYTE_ALIGNED_ONLY:
> case PIPE_CAP_VERTEX_ELEMENT_SRC_OFFSET_4BYTE_ALIGNED_ONLY:
> return !sscreen->info.has_unaligned_shader_loads;
>
> case PIPE_CAP_SPARSE_BUFFER_PAGE_SIZE:
> return sscreen->info.has_sparse_vm_mappings ?
> RADEON_SPARSE_PAGE_SIZE : 0;
>
> case PIPE_CAP_PACKED_UNIFORMS:
> - if (sscreen->debug_flags & DBG(NIR))
> + if (sscreen->options.enable_nir)
> return 1;
> return 0;
>
> /* Unsupported features. */
> case PIPE_CAP_BUFFER_SAMPLER_VIEW_RGBA_ONLY:
> case PIPE_CAP_TGSI_FS_COORD_ORIGIN_LOWER_LEFT:
> case PIPE_CAP_TGSI_CAN_COMPACT_CONSTANTS:
> case PIPE_CAP_USER_VERTEX_BUFFERS:
> case PIPE_CAP_FAKE_SW_MSAA:
> case PIPE_CAP_TEXTURE_GATHER_OFFSETS:
> @@ -418,25 +418,25 @@ static int si_get_shader_param(struct pipe_screen*
> pscreen,
> case PIPE_SHADER_CAP_MAX_CONST_BUFFERS:
> return SI_NUM_CONST_BUFFERS;
> case PIPE_SHADER_CAP_MAX_TEXTURE_SAMPLERS:
> case PIPE_SHADER_CAP_MAX_SAMPLER_VIEWS:
> return SI_NUM_SAMPLERS;
> case PIPE_SHADER_CAP_MAX_SHADER_BUFFERS:
> return SI_NUM_SHADER_BUFFERS;
> case PIPE_SHADER_CAP_MAX_SHADER_IMAGES:
> return SI_NUM_IMAGES;
> case PIPE_SHADER_CAP_MAX_UNROLL_ITERATIONS_HINT:
> - if (sscreen->debug_flags & DBG(NIR))
> + if (sscreen->options.enable_nir)
> return 0;
> return 32;
> case PIPE_SHADER_CAP_PREFERRED_IR:
> - if (sscreen->debug_flags & DBG(NIR))
> + if (sscreen->options.enable_nir)
> return PIPE_SHADER_IR_NIR;
> return PIPE_SHADER_IR_TGSI;
> case PIPE_SHADER_CAP_LOWER_IF_THRESHOLD:
> return 4;
>
> /* Supported boolean features. */
> case PIPE_SHADER_CAP_TGSI_CONT_SUPPORTED:
> case PIPE_SHADER_CAP_TGSI_SQRT_SUPPORTED:
> case PIPE_SHADER_CAP_INDIRECT_TEMP_ADDR:
> case PIPE_SHADER_CAP_INDIRECT_CONST_ADDR:
> diff --git a/src/gallium/drivers/radeonsi/si_pipe.c
> b/src/gallium/drivers/radeonsi/si_pipe.c
> index 5caeb575623..9f8bd2039ee 100644
> --- a/src/gallium/drivers/radeonsi/si_pipe.c
> +++ b/src/gallium/drivers/radeonsi/si_pipe.c
> @@ -55,21 +55,20 @@ static const struct debug_named_value debug_options[]
> = {
> { "noasm", DBG(NO_ASM), "Don't print disassembled shaders"},
> { "preoptir", DBG(PREOPT_IR), "Print the LLVM IR before initial
> optimizations" },
>
> /* Shader compiler options the shader cache should be aware of: */
> { "unsafemath", DBG(UNSAFE_MATH), "Enable unsafe math shader
> optimizations" },
> { "sisched", DBG(SI_SCHED), "Enable LLVM SI Machine Instruction
> Scheduler." },
> { "gisel", DBG(GISEL), "Enable LLVM global instruction selector."
> },
>
> /* Shader compiler options (with no effect on the shader cache): */
> { "checkir", DBG(CHECK_IR), "Enable additional sanity checks on
> shader IR" },
> - { "nir", DBG(NIR), "Enable experimental NIR shaders" },
> { "mono", DBG(MONOLITHIC_SHADERS), "Use old-style monolithic
> shaders compiled on demand" },
> { "nooptvariant", DBG(NO_OPT_VARIANT), "Disable compiling
> optimized shader variants." },
>
> /* Information logging options: */
> { "info", DBG(INFO), "Print driver information" },
> { "tex", DBG(TEX), "Print texture info" },
> { "compute", DBG(COMPUTE), "Print compute info" },
> { "vm", DBG(VM), "Print virtual addresses when creating resources"
> },
>
> /* Driver options: */
> @@ -825,30 +824,33 @@ static void si_disk_cache_create(struct si_screen
> *sscreen)
> &ctx))
> return;
>
> _mesa_sha1_final(&ctx, sha1);
> disk_cache_format_hex_id(cache_id, sha1, 20 * 2);
>
> /* These flags affect shader compilation. */
> #define ALL_FLAGS (DBG(FS_CORRECT_DERIVS_AFTER_KILL) | \
> DBG(SI_SCHED) | \
> DBG(GISEL) | \
> - DBG(UNSAFE_MATH) | \
> - DBG(NIR))
> + DBG(UNSAFE_MATH))
> uint64_t shader_debug_flags = sscreen->debug_flags &
> ALL_FLAGS;
>
> /* Add the high bits of 32-bit addresses, which affects
> * how 32-bit addresses are expanded to 64 bits.
> */
> STATIC_ASSERT(ALL_FLAGS <= UINT_MAX);
> - shader_debug_flags |= (uint64_t)sscreen->info.address32_hi << 32;
> + assert((int16_t)sscreen->info.address32_hi ==
> (int32_t)sscreen->info.address32_hi);
> + shader_debug_flags |= (uint64_t)(sscreen->info.address32_hi &
> 0xffff) << 32;
> +
> + if (sscreen->options.enable_nir)
> + shader_debug_flags |= 1ull << 48;
>
> sscreen->disk_shader_cache =
> disk_cache_create(sscreen->info.name,
> cache_id,
> shader_debug_flags);
> }
>
> static void si_set_max_shader_compiler_threads(struct pipe_screen *screen,
> unsigned max_threads)
> {
> @@ -918,22 +920,20 @@ struct pipe_screen *radeonsi_screen_create(struct
> radeon_winsys *ws,
> si_init_screen_query_functions(sscreen);
>
> /* Set these flags in debug_flags early, so that the shader cache
> takes
> * them into account.
> */
> if (driQueryOptionb(config->options,
> "glsl_correct_derivatives_after_discard"))
> sscreen->debug_flags |= DBG(FS_CORRECT_DERIVS_AFTER_KILL);
> if (driQueryOptionb(config->options, "radeonsi_enable_sisched"))
> sscreen->debug_flags |= DBG(SI_SCHED);
> - if (driQueryOptionb(config->options, "radeonsi_enable_nir"))
> - sscreen->debug_flags |= DBG(NIR);
>
> if (sscreen->debug_flags & DBG(INFO))
> ac_print_gpu_info(&sscreen->info);
>
> slab_create_parent(&sscreen->pool_transfers,
> sizeof(struct si_transfer), 64);
>
> sscreen->force_aniso = MIN2(16,
> debug_get_num_option("R600_TEX_ANISO", -1));
> if (sscreen->force_aniso >= 0) {
> printf("radeonsi: Forcing anisotropy filter to %ix\n",
> @@ -1067,22 +1067,28 @@ struct pipe_screen *radeonsi_screen_create(struct
> radeon_winsys *ws,
> sscreen->info.pfp_fw_version >= 79 &&
> sscreen->info.me_fw_version >= 142);
>
> sscreen->has_out_of_order_rast = sscreen->info.chip_class >= VI &&
> sscreen->info.max_se >= 2 &&
> !(sscreen->debug_flags &
> DBG(NO_OUT_OF_ORDER));
> sscreen->assume_no_z_fights =
> driQueryOptionb(config->options,
> "radeonsi_assume_no_z_fights");
> sscreen->commutative_blend_add =
> driQueryOptionb(config->options,
> "radeonsi_commutative_blend_add");
> - sscreen->clear_db_cache_before_clear =
> - driQueryOptionb(config->options,
> "radeonsi_clear_db_cache_before_clear");
> +
> + {
> +#define OPT_BOOL(name, dflt, description) \
> + sscreen->options.name = \
> + driQueryOptionb(config->options, "radeonsi_"#name);
> +#include "si_debug_options.inc"
> + }
> +
> sscreen->has_msaa_sample_loc_bug = (sscreen->info.family >=
> CHIP_POLARIS10 &&
> sscreen->info.family <=
> CHIP_POLARIS12) ||
> sscreen->info.family ==
> CHIP_VEGA10 ||
> sscreen->info.family ==
> CHIP_RAVEN;
> sscreen->has_ls_vgpr_init_bug = sscreen->info.family ==
> CHIP_VEGA10 ||
> sscreen->info.family == CHIP_RAVEN;
> sscreen->has_dcc_constant_encode = sscreen->info.family ==
> CHIP_RAVEN2;
>
> /* Only enable primitive binning on APUs by default. */
> sscreen->dpbb_allowed = sscreen->info.family == CHIP_RAVEN ||
> diff --git a/src/gallium/drivers/radeonsi/si_pipe.h
> b/src/gallium/drivers/radeonsi/si_pipe.h
> index 301d38649bf..accc11ca769 100644
> --- a/src/gallium/drivers/radeonsi/si_pipe.h
> +++ b/src/gallium/drivers/radeonsi/si_pipe.h
> @@ -140,21 +140,20 @@ enum {
> DBG_PREOPT_IR,
>
> /* Shader compiler options the shader cache should be aware of: */
> DBG_FS_CORRECT_DERIVS_AFTER_KILL,
> DBG_UNSAFE_MATH,
> DBG_SI_SCHED,
> DBG_GISEL,
>
> /* Shader compiler options (with no effect on the shader cache): */
> DBG_CHECK_IR,
> - DBG_NIR,
> DBG_MONOLITHIC_SHADERS,
> DBG_NO_OPT_VARIANT,
>
> /* Information logging options: */
> DBG_INFO,
> DBG_TEX,
> DBG_COMPUTE,
> DBG_VM,
>
> /* Driver options: */
> @@ -462,28 +461,32 @@ struct si_screen {
> unsigned vgt_hs_offchip_param;
> unsigned eqaa_force_coverage_samples;
> unsigned eqaa_force_z_samples;
> unsigned eqaa_force_color_samples;
> bool has_clear_state;
> bool has_distributed_tess;
> bool has_draw_indirect_multi;
> bool has_out_of_order_rast;
> bool assume_no_z_fights;
> bool commutative_blend_add;
> - bool clear_db_cache_before_clear;
> bool has_msaa_sample_loc_bug;
> bool has_ls_vgpr_init_bug;
> bool has_dcc_constant_encode;
> bool dpbb_allowed;
> bool dfsm_allowed;
> bool llvm_has_working_vgpr_indexing;
>
> + struct {
> +#define OPT_BOOL(name, dflt, description) uint8_t name:1;
>
Why not bool instead of uint8_t?
> +#include "si_debug_options.inc"
>
Why not use the .h file extension?
Other than those, this is:
Reviewed-by: Marek Olšák <marek.olsak at amd.com>
Marek
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190424/a255d1f3/attachment-0001.html>
More information about the mesa-dev
mailing list