From c5d8961b0b0724f561aadee441390627d93d3079 Mon Sep 17 00:00:00 2001 From: Bas Nieuwenhuizen Date: Sun, 5 Jul 2020 19:20:50 +0200 Subject: [PATCH] Revert "radv: add support for MRTs compaction to avoid holes" This reverts commit 7a5e6fd25f2e132ef4cacc3a5b714c4e153227b0. Since we have two different users bisecting issues to this commit, let's revert. Reviewed-by: Samuel Pitoiset 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: --- .../compiler/aco_instruction_selection.cpp | 18 +++---- src/amd/vulkan/radv_nir_to_llvm.c | 3 +- src/amd/vulkan/radv_pipeline.c | 51 +++++++++---------- src/amd/vulkan/radv_shader_info.c | 12 +++++ 4 files changed, 43 insertions(+), 41 deletions(-) diff --git a/src/amd/compiler/aco_instruction_selection.cpp b/src/amd/compiler/aco_instruction_selection.cpp index cd2263963fc..de435ff7834 100644 --- a/src/amd/compiler/aco_instruction_selection.cpp +++ b/src/amd/compiler/aco_instruction_selection.cpp @@ -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); diff --git a/src/amd/vulkan/radv_nir_to_llvm.c b/src/amd/vulkan/radv_nir_to_llvm.c index 0c81138f63d..987d1054356 100644 --- a/src/amd/vulkan/radv_nir_to_llvm.c +++ b/src/amd/vulkan/radv_nir_to_llvm.c @@ -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++; diff --git a/src/amd/vulkan/radv_pipeline.c b/src/amd/vulkan/radv_pipeline.c index 0e6be653033..098d26bb3b8 100644 --- a/src/amd/vulkan/radv_pipeline.c +++ b/src/amd/vulkan/radv_pipeline.c @@ -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; } /* diff --git a/src/amd/vulkan/radv_shader_info.c b/src/amd/vulkan/radv_shader_info.c index 62a5f170f7c..eca46c81157 100644 --- a/src/amd/vulkan/radv_shader_info.c +++ b/src/amd/vulkan/radv_shader_info.c @@ -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; } -- 2.30.2