From 77401498521a35d1dd5711c8a48196a1827450b9 Mon Sep 17 00:00:00 2001 From: Rhys Perry Date: Wed, 24 Jul 2019 19:23:21 +0100 Subject: [PATCH] nir: merge and extend nir_opt_move_comparisons and nir_opt_move_load_ubo v2: add to series v3: update Makefile.sources v4: don't remove a comment and break statement v4: use nir_can_move_instr Signed-off-by: Rhys Perry Reviewed-by: Eric Anholt --- src/amd/vulkan/radv_shader.c | 2 +- src/broadcom/compiler/nir_to_vir.c | 2 +- src/compiler/Makefile.sources | 3 +- src/compiler/nir/meson.build | 3 +- src/compiler/nir/nir.h | 4 +- ..._opt_move_comparisons.c => nir_opt_move.c} | 55 ++++---- src/compiler/nir/nir_opt_move_load_ubo.c | 117 ------------------ src/gallium/drivers/freedreno/a2xx/ir2_nir.c | 2 +- src/intel/compiler/brw_nir.c | 2 +- 9 files changed, 37 insertions(+), 153 deletions(-) rename src/compiler/nir/{nir_opt_move_comparisons.c => nir_opt_move.c} (76%) delete mode 100644 src/compiler/nir/nir_opt_move_load_ubo.c diff --git a/src/amd/vulkan/radv_shader.c b/src/amd/vulkan/radv_shader.c index 08d1e204e5f..1e6a9a950d8 100644 --- a/src/amd/vulkan/radv_shader.c +++ b/src/amd/vulkan/radv_shader.c @@ -247,7 +247,7 @@ radv_optimize_nir(struct nir_shader *shader, bool optimize_conservatively, NIR_PASS(progress, shader, nir_opt_conditional_discard); NIR_PASS(progress, shader, nir_opt_shrink_load); - NIR_PASS(progress, shader, nir_opt_move_load_ubo); + NIR_PASS(progress, shader, nir_opt_move, nir_move_load_ubo); } nir_shader * diff --git a/src/broadcom/compiler/nir_to_vir.c b/src/broadcom/compiler/nir_to_vir.c index 56f2f1470d7..03dac60645d 100644 --- a/src/broadcom/compiler/nir_to_vir.c +++ b/src/broadcom/compiler/nir_to_vir.c @@ -1360,7 +1360,7 @@ v3d_optimize_nir(struct nir_shader *s) NIR_PASS(progress, s, nir_opt_undef); } while (progress); - NIR_PASS(progress, s, nir_opt_move_load_ubo); + NIR_PASS(progress, s, nir_opt_move, nir_move_load_ubo); } static int diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.sources index c8dcf5ace10..edf1b936ac1 100644 --- a/src/compiler/Makefile.sources +++ b/src/compiler/Makefile.sources @@ -305,8 +305,7 @@ NIR_FILES = \ nir/nir_opt_intrinsics.c \ nir/nir_opt_loop_unroll.c \ nir/nir_opt_large_constants.c \ - nir/nir_opt_move_comparisons.c \ - nir/nir_opt_move_load_ubo.c \ + nir/nir_opt_move.c \ nir/nir_opt_peephole_select.c \ nir/nir_opt_rematerialize_compares.c \ nir/nir_opt_remove_phis.c \ diff --git a/src/compiler/nir/meson.build b/src/compiler/nir/meson.build index 4e3c714de0c..9b527b22f7c 100644 --- a/src/compiler/nir/meson.build +++ b/src/compiler/nir/meson.build @@ -184,8 +184,7 @@ files_libnir = files( 'nir_opt_intrinsics.c', 'nir_opt_large_constants.c', 'nir_opt_loop_unroll.c', - 'nir_opt_move_comparisons.c', - 'nir_opt_move_load_ubo.c', + 'nir_opt_move.c', 'nir_opt_peephole_select.c', 'nir_opt_rematerialize_compares.c', 'nir_opt_remove_phis.c', diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h index bd6a3617f05..314b696f738 100644 --- a/src/compiler/nir/nir.h +++ b/src/compiler/nir/nir.h @@ -3975,9 +3975,7 @@ bool nir_can_move_instr(nir_instr *instr, nir_move_options options); bool nir_opt_sink(nir_shader *shader, nir_move_options options); -bool nir_opt_move_comparisons(nir_shader *shader); - -bool nir_opt_move_load_ubo(nir_shader *shader); +bool nir_opt_move(nir_shader *shader, nir_move_options options); bool nir_opt_peephole_select(nir_shader *shader, unsigned limit, bool indirect_load_ok, bool expensive_alu_ok); diff --git a/src/compiler/nir/nir_opt_move_comparisons.c b/src/compiler/nir/nir_opt_move.c similarity index 76% rename from src/compiler/nir/nir_opt_move_comparisons.c rename to src/compiler/nir/nir_opt_move.c index 5da57dc9213..18c549804d0 100644 --- a/src/compiler/nir/nir_opt_move_comparisons.c +++ b/src/compiler/nir/nir_opt_move.c @@ -1,5 +1,6 @@ /* * Copyright © 2016 Intel Corporation + * Copyright © 2019 Valve Corporation * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), @@ -24,15 +25,15 @@ #include "nir.h" /** - * \file nir_opt_move_comparisons.c + * \file nir_opt_move.c * - * This pass moves ALU comparison operations just before their first use. + * This pass can move various operations just before their first use inside the + * same basic block. Usually this is to reduce register usage. It's probably + * not a good idea to use this in an optimization loop. * - * It only moves instructions within a single basic block; cross-block - * movement is left to global code motion. - * - * Many GPUs generate condition codes for comparisons, and use predication - * for conditional selects and control flow. In a sequence such as: + * Moving comparisons is useful because many GPUs generate condition codes + * for comparisons, and use predication for conditional selects and control + * flow. In a sequence such as: * * vec1 32 ssa_1 = flt a b * @@ -51,17 +52,14 @@ */ static bool -move_comparison_source(nir_src *src, nir_block *block, nir_instr *before) +move_source(nir_src *src, nir_block *block, nir_instr *before, nir_move_options options) { if (!src->is_ssa) return false; nir_instr *src_instr = src->ssa->parent_instr; - if (src_instr->block == block && - src_instr->type == nir_instr_type_alu && - nir_alu_instr_is_comparison(nir_instr_as_alu(src_instr))) { - + if (src_instr->block == block && nir_can_move_instr(src_instr, options)) { exec_node_remove(&src_instr->node); if (before) @@ -71,24 +69,28 @@ move_comparison_source(nir_src *src, nir_block *block, nir_instr *before) return true; } - return false; } +struct source_cb_data { + bool *progress; + nir_move_options options; +}; + static bool -move_comparison_source_cb(nir_src *src, void *data) +move_source_cb(nir_src *src, void *data_ptr) { - bool *progress = data; + struct source_cb_data data = *(struct source_cb_data*)data_ptr; nir_instr *instr = src->parent_instr; - if (move_comparison_source(src, instr->block, instr)) - *progress = true; + if (move_source(src, instr->block, instr, data.options)) + *data.progress = true; return true; /* nir_foreach_src should keep going */ } static bool -move_comparisons(nir_block *block) +move(nir_block *block, nir_move_options options) { bool progress = false; @@ -106,7 +108,7 @@ move_comparisons(nir_block *block) */ nir_if *iff = nir_block_get_following_if(block); if (iff) { - progress |= move_comparison_source(&iff->condition, block, NULL); + progress |= move_source(&iff->condition, block, NULL, options); } nir_foreach_instr_reverse(instr, block) { @@ -118,20 +120,23 @@ move_comparisons(nir_block *block) * substantially more complicated and wouldn't gain us anything since * the phi can't use a flag value anyway. */ + if (instr->type == nir_instr_type_phi) { /* We're going backwards so everything else is a phi too */ break; } else if (instr->type == nir_instr_type_alu) { /* Walk ALU instruction sources backwards so that bcsel's boolean - * condition is processed last. + * condition is processed last for when comparisons are being moved. */ nir_alu_instr *alu = nir_instr_as_alu(instr); for (int i = nir_op_infos[alu->op].num_inputs - 1; i >= 0; i--) { - progress |= move_comparison_source(&alu->src[i].src, - block, instr); + progress |= move_source(&alu->src[i].src, block, instr, options); } } else { - nir_foreach_src(instr, move_comparison_source_cb, &progress); + struct source_cb_data data; + data.progress = &progress; + data.options = options; + nir_foreach_src(instr, move_source_cb, &data); } } @@ -139,7 +144,7 @@ move_comparisons(nir_block *block) } bool -nir_opt_move_comparisons(nir_shader *shader) +nir_opt_move(nir_shader *shader, nir_move_options options) { bool progress = false; @@ -148,7 +153,7 @@ nir_opt_move_comparisons(nir_shader *shader) continue; nir_foreach_block(block, func->impl) { - if (move_comparisons(block)) { + if (move(block, options)) { nir_metadata_preserve(func->impl, nir_metadata_block_index | nir_metadata_dominance | nir_metadata_live_ssa_defs); diff --git a/src/compiler/nir/nir_opt_move_load_ubo.c b/src/compiler/nir/nir_opt_move_load_ubo.c deleted file mode 100644 index f36a62a5308..00000000000 --- a/src/compiler/nir/nir_opt_move_load_ubo.c +++ /dev/null @@ -1,117 +0,0 @@ -/* - * Copyright © 2016 Intel Corporation - * Copyright © 2018 Valve Corporation - * - * Permission is hereby granted, free of charge, to any person obtaining a - * copy of this software and associated documentation files (the "Software"), - * to deal in the Software without restriction, including without limitation - * the rights to use, copy, modify, merge, publish, distribute, sublicense, - * and/or sell copies of the Software, and to permit persons to whom the - * Software is furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice (including the next - * paragraph) shall be included in all copies or substantial portions of the - * Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS - * IN THE SOFTWARE. - */ - -#include "nir.h" - -/** - * \file nir_opt_move_load_ubo.c - * - * This pass moves load UBO operations just before their first use inside - * the same basic block. - */ -static bool -move_load_ubo_source(nir_src *src, nir_block *block, nir_instr *before) -{ - if (!src->is_ssa) - return false; - - nir_instr *src_instr = src->ssa->parent_instr; - - if (src_instr->block == block && - src_instr->type == nir_instr_type_intrinsic && - nir_instr_as_intrinsic(src_instr)->intrinsic == nir_intrinsic_load_ubo) { - - exec_node_remove(&src_instr->node); - - if (before) - exec_node_insert_node_before(&before->node, &src_instr->node); - else - exec_list_push_tail(&block->instr_list, &src_instr->node); - - return true; - } - return false; -} - -static bool -move_load_ubo_source_cb(nir_src *src, void *data) -{ - bool *progress = data; - - nir_instr *instr = src->parent_instr; - if (move_load_ubo_source(src, instr->block, instr)) - *progress = true; - - return true; /* nir_foreach_src should keep going */ -} - -static bool -move_load_ubo(nir_block *block) -{ - bool progress = false; - - nir_if *iff = nir_block_get_following_if(block); - if (iff) { - progress |= move_load_ubo_source(&iff->condition, block, NULL); - } - - nir_foreach_instr_reverse(instr, block) { - - if (instr->type == nir_instr_type_phi) { - /* We're going backwards so everything else is a phi too */ - } else if (instr->type == nir_instr_type_alu) { - nir_alu_instr *alu = nir_instr_as_alu(instr); - - for (int i = nir_op_infos[alu->op].num_inputs - 1; i >= 0; i--) { - progress |= move_load_ubo_source(&alu->src[i].src, block, instr); - } - } else { - nir_foreach_src(instr, move_load_ubo_source_cb, &progress); - } - } - - return progress; -} - -bool -nir_opt_move_load_ubo(nir_shader *shader) -{ - bool progress = false; - - nir_foreach_function(func, shader) { - if (!func->impl) - continue; - - nir_foreach_block(block, func->impl) { - if (move_load_ubo(block)) { - nir_metadata_preserve(func->impl, nir_metadata_block_index | - nir_metadata_dominance | - nir_metadata_live_ssa_defs); - progress = true; - } - } - } - - return progress; -} diff --git a/src/gallium/drivers/freedreno/a2xx/ir2_nir.c b/src/gallium/drivers/freedreno/a2xx/ir2_nir.c index 402b79691fb..9d36f7092ef 100644 --- a/src/gallium/drivers/freedreno/a2xx/ir2_nir.c +++ b/src/gallium/drivers/freedreno/a2xx/ir2_nir.c @@ -1079,7 +1079,7 @@ ir2_nir_compile(struct ir2_context *ctx, bool binning) OPT_V(ctx->nir, nir_copy_prop); OPT_V(ctx->nir, nir_opt_dce); - OPT_V(ctx->nir, nir_opt_move_comparisons); + OPT_V(ctx->nir, nir_opt_move, nir_move_comparisons); OPT_V(ctx->nir, nir_lower_int_to_float); OPT_V(ctx->nir, nir_lower_bool_to_float); diff --git a/src/intel/compiler/brw_nir.c b/src/intel/compiler/brw_nir.c index b331e62f91a..177e09654e0 100644 --- a/src/intel/compiler/brw_nir.c +++ b/src/intel/compiler/brw_nir.c @@ -875,7 +875,7 @@ brw_postprocess_nir(nir_shader *nir, const struct brw_compiler *compiler, OPT(nir_lower_to_source_mods, nir_lower_all_source_mods); OPT(nir_copy_prop); OPT(nir_opt_dce); - OPT(nir_opt_move_comparisons); + OPT(nir_opt_move, nir_move_comparisons); OPT(nir_lower_bool_to_int32); -- 2.30.2