Revert "radv: add support for MRTs compaction to avoid holes"
authorBas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
Sun, 5 Jul 2020 17:20:50 +0000 (19:20 +0200)
committerMarge Bot <eric+marge@anholt.net>
Mon, 6 Jul 2020 14:06:37 +0000 (14:06 +0000)
This reverts commit 7a5e6fd25f2e132ef4cacc3a5b714c4e153227b0.

Since we have two different users bisecting issues to this commit, let's
revert.

Reviewed-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Fixes: 7a5e6fd25f2 "radv: add support for MRTs compaction to avoid holes"
Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/3202
Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/3228
(Other report in https://gitlab.freedesktop.org/mesa/mesa/-/issues/3151#note_558589)

Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/5758>

src/amd/compiler/aco_instruction_selection.cpp
src/amd/vulkan/radv_nir_to_llvm.c
src/amd/vulkan/radv_pipeline.c
src/amd/vulkan/radv_shader_info.c

index cd2263963fc445150f22405505214c4e69afa9b5..de435ff78343b60e772c2ca13b2256c7250d8f33 100644 (file)
@@ -9979,15 +9979,15 @@ static bool export_fs_mrt_z(isel_context *ctx)
    return true;
 }
 
-static bool export_fs_mrt_color(isel_context *ctx, int slot,
-                                unsigned write_mask, Temp *outputs)
+static bool export_fs_mrt_color(isel_context *ctx, int slot)
 {
    Builder bld(ctx->program, ctx->block);
+   unsigned write_mask = ctx->outputs.mask[slot];
    Operand values[4];
 
    for (unsigned i = 0; i < 4; ++i) {
       if (write_mask & (1 << i)) {
-         values[i] = Operand(outputs[i]);
+         values[i] = Operand(ctx->outputs.temps[slot * 4u + i]);
       } else {
          values[i] = Operand(v1);
       }
@@ -9997,6 +9997,7 @@ static bool export_fs_mrt_color(isel_context *ctx, int slot,
    unsigned enabled_channels = 0;
    aco_opcode compr_op = (aco_opcode)0;
 
+   slot -= FRAG_RESULT_DATA0;
    target = V_008DFC_SQ_EXP_MRT + slot;
    col_format = (ctx->options->key.fs.col_format >> (4 * slot)) & 0xf;
 
@@ -10181,7 +10182,6 @@ static bool export_fs_mrt_color(isel_context *ctx, int slot,
 
 static void create_fs_exports(isel_context *ctx)
 {
-   unsigned compacted_mrt_index = 0;
    bool exported = false;
 
    /* Export depth, stencil and sample mask. */
@@ -10191,15 +10191,9 @@ static void create_fs_exports(isel_context *ctx)
       exported |= export_fs_mrt_z(ctx);
 
    /* Export all color render targets. */
-   for (unsigned i = FRAG_RESULT_DATA0; i < FRAG_RESULT_DATA7 + 1; ++i) {
+   for (unsigned i = FRAG_RESULT_DATA0; i < FRAG_RESULT_DATA7 + 1; ++i)
       if (ctx->outputs.mask[i])
-         if (export_fs_mrt_color(ctx, compacted_mrt_index,
-                                 ctx->outputs.mask[i],
-                                 &ctx->outputs.temps[i * 4u])) {
-            compacted_mrt_index++;
-            exported = true;
-         }
-   }
+         exported |= export_fs_mrt_color(ctx, i);
 
    if (!exported)
       create_null_export(ctx);
index 0c81138f63d677007f61f33265329b3d26ce50ee..987d10543567639240410de262ec900cda373621 100644 (file)
@@ -3586,7 +3586,8 @@ handle_fs_outputs_post(struct radv_shader_context *ctx)
                        values[j] = ac_to_float(&ctx->ac,
                                                radv_load_output(ctx, i, j));
 
-               bool ret = si_export_mrt_color(ctx, values, index,
+               bool ret = si_export_mrt_color(ctx, values,
+                                              i - FRAG_RESULT_DATA0,
                                               &color_args[index]);
                if (ret)
                        index++;
index 0e6be65303380d728a137550b9e6d8572e7f391d..098d26bb3b8974c9c5865979b7178baf6d0b9e08 100644 (file)
@@ -511,10 +511,8 @@ radv_pipeline_compute_spi_color_formats(struct radv_pipeline *pipeline,
 {
        RADV_FROM_HANDLE(radv_render_pass, pass, pCreateInfo->renderPass);
        struct radv_subpass *subpass = pass->subpasses + pCreateInfo->subpass;
-       unsigned exp_fmt[MAX_RTS] = {0};
-       unsigned is_int8[MAX_RTS] = {0}, is_int10[MAX_RTS] = {0};
-       unsigned col_format = 0;
-       unsigned col_format_is_int8 = 0, col_format_is_int10 = 0;
+       unsigned col_format = 0, is_int8 = 0, is_int10 = 0;
+       unsigned num_targets;
 
        for (unsigned i = 0; i < (blend->single_cb_enable ? 1 : subpass->color_count); ++i) {
                unsigned cf;
@@ -531,45 +529,42 @@ radv_pipeline_compute_spi_color_formats(struct radv_pipeline *pipeline,
                                                          blend_enable,
                                                          blend->need_src_alpha & (1 << i));
 
-                       is_int8[i] = format_is_int8(attachment->format);
-                       is_int10[i] = format_is_int10(attachment->format);
+                       if (format_is_int8(attachment->format))
+                               is_int8 |= 1 << i;
+                       if (format_is_int10(attachment->format))
+                               is_int10 |= 1 << i;
                }
 
-               exp_fmt[i] = cf;
+               col_format |= cf << (4 * i);
        }
 
-       if (!exp_fmt[0] && blend->need_src_alpha & (1 << 0)) {
+       if (!(col_format & 0xf) && blend->need_src_alpha & (1 << 0)) {
                /* When a subpass doesn't have any color attachments, write the
                 * alpha channel of MRT0 when alpha coverage is enabled because
                 * the depth attachment needs it.
                 */
-               exp_fmt[0] = V_028714_SPI_SHADER_32_AR;
+               col_format |= V_028714_SPI_SHADER_32_AR;
        }
 
-       /* The output for dual source blending should have the same format as
-        * the first output.
+       /* If the i-th target format is set, all previous target formats must
+        * be non-zero to avoid hangs.
         */
-       if (blend->mrt0_is_dual_src) {
-               col_format |= (exp_fmt[0] << 4) | exp_fmt[0];
-               col_format_is_int8 |= (is_int8[0] << 1) | is_int8[0];
-               col_format_is_int10 |= (is_int10[0] << 1) | is_int10[0];
-       } else {
-               /* Remove holes in SPI_SHADER_COL_FORMAT. */
-               unsigned num_color_targets = 0;
-               for (unsigned i = 0; i < MAX_RTS; i++) {
-                       if (!exp_fmt[i])
-                               continue;
-
-                       col_format |= exp_fmt[i] << (4 * num_color_targets);
-                       col_format_is_int8 |= is_int8[i] << num_color_targets;
-                       col_format_is_int10 |= is_int10[i] << num_color_targets;
-                       num_color_targets++;
+       num_targets = (util_last_bit(col_format) + 3) / 4;
+       for (unsigned i = 0; i < num_targets; i++) {
+               if (!(col_format & (0xf << (i * 4)))) {
+                       col_format |= V_028714_SPI_SHADER_32_R << (i * 4);
                }
        }
 
+       /* The output for dual source blending should have the same format as
+        * the first output.
+        */
+       if (blend->mrt0_is_dual_src)
+               col_format |= (col_format & 0xf) << 4;
+
        blend->spi_shader_col_format = col_format;
-       blend->col_format_is_int8 = col_format_is_int8;
-       blend->col_format_is_int10 = col_format_is_int10;
+       blend->col_format_is_int8 = is_int8;
+       blend->col_format_is_int10 = is_int10;
 }
 
 /*
index 62a5f170f7cf4e68f660882bb65561576fa5da86..eca46c811578c44fa102fced0c278fd26190832c 100644 (file)
@@ -846,6 +846,18 @@ radv_nir_shader_info_pass(const struct nir_shader *nir,
        info->float_controls_mode = nir->info.float_controls_execution_mode;
 
        if (nir->info.stage == MESA_SHADER_FRAGMENT) {
+               /* If the i-th output is used, all previous outputs must be
+                * non-zero to match the target format.
+                * TODO: compact MRT to avoid holes and to remove this
+                * workaround.
+                */
+               unsigned num_targets = (util_last_bit(info->ps.cb_shader_mask) + 3) / 4;
+               for (unsigned i = 0; i < num_targets; i++) {
+                       if (!(info->ps.cb_shader_mask & (0xf << (i * 4)))) {
+                               info->ps.cb_shader_mask |= 0xf << (i * 4);
+                       }
+               }
+
                if (key->fs.is_dual_src) {
                        info->ps.cb_shader_mask |= (info->ps.cb_shader_mask & 0xf) << 4;
                }