Re: [Mesa-dev] [PATCH v4 2/7] nir: propagates if condition evaluation down some alu chains
On Mon, Aug 27, 2018 at 4:09 AM Timothy Arceri
wrote:
> v2:
> - only allow nir_op_inot or nir_op_b2i when alu input is 1.
> - use some helpers as suggested by Jason.
>
> shader-db IVB results:
>
> total instructions in shared programs: 9993483 -> 9993472 (-0.00%)
> instructions in affected programs: 1300 -> 1289 (-0.85%)
> helped: 11
> HURT: 0
>
> total cycles in shared programs: 219476091 -> 219476059 (-0.00%)
> cycles in affected programs: 7675 -> 7643 (-0.42%)
> helped: 10
> HURT: 1
> ---
> src/compiler/nir/nir_opt_if.c | 135 --
> 1 file changed, 129 insertions(+), 6 deletions(-)
>
> diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c
> index 11c6693d302..6d81705f6b7 100644
> --- a/src/compiler/nir/nir_opt_if.c
> +++ b/src/compiler/nir/nir_opt_if.c
> @@ -403,9 +403,113 @@ evaluate_if_condition(nir_if *nif, nir_cursor
> cursor, uint32_t *value)
> }
> }
>
> +/*
> + * This propagates if condition evaluation down the chain of some alu
> + * instructions. For example by checking the use of some of the following
> alu
> + * instruction we can eventually replace ssa_107 with NIR_TRUE.
> + *
> + * loop {
> + * block block_1:
> + * vec1 32 ssa_85 = load_const (0x0002)
> + * vec1 32 ssa_86 = ieq ssa_48, ssa_85
> + * vec1 32 ssa_87 = load_const (0x0001)
> + * vec1 32 ssa_88 = ieq ssa_48, ssa_87
> + * vec1 32 ssa_89 = ior ssa_86, ssa_88
> + * vec1 32 ssa_90 = ieq ssa_48, ssa_0
> + * vec1 32 ssa_91 = ior ssa_89, ssa_90
> + * if ssa_86 {
> + * block block_2:
> + * ...
> + *break
> + * } else {
> + *block block_3:
> + * }
> + * block block_4:
> + * if ssa_88 {
> + *block block_5:
> + * ...
> + *break
> + * } else {
> + *block block_6:
> + * }
> + * block block_7:
> + * if ssa_90 {
> + *block block_8:
> + * ...
> + *break
> + * } else {
> + *block block_9:
> + * }
> + * block block_10:
> + * vec1 32 ssa_107 = inot ssa_91
> + * if ssa_107 {
> + *block block_11:
> + *break
> + * } else {
> + *block block_12:
> + * }
> + * }
> + */
> static bool
> -evaluate_condition_use(nir_if *nif, nir_src *use_src, void *mem_ctx,
> - bool is_if_condition)
> +propagate_condition_eval(nir_builder *b, nir_if *nif, nir_src *use_src,
> + nir_src *alu_use, nir_alu_instr *alu, void
> *mem_ctx,
> + bool is_if_condition)
> +{
> + bool progress = false;
> +
> + uint32_t bool_value;
> + nir_cursor cursor = nir_before_src(alu_use, is_if_condition);
> + if (nir_op_infos[alu->op].num_inputs == 1) {
>
Howa about just
if (alu->op == nir_op_inot || alu->op == nir_op_b2i) {
if (evaluate_if_condition()) {
replace_if_condition_with_const();
return true;
}
} else if (alu->op == alu_op_ior || alu_op_iand) {
if (evaluate_if_condition()) {
// stuff
return true;
}
}
return false;
Or, for that matter, it could be a switch statement. My point is that
checking the number of sources and then checking the number of inputs seems
kind-of pointless.
> + if ((alu->op == nir_op_inot || alu->op == nir_op_b2i) &&
> + evaluate_if_condition(nif, cursor, _value)) {
> + replace_if_condition_use_with_const(alu_use, mem_ctx, cursor,
> + bool_value, is_if_condition);
>
This isn't correct. We need to run nir_eval_const_opcode on it before
replacing the ALU destination with it.
> + progress = true;
> + }
> + } else {
> + assert(alu->op == nir_op_ior || alu->op == nir_op_iand);
> +
> + if (evaluate_if_condition(nif, cursor, _value)) {
> + nir_ssa_def *def[2];
> + for (unsigned i = 0; i < 2; i++) {
> +if (alu->src[i].src.ssa == use_src->ssa) {
> + if (is_if_condition) {
> + b->cursor =
> + nir_before_cf_node(_use->parent_if->cf_node);
> + } else {
> + b->cursor = nir_before_instr(alu_use->parent_instr);
> + }
>
This should be nir_before_src
> +
> + nir_const_value const_value;
> + const_value.u32[0] = bool_value;
> +
> + def[i] = nir_build_imm(b, 1, 32, const_value);
> +} else {
> + def[i] = alu->src[i].src.ssa;
> +}
> + }
> +
> + nir_ssa_def *nalu =
> +nir_build_alu(b, alu->op, def[0], def[1], NULL, NULL);
> +
> + /* Rewrite use to use new alu instruction */
> + nir_src new_src = nir_src_for_ssa(nalu);
> +
> + if (is_if_condition)
> +nir_if_rewrite_condition(alu_use->parent_if, new_src);
> + else
> +
[Mesa-dev] [PATCH v4 2/7] nir: propagates if condition evaluation down some alu chains
v2:
- only allow nir_op_inot or nir_op_b2i when alu input is 1.
- use some helpers as suggested by Jason.
shader-db IVB results:
total instructions in shared programs: 9993483 -> 9993472 (-0.00%)
instructions in affected programs: 1300 -> 1289 (-0.85%)
helped: 11
HURT: 0
total cycles in shared programs: 219476091 -> 219476059 (-0.00%)
cycles in affected programs: 7675 -> 7643 (-0.42%)
helped: 10
HURT: 1
---
src/compiler/nir/nir_opt_if.c | 135 --
1 file changed, 129 insertions(+), 6 deletions(-)
diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c
index 11c6693d302..6d81705f6b7 100644
--- a/src/compiler/nir/nir_opt_if.c
+++ b/src/compiler/nir/nir_opt_if.c
@@ -403,9 +403,113 @@ evaluate_if_condition(nir_if *nif, nir_cursor cursor,
uint32_t *value)
}
}
+/*
+ * This propagates if condition evaluation down the chain of some alu
+ * instructions. For example by checking the use of some of the following alu
+ * instruction we can eventually replace ssa_107 with NIR_TRUE.
+ *
+ * loop {
+ * block block_1:
+ * vec1 32 ssa_85 = load_const (0x0002)
+ * vec1 32 ssa_86 = ieq ssa_48, ssa_85
+ * vec1 32 ssa_87 = load_const (0x0001)
+ * vec1 32 ssa_88 = ieq ssa_48, ssa_87
+ * vec1 32 ssa_89 = ior ssa_86, ssa_88
+ * vec1 32 ssa_90 = ieq ssa_48, ssa_0
+ * vec1 32 ssa_91 = ior ssa_89, ssa_90
+ * if ssa_86 {
+ * block block_2:
+ * ...
+ *break
+ * } else {
+ *block block_3:
+ * }
+ * block block_4:
+ * if ssa_88 {
+ *block block_5:
+ * ...
+ *break
+ * } else {
+ *block block_6:
+ * }
+ * block block_7:
+ * if ssa_90 {
+ *block block_8:
+ * ...
+ *break
+ * } else {
+ *block block_9:
+ * }
+ * block block_10:
+ * vec1 32 ssa_107 = inot ssa_91
+ * if ssa_107 {
+ *block block_11:
+ *break
+ * } else {
+ *block block_12:
+ * }
+ * }
+ */
static bool
-evaluate_condition_use(nir_if *nif, nir_src *use_src, void *mem_ctx,
- bool is_if_condition)
+propagate_condition_eval(nir_builder *b, nir_if *nif, nir_src *use_src,
+ nir_src *alu_use, nir_alu_instr *alu, void *mem_ctx,
+ bool is_if_condition)
+{
+ bool progress = false;
+
+ uint32_t bool_value;
+ nir_cursor cursor = nir_before_src(alu_use, is_if_condition);
+ if (nir_op_infos[alu->op].num_inputs == 1) {
+ if ((alu->op == nir_op_inot || alu->op == nir_op_b2i) &&
+ evaluate_if_condition(nif, cursor, _value)) {
+ replace_if_condition_use_with_const(alu_use, mem_ctx, cursor,
+ bool_value, is_if_condition);
+ progress = true;
+ }
+ } else {
+ assert(alu->op == nir_op_ior || alu->op == nir_op_iand);
+
+ if (evaluate_if_condition(nif, cursor, _value)) {
+ nir_ssa_def *def[2];
+ for (unsigned i = 0; i < 2; i++) {
+if (alu->src[i].src.ssa == use_src->ssa) {
+ if (is_if_condition) {
+ b->cursor =
+ nir_before_cf_node(_use->parent_if->cf_node);
+ } else {
+ b->cursor = nir_before_instr(alu_use->parent_instr);
+ }
+
+ nir_const_value const_value;
+ const_value.u32[0] = bool_value;
+
+ def[i] = nir_build_imm(b, 1, 32, const_value);
+} else {
+ def[i] = alu->src[i].src.ssa;
+}
+ }
+
+ nir_ssa_def *nalu =
+nir_build_alu(b, alu->op, def[0], def[1], NULL, NULL);
+
+ /* Rewrite use to use new alu instruction */
+ nir_src new_src = nir_src_for_ssa(nalu);
+
+ if (is_if_condition)
+nir_if_rewrite_condition(alu_use->parent_if, new_src);
+ else
+nir_instr_rewrite_src(alu_use->parent_instr, alu_use, new_src);
+
+ progress = true;
+ }
+ }
+
+ return progress;
+}
+
+static bool
+evaluate_condition_use(nir_builder *b, nir_if *nif, nir_src *use_src,
+ void *mem_ctx, bool is_if_condition)
{
bool progress = false;
@@ -417,23 +521,42 @@ evaluate_condition_use(nir_if *nif, nir_src *use_src,
void *mem_ctx,
progress = true;
}
+ if (!is_if_condition &&
+ use_src->parent_instr->type == nir_instr_type_alu &&
+ (nir_instr_as_alu(use_src->parent_instr)->op == nir_op_ior ||
+nir_instr_as_alu(use_src->parent_instr)->op == nir_op_iand ||
+nir_op_infos[nir_instr_as_alu(use_src->parent_instr)->op].num_inputs
== 1)) {
+
+ nir_alu_instr *alu = nir_instr_as_alu(use_src->parent_instr);
+
+ nir_foreach_use_safe(alu_use, >dest.dest.ssa) {
+progress |= propagate_condition_eval(b, nif, use_src,
