[Mesa-dev] [PATCH 1/2] nir: add nir_opt_move_load_ubo() optimization pass
Timothy Arceri
tarceri at itsqueeze.com
Fri Mar 16 08:27:58 UTC 2018
On 16/03/18 11:38, Timothy Arceri wrote:
>
>
> On 09/03/18 01:50, Samuel Pitoiset wrote:
>> This pass moves load UBO operations just before their first use,
>> loosely based on nir_opt_move_comparisons.
>>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>> ---
>> src/compiler/Makefile.sources | 1 +
>> src/compiler/nir/meson.build | 1 +
>> src/compiler/nir/nir.h | 2 +
>> src/compiler/nir/nir_opt_move_load_ubo.c | 116
>> +++++++++++++++++++++++++++++++
>> 4 files changed, 120 insertions(+)
>> create mode 100644 src/compiler/nir/nir_opt_move_load_ubo.c
>>
>> diff --git a/src/compiler/Makefile.sources
>> b/src/compiler/Makefile.sources
>> index 37340ba809..55143dbc66 100644
>> --- a/src/compiler/Makefile.sources
>> +++ b/src/compiler/Makefile.sources
>> @@ -266,6 +266,7 @@ NIR_FILES = \
>> nir/nir_opt_intrinsics.c \
>> nir/nir_opt_loop_unroll.c \
>> nir/nir_opt_move_comparisons.c \
>> + nir/nir_opt_move_load_ubo.c \
>> nir/nir_opt_peephole_select.c \
>> nir/nir_opt_remove_phis.c \
>> nir/nir_opt_shrink_load.c \
>> diff --git a/src/compiler/nir/meson.build b/src/compiler/nir/meson.build
>> index a70c236b95..289bb9ea78 100644
>> --- a/src/compiler/nir/meson.build
>> +++ b/src/compiler/nir/meson.build
>> @@ -160,6 +160,7 @@ files_libnir = files(
>> 'nir_opt_intrinsics.c',
>> 'nir_opt_loop_unroll.c',
>> 'nir_opt_move_comparisons.c',
>> + 'nir_opt_move_load_ubo.c',
>> 'nir_opt_peephole_select.c',
>> 'nir_opt_remove_phis.c',
>> 'nir_opt_shrink_load.c',
>> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
>> index 5b28c727c8..4224da5f82 100644
>> --- a/src/compiler/nir/nir.h
>> +++ b/src/compiler/nir/nir.h
>> @@ -2786,6 +2786,8 @@ bool nir_opt_loop_unroll(nir_shader *shader,
>> nir_variable_mode indirect_mask);
>> bool nir_opt_move_comparisons(nir_shader *shader);
>> +bool nir_opt_move_load_ubo(nir_shader *shader);
>> +
>> bool nir_opt_peephole_select(nir_shader *shader, unsigned limit);
>> bool nir_opt_remove_phis(nir_shader *shader);
>> diff --git a/src/compiler/nir/nir_opt_move_load_ubo.c
>> b/src/compiler/nir/nir_opt_move_load_ubo.c
>> new file mode 100644
>> index 0000000000..642651152b
>> --- /dev/null
>> +++ b/src/compiler/nir/nir_opt_move_load_ubo.c
>> @@ -0,0 +1,116 @@
>> +/*
>> + * 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.
>> + */
>> +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);
>
>
Ignore my comments below. As Samuel pointed out on irc the if above
makes sure we only move within the same basic block.
>
> I'm not sure this is completely safe. It assumes we will never see a
> case were the load_ubo src is used inside both branches of an if:
>
> vec1 32 ssa_6 = load_const (0x00000010 /* 0.000000 */)
> vec1 32 ssa_7 = intrinsic load_ubo (ssa_0, ssa_0) () ()
> vec1 32 ssa_8 = ieq ssa_7, ssa_0
> /* succs: block_1 block_2 */
> if ssa_8 {
> block block_1:
> /* preds: block_0 */
> vec1 32 ssa_11 = imov ssa_7.x
> vec1 32 ssa_12 = fadd ssa_1.y, ssa_10.y
> vec1 32 ssa_13 = imov ssa_7.z
> vec1 32 ssa_14 = imov ssa_7.w
> /* succs: block_3 */
> } else {
> block block_2:
> /* preds: block_0 */
> vec1 32 ssa_17 = fadd ssa_2.z, ssa_16.z
> vec1 32 ssa_18 = ieq ssa_7, ssa_3
> vec1 32 ssa_19 = load_const (0x40000000 /* 2.000000 */)
> vec1 32 ssa_20 = fadd ssa_19, ssa_16.z
> vec1 32 ssa_21 = imov ssa_7.x
> vec1 32 ssa_22 = imov ssa_7.y
> vec1 32 ssa_23 = bcsel ssa_18, ssa_20, ssa_17
> vec1 32 ssa_24 = imov ssa_7.w
> vec1 32 ssa_25 = iadd ssa_5, ssa_4
> vec1 32 ssa_26 = bcsel ssa_18, ssa_25, ssa_7
> /* succs: block_3 */
> }
> block block_3:
>
>
> I tried (and failed) to make a piglit test to trip this up but the GLSL
> IR ops kept moving the load inside each of the if branches. I'm not so
> sure that will always be the case.
>
More information about the mesa-dev
mailing list