From 64662dd5baeec19a618156b52df7a7e7adba94cf Mon Sep 17 00:00:00 2001 From: Pierre-Eric Pelloux-Prayer Date: Fri, 24 Apr 2020 11:58:12 +0200 Subject: [PATCH] radeonsi: add workaround for issue 2647 MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit For unknown reasons pixel shaders in KSP game get executed with infinite interpolation coefficients and this causes an infinite loop in the shader. This commit adds a hacky workaround that kills pixel shaders if invalid interp coeffs are detected and enables it for KSP. Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/2174 Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/2647 Reviewed-by: Marek Olšák Part-of: --- src/amd/llvm/ac_nir_to_llvm.c | 26 ++++++++++++++++--- src/amd/llvm/ac_shader_abi.h | 3 +++ .../drivers/radeonsi/si_debug_options.h | 1 + src/gallium/drivers/radeonsi/si_pipe.c | 7 ++++- src/gallium/drivers/radeonsi/si_pipe.h | 1 + src/gallium/drivers/radeonsi/si_shader_llvm.c | 7 +++++ src/util/00-mesa-defaults.conf | 4 +++ 7 files changed, 45 insertions(+), 4 deletions(-) diff --git a/src/amd/llvm/ac_nir_to_llvm.c b/src/amd/llvm/ac_nir_to_llvm.c index 2a495eb76e4..42cd952b69f 100644 --- a/src/amd/llvm/ac_nir_to_llvm.c +++ b/src/amd/llvm/ac_nir_to_llvm.c @@ -51,6 +51,7 @@ struct ac_nir_context { struct hash_table *defs; struct hash_table *phis; struct hash_table *vars; + struct hash_table *verified_interp; LLVMValueRef main_function; LLVMBasicBlockRef continue_block; @@ -3511,13 +3512,26 @@ static LLVMValueRef load_interpolated_input(struct ac_nir_context *ctx, unsigned bitsize) { LLVMValueRef attr_number = LLVMConstInt(ctx->ac.i32, index, false); + LLVMValueRef interp_param_f; - interp_param = LLVMBuildBitCast(ctx->ac.builder, + interp_param_f = LLVMBuildBitCast(ctx->ac.builder, interp_param, ctx->ac.v2f32, ""); LLVMValueRef i = LLVMBuildExtractElement( - ctx->ac.builder, interp_param, ctx->ac.i32_0, ""); + ctx->ac.builder, interp_param_f, ctx->ac.i32_0, ""); LLVMValueRef j = LLVMBuildExtractElement( - ctx->ac.builder, interp_param, ctx->ac.i32_1, ""); + ctx->ac.builder, interp_param_f, ctx->ac.i32_1, ""); + + /* Workaround for issue 2647: kill threads with infinite interpolation coeffs */ + if (ctx->verified_interp && + !_mesa_hash_table_search(ctx->verified_interp, interp_param)) { + LLVMValueRef args[2]; + args[0] = i; + args[1] = LLVMConstInt(ctx->ac.i32, S_NAN | Q_NAN | N_INFINITY | P_INFINITY, false); + LLVMValueRef cond = ac_build_intrinsic(&ctx->ac, "llvm.amdgcn.class.f32", ctx->ac.i1, + args, 2, AC_FUNC_ATTR_READNONE); + ac_build_kill_if_false(&ctx->ac, LLVMBuildNot(ctx->ac.builder, cond, "")); + _mesa_hash_table_insert(ctx->verified_interp, interp_param, interp_param); + } LLVMValueRef values[4]; assert(bitsize == 16 || bitsize == 32); @@ -5253,6 +5267,10 @@ void ac_nir_translate(struct ac_llvm_context *ac, struct ac_shader_abi *abi, ctx.vars = _mesa_hash_table_create(NULL, _mesa_hash_pointer, _mesa_key_pointer_equal); + if (ctx.abi->kill_ps_if_inf_interp) + ctx.verified_interp = _mesa_hash_table_create(NULL, _mesa_hash_pointer, + _mesa_key_pointer_equal); + func = (struct nir_function *)exec_list_get_head(&nir->functions); nir_index_ssa_defs(func->impl); @@ -5287,6 +5305,8 @@ void ac_nir_translate(struct ac_llvm_context *ac, struct ac_shader_abi *abi, ralloc_free(ctx.defs); ralloc_free(ctx.phis); ralloc_free(ctx.vars); + if (ctx.abi->kill_ps_if_inf_interp) + ralloc_free(ctx.verified_interp); } bool diff --git a/src/amd/llvm/ac_shader_abi.h b/src/amd/llvm/ac_shader_abi.h index 18f85a7911c..ea3717413d2 100644 --- a/src/amd/llvm/ac_shader_abi.h +++ b/src/amd/llvm/ac_shader_abi.h @@ -186,6 +186,9 @@ struct ac_shader_abi { /* Whether bounds checks are required */ bool robust_buffer_access; + + /* Check for Inf interpolation coeff */ + bool kill_ps_if_inf_interp; }; #endif /* AC_SHADER_ABI_H */ diff --git a/src/gallium/drivers/radeonsi/si_debug_options.h b/src/gallium/drivers/radeonsi/si_debug_options.h index 83c7425e094..bf4329ad803 100644 --- a/src/gallium/drivers/radeonsi/si_debug_options.h +++ b/src/gallium/drivers/radeonsi/si_debug_options.h @@ -7,5 +7,6 @@ OPT_BOOL(halt_shaders, false, "Halt shaders at the start (will hang)") OPT_BOOL(vs_fetch_always_opencode, false, "Always open code vertex fetches (less efficient, purely for testing)") OPT_BOOL(prim_restart_tri_strips_only, false, "Only enable primitive restart for triangle strips") +OPT_BOOL(no_infinite_interp, false, "Kill PS with infinite interp coeff") #undef OPT_BOOL diff --git a/src/gallium/drivers/radeonsi/si_pipe.c b/src/gallium/drivers/radeonsi/si_pipe.c index 86511428376..f3457924e77 100644 --- a/src/gallium/drivers/radeonsi/si_pipe.c +++ b/src/gallium/drivers/radeonsi/si_pipe.c @@ -67,6 +67,7 @@ static const struct debug_named_value debug_options[] = { {"w64ge", DBG(W64_GE), "Use Wave64 for vertex, tessellation, and geometry shaders."}, {"w64ps", DBG(W64_PS), "Use Wave64 for pixel shaders."}, {"w64cs", DBG(W64_CS), "Use Wave64 for computes shaders."}, + {"noinfinterp", DBG(KILL_PS_INF_INTERP), "Kill PS with infinite interp coeff"}, /* Shader compiler options (with no effect on the shader cache): */ {"checkir", DBG(CHECK_IR), "Enable additional sanity checks on shader IR"}, @@ -879,7 +880,7 @@ static void si_disk_cache_create(struct si_screen *sscreen) disk_cache_format_hex_id(cache_id, sha1, 20 * 2); /* These flags affect shader compilation. */ -#define ALL_FLAGS (DBG(GISEL)) +#define ALL_FLAGS (DBG(GISEL) | DBG(KILL_PS_INF_INTERP)) uint64_t shader_debug_flags = sscreen->debug_flags & ALL_FLAGS; /* Add the high bits of 32-bit addresses, which affects @@ -996,6 +997,10 @@ static struct pipe_screen *radeonsi_screen_create_impl(struct radeon_winsys *ws, #include "si_debug_options.h" } + if (sscreen->options.no_infinite_interp) { + sscreen->debug_flags |= DBG(KILL_PS_INF_INTERP); + } + si_disk_cache_create(sscreen); /* Determine the number of shader compiler threads. */ diff --git a/src/gallium/drivers/radeonsi/si_pipe.h b/src/gallium/drivers/radeonsi/si_pipe.h index e4104cf8d78..6b934c1dff4 100644 --- a/src/gallium/drivers/radeonsi/si_pipe.h +++ b/src/gallium/drivers/radeonsi/si_pipe.h @@ -160,6 +160,7 @@ enum DBG_W64_GE, DBG_W64_PS, DBG_W64_CS, + DBG_KILL_PS_INF_INTERP, /* Shader compiler options (with no effect on the shader cache): */ DBG_CHECK_IR, diff --git a/src/gallium/drivers/radeonsi/si_shader_llvm.c b/src/gallium/drivers/radeonsi/si_shader_llvm.c index b510b637788..d26b80423fd 100644 --- a/src/gallium/drivers/radeonsi/si_shader_llvm.c +++ b/src/gallium/drivers/radeonsi/si_shader_llvm.c @@ -441,6 +441,13 @@ bool si_nir_build_llvm(struct si_shader_context *ctx, struct nir_shader *nir) ctx->abi.interp_at_sample_force_center = ctx->shader->key.mono.u.ps.interpolate_at_sample_force_center; + + ctx->abi.kill_ps_if_inf_interp = + (ctx->screen->debug_flags & DBG(KILL_PS_INF_INTERP)) && + (ctx->shader->selector->info.uses_persp_center || + ctx->shader->selector->info.uses_persp_centroid || + ctx->shader->selector->info.uses_persp_sample); + } else if (nir->info.stage == MESA_SHADER_COMPUTE) { if (nir->info.cs.user_data_components_amd) { ctx->abi.user_data = ac_get_arg(&ctx->ac, ctx->cs_user_data); diff --git a/src/util/00-mesa-defaults.conf b/src/util/00-mesa-defaults.conf index d2a82b2a19a..acda91f6c78 100644 --- a/src/util/00-mesa-defaults.conf +++ b/src/util/00-mesa-defaults.conf @@ -629,6 +629,10 @@ TODO: document the other workarounds. + + +