From: Rhys Perry Date: Wed, 24 Jul 2019 18:23:21 +0000 (+0100) Subject: nir: merge and extend nir_opt_move_comparisons and nir_opt_move_load_ubo X-Git-Url: https://git.libre-soc.org/?p=mesa.git;a=commitdiff_plain;h=77401498521a35d1dd5711c8a48196a1827450b9 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 --- 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.c b/src/compiler/nir/nir_opt_move.c new file mode 100644 index 00000000000..18c549804d0 --- /dev/null +++ b/src/compiler/nir/nir_opt_move.c @@ -0,0 +1,166 @@ +/* + * 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"), + * 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.c + * + * 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. + * + * 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 + * + * vec1 32 ssa_2 = bcsel ssa_1 c d + * + * the backend would likely do the comparison, producing condition codes, + * then save those to a boolean value. The intervening operations might + * trash the condition codes. Then, in order to do the bcsel, it would + * need to re-populate the condition code register based on the boolean. + * + * By moving the comparison just before the bcsel, the condition codes could + * be used directly. This eliminates the need to reload them from the boolean + * (generally eliminating an instruction). It may also eliminate the need to + * create a boolean value altogether (unless it's used elsewhere), which could + * lower register pressure. + */ + +static bool +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 && nir_can_move_instr(src_instr, options)) { + 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; +} + +struct source_cb_data { + bool *progress; + nir_move_options options; +}; + +static bool +move_source_cb(nir_src *src, void *data_ptr) +{ + struct source_cb_data data = *(struct source_cb_data*)data_ptr; + + nir_instr *instr = src->parent_instr; + if (move_source(src, instr->block, instr, data.options)) + *data.progress = true; + + return true; /* nir_foreach_src should keep going */ +} + +static bool +move(nir_block *block, nir_move_options options) +{ + bool progress = false; + + /* We use a simple approach: walk instructions backwards. + * + * If the instruction's source is a comparison from the same block, + * simply move it here. This may break SSA if it's used earlier in + * the block as well. However, as we walk backwards, we'll find the + * earlier use and move it again, further up. It eventually ends up + * dominating all uses again, restoring SSA form. + * + * Before walking instructions, we consider the if-condition at the + * end of the block, if one exists. It's effectively a use at the + * bottom of the block. + */ + nir_if *iff = nir_block_get_following_if(block); + if (iff) { + progress |= move_source(&iff->condition, block, NULL, options); + } + + nir_foreach_instr_reverse(instr, block) { + /* The sources of phi instructions happen after the predecessor block + * but before this block. (Yes, that's between blocks). This means + * that we don't need to move them in order for them to be correct. + * We could move them to encourage comparisons that are used in a phi to + * the end of the block, doing so correctly would make the pass + * 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 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_source(&alu->src[i].src, block, instr, options); + } + } else { + struct source_cb_data data; + data.progress = &progress; + data.options = options; + nir_foreach_src(instr, move_source_cb, &data); + } + } + + return progress; +} + +bool +nir_opt_move(nir_shader *shader, nir_move_options options) +{ + bool progress = false; + + nir_foreach_function(func, shader) { + if (!func->impl) + continue; + + nir_foreach_block(block, func->impl) { + if (move(block, options)) { + 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/compiler/nir/nir_opt_move_comparisons.c b/src/compiler/nir/nir_opt_move_comparisons.c deleted file mode 100644 index 5da57dc9213..00000000000 --- a/src/compiler/nir/nir_opt_move_comparisons.c +++ /dev/null @@ -1,161 +0,0 @@ -/* - * Copyright © 2016 Intel 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_comparisons.c - * - * This pass moves ALU comparison operations just before their first use. - * - * 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: - * - * vec1 32 ssa_1 = flt a b - * - * vec1 32 ssa_2 = bcsel ssa_1 c d - * - * the backend would likely do the comparison, producing condition codes, - * then save those to a boolean value. The intervening operations might - * trash the condition codes. Then, in order to do the bcsel, it would - * need to re-populate the condition code register based on the boolean. - * - * By moving the comparison just before the bcsel, the condition codes could - * be used directly. This eliminates the need to reload them from the boolean - * (generally eliminating an instruction). It may also eliminate the need to - * create a boolean value altogether (unless it's used elsewhere), which could - * lower register pressure. - */ - -static bool -move_comparison_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_alu && - nir_alu_instr_is_comparison(nir_instr_as_alu(src_instr))) { - - 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_comparison_source_cb(nir_src *src, void *data) -{ - bool *progress = data; - - nir_instr *instr = src->parent_instr; - if (move_comparison_source(src, instr->block, instr)) - *progress = true; - - return true; /* nir_foreach_src should keep going */ -} - -static bool -move_comparisons(nir_block *block) -{ - bool progress = false; - - /* We use a simple approach: walk instructions backwards. - * - * If the instruction's source is a comparison from the same block, - * simply move it here. This may break SSA if it's used earlier in - * the block as well. However, as we walk backwards, we'll find the - * earlier use and move it again, further up. It eventually ends up - * dominating all uses again, restoring SSA form. - * - * Before walking instructions, we consider the if-condition at the - * end of the block, if one exists. It's effectively a use at the - * bottom of the block. - */ - nir_if *iff = nir_block_get_following_if(block); - if (iff) { - progress |= move_comparison_source(&iff->condition, block, NULL); - } - - nir_foreach_instr_reverse(instr, block) { - /* The sources of phi instructions happen after the predecessor block - * but before this block. (Yes, that's between blocks). This means - * that we don't need to move them in order for them to be correct. - * We could move them to encourage comparisons that are used in a phi to - * the end of the block, doing so correctly would make the pass - * 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. - */ - 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); - } - } else { - nir_foreach_src(instr, move_comparison_source_cb, &progress); - } - } - - return progress; -} - -bool -nir_opt_move_comparisons(nir_shader *shader) -{ - bool progress = false; - - nir_foreach_function(func, shader) { - if (!func->impl) - continue; - - nir_foreach_block(block, func->impl) { - if (move_comparisons(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/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);