radeonsi: add workaround for issue 2647

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 <marek.olsak@amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4700>

diff --git a/src/amd/llvm/ac_nir_to_llvm.c b/src/amd/llvm/ac_nir_to_llvm.c
index 2a495eb76e4e21a8d95ebcc676812bdc95961493..42cd952b69f31e1acf922efd1d821b55656c69f4 100644 (file)
--- 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 18f85a7911cd216a1ec44115c13473f63ddcdf14..ea3717413d2d49415dd9327534aee339417c6358 100644 (file)
--- 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 83c7425e09466e86f2fd50f0bba8618bd3d1fa75..bf4329ad803540fcd9dccab57bfb7a7bd0f2e3a7 100644 (file)
--- 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 865114283766fc139a145685f3ec73c9b635531b..f3457924e77557bcb00fd6b80a48f673ab8f294f 100644 (file)
--- 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 e4104cf8d781a71721e8c2e3653f7913635a8be1..6b934c1dff47d0aa22d871d2b58095755c34b97e 100644 (file)
--- 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 b510b6377880a0167a5b08eeb931ff313ff34ead..d26b80423fd46805a2e1d9bbdd9d833313c76144 100644 (file)
--- 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 d2a82b2a19a8592e4b4c6ce25a19015abaeb8915..acda91f6c78cf98a3d6f8271bf61faad07188d7f 100644 (file)
--- a/src/util/00-mesa-defaults.conf
+++ b/src/util/00-mesa-defaults.conf
@@ -629,6 +629,10 @@ TODO: document the other workarounds.
         <application name="Peace, Death!" executable="runner" sha1="5b909f3d21799773370adf084f649848f098234e">
             <option name="radeonsi_sync_compile" value="true" />
         </application>
+        <!-- https://gitlab.freedesktop.org/mesa/mesa/issues/2647 -->
+        <application name="Kerbal Space Program" executable="KSP.x86_64">
+            <option name="radeonsi_no_infinite_interp" value="true" />
+        </application>
     </device>
     <device driver="virtio_gpu">
         <!-- Some Valve games do a final blit to a BRGA_sRGB surface. On a GLES